Repository: boto/botocore (python)

Issue:
Not compatible with endpoints that have a url_path (regression from boto)
Even though `url_path` is part of the `request_dict`, it is only ever set to `/`, the hard-coded default here https://github.com/boto/botocore/blob/20f8eb43d75a3d10cea3e4b5976be40be49c7660/botocore/serialize.py#L170.
This breaks endpoints such as `https://nova.rc.nectar.org.au:8773/services/Cloud`.

Here is an example how it worked with boto: https://support.rc.nectar.org.au/docs/boto

It would be great if this behaviour was preserved in botocore

Pull request:
Support URL paths in an endpoint_url
Fixes #420
Also fixes aws/aws-cli#961
Closes #460

This fixes the issue by extracting out the logic for converting
a request_dict that we get from a serializer to an AWSRequest object
that's used by requests.  This also lets have have more
specific tests for this functionality.

cc @kyleknap @danielgtaylor 

Patch:
diff --git a/botocore/endpoint.py b/botocore/endpoint.py
index 7473917e0..dc84ea6be 100644
--- a/botocore/endpoint.py
+++ b/botocore/endpoint.py
@@ -23,8 +23,9 @@ from botocore.vendored import six
 
 from botocore.exceptions import UnknownEndpointError
 from botocore.awsrequest import AWSRequest
-from botocore.compat import urljoin, filter_ssl_san_warnings
-from botocore.utils import percent_encode_sequence
+from botocore.compat import filter_ssl_san_warnings, urlsplit
+from botocore.compat import urlunsplit
+from botocore.utils import percent_encode_sequence, normalize_url_path
 from botocore.hooks import first_non_none_response
 from botocore.response import StreamingBody
 from botocore import parsers
@@ -71,6 +72,61 @@ class PreserveAuthSession(Session):
         pass
 
 
