From e02a8f0e3f8af56dfc64c547c4c44b15250b447d Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Fri, 13 Jan 2017 13:07:05 +1100 Subject: [PATCH 1/8] fix a bug where creating a connection from URL would not work when request actions start with /. Urlparse instead of ''.join --- libcloud/common/base.py | 4 +--- libcloud/httplib_ssl.py | 12 ++++++++++-- libcloud/test/test_connection.py | 14 ++++++++++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/libcloud/common/base.py b/libcloud/common/base.py index d33872ed75..c045cd287d 100644 --- a/libcloud/common/base.py +++ b/libcloud/common/base.py @@ -674,9 +674,7 @@ def request(self, action, params=None, data=None, headers=None, return response def morph_action_hook(self, action): - if not action.startswith("/"): - action = "/" + action - return self.request_path + action + return urlparse.urljoin(self.request_path, action) def add_default_params(self, params): """ diff --git a/libcloud/httplib_ssl.py b/libcloud/httplib_ssl.py index 8bd9d89c50..913ad6ca48 100644 --- a/libcloud/httplib_ssl.py +++ b/libcloud/httplib_ssl.py @@ -182,16 +182,24 @@ def __init__(self, host, port, secure=None, **kwargs): self.set_http_proxy(proxy_url=proxy_url) self.session.timeout = kwargs.get('timeout', 60) + @property + def verification(self): + """ + The option for SSL verification given to underlying requests + """ + return self.ca_cert if self.ca_cert is not None else self.verify + def request(self, method, url, body=None, headers=None, raw=False, stream=False): + url = urlparse.urljoin(self.host, url) self.response = self.session.request( method=method.lower(), - url=''.join([self.host, url]), + url=url, data=body, headers=headers, allow_redirects=1, stream=stream, - verify=self.ca_cert if self.ca_cert is not None else self.verify + verify=self.verification ) def prepared_request(self, method, url, body=None, diff --git a/libcloud/test/test_connection.py b/libcloud/test/test_connection.py index d58ff3f9a2..d4bca52f54 100644 --- a/libcloud/test/test_connection.py +++ b/libcloud/test/test_connection.py @@ -21,6 +21,8 @@ from mock import Mock, patch +import requests_mock + from libcloud.test import unittest from libcloud.common.base import Connection from libcloud.httplib_ssl import LibcloudBaseConnection @@ -109,6 +111,18 @@ def test_connection_to_unusual_port(self): conn = LibcloudConnection(host='localhost', port=80) self.assertEqual(conn.host, 'http://localhost') + def test_connection_url_merging(self): + """ + Test that the connection class will parse URLs correctly + """ + conn = Connection(url='http://test.com/') + conn.connect() + self.assertEqual(conn.connection.host, 'http://test.com') + with requests_mock.mock() as m: + m.get('http://test.com/test', text='data') + response = conn.request('/test') + self.assertEqual(response.body, 'data') + def test_secure_connection_unusual_port(self): """ Test that the connection class will default to secure (https) even From 11fc4c8d05b300fc4b0b71ff5c48c35f1dbd293b Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Fri, 13 Jan 2017 13:12:59 +1100 Subject: [PATCH 2/8] make redirects configurable --- libcloud/httplib_ssl.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libcloud/httplib_ssl.py b/libcloud/httplib_ssl.py index 913ad6ca48..0110e69ceb 100644 --- a/libcloud/httplib_ssl.py +++ b/libcloud/httplib_ssl.py @@ -32,6 +32,8 @@ 'LibcloudConnection' ] +ALLOW_REDIRECTS = 1 + HTTP_PROXY_ENV_VARIABLE_NAME = 'http_proxy' # Error message which is thrown when establishing SSL / TLS connection fails @@ -197,7 +199,7 @@ def request(self, method, url, body=None, headers=None, raw=False, url=url, data=body, headers=headers, - allow_redirects=1, + allow_redirects=ALLOW_REDIRECTS, stream=stream, verify=self.verification ) From 4c1fff09a776cc1ebfa9d0776abee807c31d2e18 Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Fri, 13 Jan 2017 13:16:17 +1100 Subject: [PATCH 3/8] remove get_socket_error_exception as this behaviour is handled by requests --- libcloud/httplib_ssl.py | 48 ----------------------------------------- 1 file changed, 48 deletions(-) diff --git a/libcloud/httplib_ssl.py b/libcloud/httplib_ssl.py index 0110e69ceb..302fe25d90 100644 --- a/libcloud/httplib_ssl.py +++ b/libcloud/httplib_ssl.py @@ -36,24 +36,6 @@ HTTP_PROXY_ENV_VARIABLE_NAME = 'http_proxy' -# Error message which is thrown when establishing SSL / TLS connection fails -UNSUPPORTED_TLS_VERSION_ERROR_MSG = """ -Failed to establish SSL / TLS connection (%s). It is possible that the server \ -doesn't support requested SSL / TLS version (%s). -For information on how to work around this issue, please see \ -https://libcloud.readthedocs.org/en/latest/other/\ -ssl-certificate-validation.html#changing-used-ssl-tls-version -""".strip() - -# Maps ssl.PROTOCOL_* constant to the actual SSL / TLS version name -SSL_CONSTANT_TO_TLS_VERSION_MAP = { - 0: 'SSL v2', - 2: 'SSLv3, TLS v1.0, TLS v1.1, TLS v1.2', - 3: 'TLS v1.0', - 4: 'TLS v1.1', - 5: 'TLS v1.2' -} - class LibcloudBaseConnection(object): """ @@ -290,33 +272,3 @@ def reason(self): def version(self): # requests doesn't expose this return '11' - - -def get_socket_error_exception(ssl_version, exc): - """ - Function which intercepts socket.error exceptions and re-throws an - exception with a more user-friendly message in case server doesn't support - requested SSL version. - """ - exc_msg = str(exc) - - # Re-throw an exception with a more friendly error message - if 'connection reset by peer' in exc_msg.lower(): - ssl_version_name = SSL_CONSTANT_TO_TLS_VERSION_MAP[ssl_version] - msg = (UNSUPPORTED_TLS_VERSION_ERROR_MSG % - (exc_msg, ssl_version_name)) - - # Note: In some cases arguments are (errno, message) and in - # other it's just (message,) - exc_args = getattr(exc, 'args', []) - - if len(exc_args) == 2: - new_exc_args = [exc.args[0], msg] - else: - new_exc_args = [msg] - - new_exc = socket.error(*new_exc_args) - new_exc.original_exc = exc - return new_exc - else: - return exc From 42c58a4635d9d7b66e9b9f3b8d8cd9f889866fe1 Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Fri, 13 Jan 2017 13:18:56 +1100 Subject: [PATCH 4/8] tidy up hashing algo method --- libcloud/storage/base.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/libcloud/storage/base.py b/libcloud/storage/base.py index 233cc2502a..6db1d4dcf6 100644 --- a/libcloud/storage/base.py +++ b/libcloud/storage/base.py @@ -650,20 +650,14 @@ def _hash_buffered_stream(self, stream, hasher, blocksize=65536): return (hasher.hexdigest(), total_len) if not hasattr(stream, '__exit__'): for s in stream: - if isinstance(s, str): - hasher.update(s) - else: - hasher.update(s) + hasher.update(s) total_len = total_len + len(s) return (hasher.hexdigest(), total_len) with stream: buf = stream.read(blocksize) while len(buf) > 0: total_len = total_len + len(buf) - if isinstance(buf, str): - hasher.update(buf) - else: - hasher.update(buf) + hasher.update(buf) buf = stream.read(blocksize) return (hasher.hexdigest(), total_len) From 57e8b62c55774fffcb0d5f8d438a0416b7d6e12f Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Fri, 13 Jan 2017 13:22:27 +1100 Subject: [PATCH 5/8] linting updates --- libcloud/httplib_ssl.py | 1 - 1 file changed, 1 deletion(-) diff --git a/libcloud/httplib_ssl.py b/libcloud/httplib_ssl.py index 302fe25d90..30579bea06 100644 --- a/libcloud/httplib_ssl.py +++ b/libcloud/httplib_ssl.py @@ -19,7 +19,6 @@ """ import os -import socket import warnings import requests From fedace00261bcbb0a45bda003b968101a897d4ed Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Fri, 13 Jan 2017 14:04:28 +1100 Subject: [PATCH 6/8] fix morph_action_hook and create a list of tests to make sure every scenarion works as expected --- libcloud/common/base.py | 7 +++++- libcloud/test/test_connection.py | 37 ++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/libcloud/common/base.py b/libcloud/common/base.py index c045cd287d..c5759f33f9 100644 --- a/libcloud/common/base.py +++ b/libcloud/common/base.py @@ -674,7 +674,12 @@ def request(self, action, params=None, data=None, headers=None, return response def morph_action_hook(self, action): - return urlparse.urljoin(self.request_path, action) + url = urlparse.urljoin(self.request_path.lstrip('/').rstrip('/') + + '/', action.lstrip('/')) + if not url.startswith('/'): + return '/' + url + else: + return url def add_default_params(self, params): """ diff --git a/libcloud/test/test_connection.py b/libcloud/test/test_connection.py index d4bca52f54..8a4e9b64a9 100644 --- a/libcloud/test/test_connection.py +++ b/libcloud/test/test_connection.py @@ -123,6 +123,43 @@ def test_connection_url_merging(self): response = conn.request('/test') self.assertEqual(response.body, 'data') + def test_morph_action_hook(self): + conn = Connection(url="http://test.com") + + conn.request_path = '' + self.assertEqual(conn.morph_action_hook('/test'), '/test') + self.assertEqual(conn.morph_action_hook('test'), '/test') + + conn.request_path = '/v1' + self.assertEqual(conn.morph_action_hook('/test'), '/v1/test') + self.assertEqual(conn.morph_action_hook('test'), '/v1/test') + + conn.request_path = '/v1' + self.assertEqual(conn.morph_action_hook('/test'), '/v1/test') + self.assertEqual(conn.morph_action_hook('test'), '/v1/test') + + conn.request_path = 'v1' + self.assertEqual(conn.morph_action_hook('/test'), '/v1/test') + self.assertEqual(conn.morph_action_hook('test'), '/v1/test') + + conn.request_path = 'v1/' + self.assertEqual(conn.morph_action_hook('/test'), '/v1/test') + self.assertEqual(conn.morph_action_hook('test'), '/v1/test') + + def test_connect_with_prefix(self): + """ + Test that a connection with a base path (e.g. /v1/) will + add the base path to requests + """ + conn = Connection(url='http://test.com/') + conn.connect() + conn.request_path = '/v1' + self.assertEqual(conn.connection.host, 'http://test.com') + with requests_mock.mock() as m: + m.get('http://test.com/v1/test', text='data') + response = conn.request('/test') + self.assertEqual(response.body, 'data') + def test_secure_connection_unusual_port(self): """ Test that the connection class will default to secure (https) even From 226783bcfa020eeb0e4738bd2a6f863f5aa72a43 Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Fri, 13 Jan 2017 14:10:23 +1100 Subject: [PATCH 7/8] fix name of profitbricks test --- libcloud/test/compute/test_profitbricks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcloud/test/compute/test_profitbricks.py b/libcloud/test/compute/test_profitbricks.py index ac83b1c109..62c892bd1c 100644 --- a/libcloud/test/compute/test_profitbricks.py +++ b/libcloud/test/compute/test_profitbricks.py @@ -4962,7 +4962,7 @@ def _cloudapi_v3_datacenters_dc_1_lans_10( GET - fetch a list of snapshots ''' - def _cloudapi_v3__snapshots( + def _cloudapi_v3_snapshots( self, method, url, body, headers ): if method == 'GET': From ee5a227983b8b8a271f693c9f86879adb1539509 Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Fri, 13 Jan 2017 14:19:14 +1100 Subject: [PATCH 8/8] profitbricks driver doesn't format URLs correctly --- libcloud/compute/drivers/profitbricks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcloud/compute/drivers/profitbricks.py b/libcloud/compute/drivers/profitbricks.py index 8013e3ba27..43996d460a 100644 --- a/libcloud/compute/drivers/profitbricks.py +++ b/libcloud/compute/drivers/profitbricks.py @@ -127,7 +127,7 @@ def request(self, action, params=None, data=None, headers=None, host and protocol components. ''' if not with_full_url or with_full_url is False: - action = self.api_prefix + action + action = self.api_prefix + action.lstrip('/') else: action = action.replace( 'https://{host}'.format(host=self.host),