From 37d0099cfd7bfe4bdb9f1cddc6bb2b62f4609f60 Mon Sep 17 00:00:00 2001 From: Phil Adams Date: Mon, 15 Nov 2021 09:39:22 -0600 Subject: [PATCH] fix: strip trailing slashes in BaseService.set_service_url (#130) This commit modifies the BaseService.set_service_url() function so that it removes any trailing slashes. Previously, a service URL containing a trailing slash (e.g. "https://myserver.com/api/v1/") would result in a request URL that contains two slashes between the service URL and the operation path (e.g. "https://myserver.com/api/v1//myoperationpath"), and some server implementations might fail to process such a request properly. With the changes in this commit, a request URL of "https://myserver.com/api/v1/myoperationpath" will be used instead. --- ibm_cloud_sdk_core/base_service.py | 6 +++ test/test_base_service.py | 70 ++++++++++++++++++------------ test/test_utils.py | 16 +++++++ 3 files changed, 65 insertions(+), 27 deletions(-) diff --git a/ibm_cloud_sdk_core/base_service.py b/ibm_cloud_sdk_core/base_service.py index 569885d..d9fc742 100644 --- a/ibm_cloud_sdk_core/base_service.py +++ b/ibm_cloud_sdk_core/base_service.py @@ -224,6 +224,8 @@ def set_service_url(self, service_url: str) -> None: 'The service url shouldn\'t start or end with curly brackets or quotes. ' 'Be sure to remove any {} and \" characters surrounding your service url' ) + if service_url is not None: + service_url = service_url.rstrip('/') self.service_url = service_url def get_http_client(self) -> requests.sessions.Session: @@ -368,6 +370,10 @@ def prepare_request(self, # validate the service url is set if not self.service_url: raise ValueError('The service_url is required') + + # Combine the service_url and operation path to form the request url. + # Note: we have already stripped any trailing slashes from the service_url + # and we know that the operation path ('url') will start with a slash. request['url'] = strip_extra_slashes(self.service_url + url) headers = remove_null_values(headers) if headers else {} diff --git a/test/test_base_service.py b/test/test_base_service.py index 0fd5a98..6df06f0 100644 --- a/test/test_base_service.py +++ b/test/test_base_service.py @@ -20,7 +20,6 @@ from ibm_cloud_sdk_core import get_authenticator_from_environment from ibm_cloud_sdk_core.authenticators import (IAMAuthenticator, NoAuthAuthenticator, Authenticator, BasicAuthenticator, CloudPakForDataAuthenticator) -from ibm_cloud_sdk_core.utils import strip_extra_slashes class IncludeExternalConfigService(BaseService): @@ -698,6 +697,7 @@ def test_user_agent_header(): assert response.get_result().request.headers.__getitem__( 'user-agent') == user_agent_header['User-Agent'] + @responses.activate def test_reserved_keys(caplog): service = AnyServiceV1('2021-07-02', authenticator=NoAuthAuthenticator()) @@ -723,6 +723,7 @@ def test_reserved_keys(caplog): assert caplog.record_tuples[2][2] == '"headers" has been removed from the request' assert caplog.record_tuples[3][2] == '"cookies" has been removed from the request' + @responses.activate def test_ssl_error(): responses.add( @@ -810,56 +811,71 @@ def test_json(): service = AnyServiceV1('2018-11-20', authenticator=NoAuthAuthenticator()) req = service.prepare_request('POST', url='', headers={ 'X-opt-out': True}, data={'hello': 'world', 'fóó': 'bår'}) - assert req.get('data') == b'{"hello": "world", "f\\u00f3\\u00f3": "b\\u00e5r"}' + assert req.get( + 'data') == b'{"hello": "world", "f\\u00f3\\u00f3": "b\\u00e5r"}' -def test_trailing_slash(): - assert strip_extra_slashes('') == '' - assert strip_extra_slashes('//') == '/' - assert strip_extra_slashes('/////') == '/' - assert strip_extra_slashes('https://host') == 'https://host' - assert strip_extra_slashes('https://host/') == 'https://host/' - assert strip_extra_slashes('https://host//') == 'https://host/' - assert strip_extra_slashes('https://host/path') == 'https://host/path' - assert strip_extra_slashes('https://host/path/') == 'https://host/path/' - assert strip_extra_slashes('https://host/path//') == 'https://host/path/' - assert strip_extra_slashes('https://host//path//') == 'https://host//path/' - +def test_service_url_handling(): service = AnyServiceV1( - '2018-11-20', service_url='https://host/', authenticator=NoAuthAuthenticator()) - assert service.service_url == 'https://host/' + '2018-11-20', service_url='https://host///////', authenticator=NoAuthAuthenticator()) + assert service.service_url == 'https://host' + service.set_service_url('https://host/') - assert service.service_url == 'https://host/' + assert service.service_url == 'https://host' + req = service.prepare_request('POST', url='/path/', headers={'X-opt-out': True}, data={'hello': 'world'}) - assert req.get('url') == 'https://host//path/' + assert req.get('url') == 'https://host/path/' service = AnyServiceV1( '2018-11-20', service_url='https://host/', authenticator=NoAuthAuthenticator()) - assert service.service_url == 'https://host/' + assert service.service_url == 'https://host' + service.set_service_url('https://host/') - assert service.service_url == 'https://host/' + assert service.service_url == 'https://host' + req = service.prepare_request('POST', url='/', headers={'X-opt-out': True}, data={'hello': 'world'}) assert req.get('url') == 'https://host/' + req = service.prepare_request('POST', + url='////', + headers={'X-opt-out': True}, + data={'hello': 'world'}) + assert req.get('url') == 'https://host/' + service.set_service_url(None) assert service.service_url is None service = AnyServiceV1('2018-11-20', service_url='/', authenticator=NoAuthAuthenticator()) - assert service.service_url == '/' + assert service.service_url == '' + service.set_service_url('/') - assert service.service_url == '/' - req = service.prepare_request('POST', - url='/', - headers={'X-opt-out': True}, - data={'hello': 'world'}) - assert req.get('url') == '/' + assert service.service_url == '' + + with pytest.raises(ValueError) as err: + service.prepare_request('POST', + url='/', + headers={'X-opt-out': True}, + data={'hello': 'world'}) + assert str(err.value) == 'The service_url is required' + + +def test_service_url_slash(): + service = AnyServiceV1('2018-11-20', service_url='/', + authenticator=NoAuthAuthenticator()) + assert service.service_url == '' + with pytest.raises(ValueError) as err: + service.prepare_request('POST', + url='/', + headers={'X-opt-out': True}, + data={'hello': 'world'}) + assert str(err.value) == 'The service_url is required' def test_service_url_not_set(): diff --git a/test/test_utils.py b/test/test_utils.py index 8347744..e69a5a9 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -28,6 +28,7 @@ from ibm_cloud_sdk_core import get_query_param from ibm_cloud_sdk_core import read_external_sources from ibm_cloud_sdk_core.authenticators import Authenticator, BasicAuthenticator, IAMAuthenticator +from ibm_cloud_sdk_core.utils import strip_extra_slashes def datetime_test(datestr: str, expected: str): @@ -616,3 +617,18 @@ def test_read_external_sources_2(): config = read_external_sources('service_1') assert config.get('URL') == 'service1.com/api' + + +def test_strip_extra_slashes(): + assert strip_extra_slashes('') == '' + assert strip_extra_slashes('//') == '/' + assert strip_extra_slashes('/////') == '/' + assert strip_extra_slashes('https://host') == 'https://host' + assert strip_extra_slashes('https://host/') == 'https://host/' + assert strip_extra_slashes('https://host//') == 'https://host/' + assert strip_extra_slashes('https://host/path') == 'https://host/path' + assert strip_extra_slashes('https://host/path/') == 'https://host/path/' + assert strip_extra_slashes('https://host/path//') == 'https://host/path/' + assert strip_extra_slashes('https://host//path//') == 'https://host//path/' + assert strip_extra_slashes( + 'https://host//path//////////') == 'https://host//path/'