+class RequestCreator(object):
+    """This object can convert a request dict to an AWSRequest class."""
+
+    def create_request_object(self, request_dict, user_agent, endpoint_url):
+        """
+
+        :type request_dict: dict
+        :param request_dict:  The request dict (created from the ``serialize``
+            module).
+
+        :type user_agent: string
+        :param user_agent: The user agent to use for this request.
+
+        :type endpoint_url: string
+        :param endpoint_url: The full endpoint url, which contains at least
+            the scheme, the hostname, and optionally any path components.
+
+        :rtype: ``botocore.awsrequest.AWSRequest``
+        :return: An AWSRequest object based on the request_dict.
+
+        """
+        r = request_dict
+        headers = r['headers']
+        headers['User-Agent'] = user_agent
+        url = self._urljoin(endpoint_url, r['url_path'])
+        if r['query_string']:
+            encoded_query_string = percent_encode_sequence(r['query_string'])
+            if '?' not in url:
+                url += '?%s' % encoded_query_string
+            else:
+                url += '&%s' % encoded_query_string
+        request = AWSRequest(method=r['method'], url=url,
+                             data=r['body'],
+                             headers=headers)
+        return request
+
+    def _urljoin(self, endpoint_url, url_path):
+        p = urlsplit(endpoint_url)
+        # <part>   - <index>
+        # scheme   - p[0]
+        # netloc   - p[1]
+        # path     - p[2]
+        # query    - p[3]
+        # fragment - p[4]
+        if not url_path or url_path == '/':
+            # If there's no path component, ensure the URL ends with
+            # a '/' for backwards compatibility.
+            if not p[2]:
+                return endpoint_url + '/'
+            return endpoint_url
+        new_path = normalize_url_path(p[2] + url_path)
+        reconstructed = urlunsplit((p[0], p[1], new_path, p[3], p[4]))
+        return reconstructed
+
+
 class Endpoint(object):
     """
     Represents an endpoint for a particular service in a specific
@@ -100,6 +156,7 @@ class Endpoint(object):
         if response_parser_factory is None:
             response_parser_factory = parsers.ResponseParserFactory()
         self._response_parser_factory = response_parser_factory
+        self._request_creator = RequestCreator()
 
     def __repr__(self):
         return '%s(%s)' % (self._endpoint_prefix, self.host)
@@ -110,7 +167,8 @@ class Endpoint(object):
         return self._send_request(request_dict, operation_model)
 
     def create_request(self, params, operation_model=None):
-        request = self._create_request_object(params)
+        request = self._request_creator.create_request_object(
+            params, self._user_agent, self.host)
         if operation_model:
             event_name = 'request-created.{endpoint_prefix}.{op_name}'.format(
                 endpoint_prefix=self._endpoint_prefix,
@@ -120,23 +178,6 @@ class Endpoint(object):
         prepared_request = self.prepare_request(request)
         return prepared_request
 
-    def _create_request_object(self, request_dict):
-        r = request_dict
-        user_agent = self._user_agent
-        headers = r['headers']
-        headers['User-Agent'] = user_agent
-        url = urljoin(self.host, r['url_path'])
-        if r['query_string']:
-            encoded_query_string = percent_encode_sequence(r['query_string'])
-            if '?' not in url:
-                url += '?%s' % encoded_query_string
-            else:
-                url += '&%s' % encoded_query_string
-        request = AWSRequest(method=r['method'], url=url,
-                             data=r['body'],
-                             headers=headers)
-        return request
-
     def _encode_headers(self, headers):
         # In place encoding of headers to utf-8 if they are unicode.
         for key, value in headers.items():
@@ -336,7 +377,6 @@ class EndpointCreator(object):
 
         return region_name
 
-
     def _get_endpoint(self, service_model, region_name, endpoint_url,
                       verify, response_parser_factory):
         service_name = service_model.signing_name


Test patch:
diff --git a/tests/unit/test_endpoint.py b/tests/unit/test_endpoint.py
index cc274a300..701e2ff25 100644
--- a/tests/unit/test_endpoint.py
+++ b/tests/unit/test_endpoint.py
@@ -22,6 +22,7 @@ from botocore.awsrequest import AWSRequest
 from botocore.endpoint import get_endpoint, Endpoint, DEFAULT_TIMEOUT
 from botocore.endpoint import EndpointCreator
 from botocore.endpoint import PreserveAuthSession
+from botocore.endpoint import RequestCreator
 from botocore.auth import SigV4Auth
 from botocore.session import Session
 from botocore.exceptions import UnknownServiceStyle
@@ -391,3 +392,92 @@ class TestAWSSession(unittest.TestCase):
         self.assertEqual(
             redirected_request.headers['Authorization'],
             'original auth header')
+
+
+class TestRequestCreator(unittest.TestCase):
+    def setUp(self):
+        self.request_creator = RequestCreator()
+        self.user_agent = 'botocore/1.0'
+        self.endpoint_url = 'https://s3.amazonaws.com'
+        self.base_request_dict = {
+            'body': '',
+            'headers': {},
+            'method': u'GET',
+            'query_string': '',
+            'url_path': '/'
+        }
+
+    def create_request(self, request_dict, endpoint_url=None,
+                       user_agent=None):
+        self.base_request_dict.update(request_dict)
+        if user_agent is None:
+            user_agent = self.user_agent
+        if endpoint_url is None:
+            endpoint_url = self.endpoint_url
+        return self.request_creator.create_request_object(
+            self.base_request_dict, user_agent, endpoint_url)
+
+    def test_create_request_object_for_get(self):
+        request_dict = {
+            'method': u'GET',
+            'url_path': '/'
+        }
+        request = self.create_request(
+            request_dict, endpoint_url='https://s3.amazonaws.com')
+        self.assertEqual(request.method, 'GET')
+        self.assertEqual(request.url, 'https://s3.amazonaws.com/')
+        self.assertEqual(request.headers['User-Agent'], self.user_agent)
+
+    def test_query_string_serialized_to_url(self):
+        request_dict = {
+            'method': u'GET',
+            'query_string': {u'prefix': u'foo'},
+            'url_path': u'/mybucket'
+        }
+        request = self.create_request(request_dict)
+        self.assertEqual(
+            request.url,
+            'https://s3.amazonaws.com/mybucket?prefix=foo')
+
+    def test_url_path_combined_with_endpoint_url(self):
+        # This checks the case where a user specifies and
+        # endpoint_url that has a path component, and the
+        # serializer gives us a request_dict that has a url
+        # component as well (say from a rest-* service).
+        request_dict = {
+            'query_string': {u'prefix': u'foo'},
+            'url_path': u'/mybucket'
+        }
+        endpoint_url = 'https://custom.endpoint/foo/bar'
+        request = self.create_request(request_dict, endpoint_url)
+        self.assertEqual(
+            request.url,
+            'https://custom.endpoint/foo/bar/mybucket?prefix=foo')
+
+    def test_url_path_with_trailing_slash(self):
+        self.assertEqual(
+            self.create_request(
+                {'url_path': u'/mybucket'},
+                endpoint_url='https://custom.endpoint/foo/bar/').url,
+            'https://custom.endpoint/foo/bar/mybucket')
+
+    def test_url_path_is_slash(self):
+        self.assertEqual(
+            self.create_request(
+                {'url_path': u'/'},
+                endpoint_url='https://custom.endpoint/foo/bar/').url,
+            'https://custom.endpoint/foo/bar/')
+
+    def test_url_path_is_slash_with_endpoint_url_no_slash(self):
+        self.assertEqual(
+            self.create_request(
+                {'url_path': u'/'},
+                endpoint_url='https://custom.endpoint/foo/bar').url,
+            'https://custom.endpoint/foo/bar')
+
+    def test_custom_endpoint_with_query_string(self):
+        self.assertEqual(
+            self.create_request(
+                {'url_path': u'/baz', 'query_string': {'x': 'y'}},
+                endpoint_url='https://custom.endpoint/foo/bar?foo=bar').url,
+            'https://custom.endpoint/foo/bar/baz?foo=bar&x=y')