Fixed space encoding in base string URI used in the signature base string.

3 files changed, 55 insertions, 15 deletions

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 2cc0dd3..8036614 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -1,6 +1,11 @@
 Changelog
 =========
 
+TBD
+---
+
+* Fixed space encoding in base string URI used in the signature base string.
+
 3.0.0 (2019-01-01)
 ------------------
 OAuth2.0 Provider - outstanding Features
diff --git a/oauthlib/oauth1/rfc5849/signature.py b/oauthlib/oauth1/rfc5849/signature.py
index e90d6f3..589b68a 100644
--- a/oauthlib/oauth1/rfc5849/signature.py
+++ b/oauthlib/oauth1/rfc5849/signature.py
@@ -105,9 +105,9 @@ def construct_base_string(http_method, base_string_uri,
     return base_string
 
 
-def normalize_base_string_uri(uri, host=None):
+def base_string_uri(uri, host=None):
     """**Base String URI**
-    Per `section 3.4.1.2`_ of the spec.
+    Per `section 3.4.1.2`_ of RFC 5849.
 
     For example, the HTTP request::
 
@@ -177,7 +177,31 @@ def normalize_base_string_uri(uri, host=None):
         if (scheme, port) in default_ports:
             netloc = host
 
-    return urlparse.urlunparse((scheme, netloc, path, params, '', ''))
+    v = urlparse.urlunparse((scheme, netloc, path, params, '', ''))
+
+    # RFC 5849 does not specify which characters are encoded in the
+    # "base string URI", nor how they are encoded - which is very bad, since
+    # the signatures won't match if there are any differences. Fortunately,
+    # most URIs only use characters that are clearly not encoded (e.g. digits
+    # and A-Z, a-z), so have avoided any differences between implementations.
+    #
+    # The example from its section 3.4.1.2 illustrates that spaces in
+    # the path are percent encoded. But it provides no guidance as to what other
+    # characters (if any) must be encoded (nor how); nor if characters in the
+    # other components are to be encoded or not.
+    #
+    # This implementation **assumes** that **only** the space is percent-encoded
+    # and it is done to the entire value (not just to spaces in the path).
+    #
+    # This code may need to be changed if it is discovered that other characters
+    # are expected to be encoded.
+    #
+    # Note: the "base string URI" returned by this function will be encoded
+    # again before being concatenated into the "signature base string". So any
+    # spaces in the URI will actually appear in the "signature base string"
+    # as "%2520" (the "%20" further encoded according to section 3.6).
+
+    return v.replace(' ', '%20')
 
 
 # ** Request Parameters **
@@ -624,8 +648,8 @@ def verify_hmac_sha1(request, client_secret=None,
 
     """
     norm_params = normalize_parameters(request.params)
-    uri = normalize_base_string_uri(request.uri)
-    base_string = construct_base_string(request.http_method, uri, norm_params)
+    bs_uri = base_string_uri(request.uri)
+    base_string = construct_base_string(request.http_method, bs_uri, norm_params)
     signature = sign_hmac_sha1(base_string, client_secret,
                                resource_owner_secret)
     match = safe_string_equals(signature, request.signature)
@@ -657,8 +681,8 @@ def verify_rsa_sha1(request, rsa_public_key):
     .. _`RFC2616 section 5.2`: https://tools.ietf.org/html/rfc2616#section-5.2
     """
     norm_params = normalize_parameters(request.params)
-    uri = normalize_base_string_uri(request.uri)
-    message = construct_base_string(request.http_method, uri, norm_params).encode('utf-8')
+    bs_uri = base_string_uri(request.uri)
+    message = construct_base_string(request.http_method, bs_uri, norm_params).encode('utf-8')
     sig = binascii.a2b_base64(request.signature.encode('utf-8'))
 
     alg = _jwt_rs1_signing_algorithm()
diff --git a/tests/oauth1/rfc5849/test_signatures.py b/tests/oauth1/rfc5849/test_signatures.py
index 48609e5..db2c532 100644
--- a/tests/oauth1/rfc5849/test_signatures.py
+++ b/tests/oauth1/rfc5849/test_signatures.py
@@ -4,7 +4,7 @@ from __future__ import absolute_import, unicode_literals
 from oauthlib.common import unicode_type
 from oauthlib.oauth1.rfc5849.signature import (collect_parameters,
                                                construct_base_string,
-                                               normalize_base_string_uri,
+                                               base_string_uri,
                                                normalize_parameters,
                                                sign_hmac_sha1,
                                                sign_hmac_sha1_with_client,
@@ -125,7 +125,7 @@ class SignatureTests(TestCase):
 
         self.assertEqual(self.control_base_string, base_string)
 
-    def test_normalize_base_string_uri(self):
+    def test_base_string_uri(self):
         """
         Example text to be turned into a normalized base string uri::
 
@@ -137,33 +137,44 @@ class SignatureTests(TestCase):
             https://www.example.net:8080/
         """
 
+        # test first example from RFC 5849 section 3.4.1.2.
+        # Note: there is a space between "r" and "v"
+        uri = 'http://EXAMPLE.COM:80/r v/X?id=123'
+        self.assertEqual(base_string_uri(uri),
+                         'http://example.com/r%20v/X')
+
+        # test second example from RFC 5849 section 3.4.1.2.
+        uri = 'https://www.example.net:8080/?q=1'
+        self.assertEqual(base_string_uri(uri),
+                         'https://www.example.net:8080/')
+
         # test for unicode failure
         uri = b"www.example.com:8080"
-        self.assertRaises(ValueError, normalize_base_string_uri, uri)
+        self.assertRaises(ValueError, base_string_uri, uri)
 
         # test for missing scheme
         uri = "www.example.com:8080"
-        self.assertRaises(ValueError, normalize_base_string_uri, uri)
+        self.assertRaises(ValueError, base_string_uri, uri)
 
         # test a URI with the default port
         uri = "http://www.example.com:80/"
-        self.assertEqual(normalize_base_string_uri(uri),
+        self.assertEqual(base_string_uri(uri),
                          "http://www.example.com/")
 
         # test a URI missing a path
         uri = "http://www.example.com"
-        self.assertEqual(normalize_base_string_uri(uri),
+        self.assertEqual(base_string_uri(uri),
                          "http://www.example.com/")
 
         # test a relative URI
         uri = "/a-host-relative-uri"
         host = "www.example.com"
-        self.assertRaises(ValueError, normalize_base_string_uri, (uri, host))
+        self.assertRaises(ValueError, base_string_uri, (uri, host))
 
         # test overriding the URI's netloc with a host argument
         uri = "http://www.example.com/a-path"
         host = "alternatehost.example.com"
-        self.assertEqual(normalize_base_string_uri(uri, host),
+        self.assertEqual(base_string_uri(uri, host),
                          "http://alternatehost.example.com/a-path")
 
     def test_collect_parameters(self):