From 2e74ed79236040fbdfb14c76e5e7684888b415e3 Mon Sep 17 00:00:00 2001 From: Sarah Bird Date: Wed, 15 Mar 2017 17:39:56 -0500 Subject: [PATCH 1/5] Factor out adding headers, and update test Have also confirmed that this test would fail on original implementation of add_header that was fixed in 55ed929 --- .../webdriver/remote/remote_connection.py | 28 +++++++++++---- .../remote/test_remote_connection.py | 36 +++++++++++-------- 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/py/selenium/webdriver/remote/remote_connection.py b/py/selenium/webdriver/remote/remote_connection.py index f66d343da5932..aa03ce45e0d23 100644 --- a/py/selenium/webdriver/remote/remote_connection.py +++ b/py/selenium/webdriver/remote/remote_connection.py @@ -414,6 +414,27 @@ def execute(self, command, params): url = '%s%s' % (self._url, path) return self._request(command_info[0], url, body=data) + @classmethod + def _add_request_headers(cls, request, parsed_url): + """ + Helper function to add appropriate headers to remote request. + + :Args: + - request - the remote request being created + - parsed_url - the parsed url + + :Returns: + Request with headers added + """ + request.add_header('Accept', 'application/json') + request.add_header('Content-Type', 'application/json;charset=UTF-8') + + if parsed_url.username: + base64string = base64.b64encode('{0.username}:{0.password}'.format(parsed_url).encode()) + request.add_header('Authorization', 'Basic {}'.format(base64string.decode())) + + return request + def _request(self, method, url, body=None): """ Send an HTTP request to the remote server. @@ -472,12 +493,7 @@ def _request(self, method, url, body=None): else: request = Request(url, data=body.encode('utf-8'), method=method) - request.add_header('Accept', 'application/json') - request.add_header('Content-Type', 'application/json;charset=UTF-8') - - if parsed_url.username: - base64string = base64.b64encode('{0.username}:{0.password}'.format(parsed_url).encode()) - request.add_header('Authorization', 'Basic {}'.format(base64string).decode()) + request = self._add_request_headers(request, parsed_url) if password_manager: opener = url_request.build_opener(url_request.HTTPRedirectHandler(), diff --git a/py/test/unit/selenium/webdriver/remote/test_remote_connection.py b/py/test/unit/selenium/webdriver/remote/test_remote_connection.py index 9d5bbb54dcfdc..37eebda14720d 100644 --- a/py/test/unit/selenium/webdriver/remote/test_remote_connection.py +++ b/py/test/unit/selenium/webdriver/remote/test_remote_connection.py @@ -15,21 +15,29 @@ # specific language governing permissions and limitations # under the License. -import pytest +try: + from urllib import parse +except ImportError: # above is available in py3+, below is py2.7 + import urlparse as parse -from selenium.webdriver.remote.remote_connection import RemoteConnection +from selenium.webdriver.remote.remote_connection import ( + RemoteConnection, + Request +) -def test_basic_auth(mocker): - def check(request, timeout): - assert request.headers['Authorization'] == 'Basic dXNlcjpwYXNz' +def test_addition_of_auth_headers(mocker): + url = 'http://user:pass@remote' + parsed_url = parse.urlparse(url) + cleaned_url = parse.urlunparse(( + parsed_url.scheme, + parsed_url.hostname, + parsed_url.path, + parsed_url.params, + parsed_url.query, + parsed_url.fragment) + ) + request = Request(cleaned_url) + RemoteConnection._add_request_headers(request, parsed_url) + assert request.headers['Authorization'] == 'Basic dXNlcjpwYXNz' - try: - method = mocker.patch('urllib.request.OpenerDirector.open') - except ImportError: - method = mocker.patch('urllib2.OpenerDirector.open') - method.side_effect = check - - with pytest.raises(AttributeError): - RemoteConnection('http://user:pass@remote', resolve_ip=False) \ - .execute('status', {}) From 206be5283d6addf8a990756c587b8baab7014872 Mon Sep 17 00:00:00 2001 From: Sarah Bird Date: Wed, 15 Mar 2017 18:41:14 -0500 Subject: [PATCH 2/5] Refactor method onto remote_connection.Request Test method seperately, and then test that RemoteConnection calls it. --- .../webdriver/remote/remote_connection.py | 39 ++++++++----------- .../remote/test_remote_connection.py | 34 +++++++++++++++- 2 files changed, 49 insertions(+), 24 deletions(-) diff --git a/py/selenium/webdriver/remote/remote_connection.py b/py/selenium/webdriver/remote/remote_connection.py index aa03ce45e0d23..7e4915fbd15c5 100644 --- a/py/selenium/webdriver/remote/remote_connection.py +++ b/py/selenium/webdriver/remote/remote_connection.py @@ -63,6 +63,22 @@ def get_method(self): """ return self._method + def add_remote_connection_headers(self, parsed_url): + """ + Helper function to add appropriate headers to remote request. + + :Args: + - parsed_url - the parsed url + """ + self.add_header('Accept', 'application/json') + self.add_header('Content-Type', 'application/json;charset=UTF-8') + + if parsed_url.username: + base64string = base64.b64encode('{0.username}:{0.password}'.format(parsed_url).encode()) + self.add_header('Authorization', 'Basic {}'.format(base64string.decode())) + + return self.headers + class Response(object): """ @@ -414,27 +430,6 @@ def execute(self, command, params): url = '%s%s' % (self._url, path) return self._request(command_info[0], url, body=data) - @classmethod - def _add_request_headers(cls, request, parsed_url): - """ - Helper function to add appropriate headers to remote request. - - :Args: - - request - the remote request being created - - parsed_url - the parsed url - - :Returns: - Request with headers added - """ - request.add_header('Accept', 'application/json') - request.add_header('Content-Type', 'application/json;charset=UTF-8') - - if parsed_url.username: - base64string = base64.b64encode('{0.username}:{0.password}'.format(parsed_url).encode()) - request.add_header('Authorization', 'Basic {}'.format(base64string.decode())) - - return request - def _request(self, method, url, body=None): """ Send an HTTP request to the remote server. @@ -493,7 +488,7 @@ def _request(self, method, url, body=None): else: request = Request(url, data=body.encode('utf-8'), method=method) - request = self._add_request_headers(request, parsed_url) + request.add_remote_connection_headers(parsed_url) if password_manager: opener = url_request.build_opener(url_request.HTTPRedirectHandler(), diff --git a/py/test/unit/selenium/webdriver/remote/test_remote_connection.py b/py/test/unit/selenium/webdriver/remote/test_remote_connection.py index 37eebda14720d..a8818a8f6539f 100644 --- a/py/test/unit/selenium/webdriver/remote/test_remote_connection.py +++ b/py/test/unit/selenium/webdriver/remote/test_remote_connection.py @@ -26,7 +26,7 @@ ) -def test_addition_of_auth_headers(mocker): +def test_add_remote_connection_headers_adds_auth_header(): url = 'http://user:pass@remote' parsed_url = parse.urlparse(url) cleaned_url = parse.urlunparse(( @@ -38,6 +38,36 @@ def test_addition_of_auth_headers(mocker): parsed_url.fragment) ) request = Request(cleaned_url) - RemoteConnection._add_request_headers(request, parsed_url) + request.add_remote_connection_headers(parsed_url) assert request.headers['Authorization'] == 'Basic dXNlcjpwYXNz' + +class MockResponse: + code = 200 + headers = [] + def read(self): + return b"{}" + def close(self): + pass + def getheader(self, *args, **kwargs): + pass + + +def test_remote_connection_calls_add_remote_connection_headers(mocker): + # Stub out response + try: + mock_open = mocker.patch('urllib.request.OpenerDirector.open') + except ImportError: + mock_open = mocker.patch('urllib2.OpenerDirector.open') + mock_open.return_value = MockResponse() + + # Add mock to test that RemoteConnection._request calls + # add_remote_connection_headers method. + mock = mocker.patch( + 'selenium.webdriver.remote.remote_connection.Request.add_remote_connection_headers' + ) + url = 'http://user:pass@remote' + command = 'status' + RemoteConnection(url, resolve_ip=False).execute(command, {}) + expected_url = parse.urlparse("%s/%s" % (url, command)) + mock.assert_called_once_with(expected_url) From 2a1523924a041456fed94cab5b9412f7c2c3b082 Mon Sep 17 00:00:00 2001 From: Sarah Bird Date: Fri, 17 Mar 2017 13:48:55 -0500 Subject: [PATCH 3/5] No need to use cleaned_url when building Request --- .../webdriver/remote/test_remote_connection.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/py/test/unit/selenium/webdriver/remote/test_remote_connection.py b/py/test/unit/selenium/webdriver/remote/test_remote_connection.py index a8818a8f6539f..9a7240af39823 100644 --- a/py/test/unit/selenium/webdriver/remote/test_remote_connection.py +++ b/py/test/unit/selenium/webdriver/remote/test_remote_connection.py @@ -28,17 +28,8 @@ def test_add_remote_connection_headers_adds_auth_header(): url = 'http://user:pass@remote' - parsed_url = parse.urlparse(url) - cleaned_url = parse.urlunparse(( - parsed_url.scheme, - parsed_url.hostname, - parsed_url.path, - parsed_url.params, - parsed_url.query, - parsed_url.fragment) - ) - request = Request(cleaned_url) - request.add_remote_connection_headers(parsed_url) + request = Request(url) + request.add_remote_connection_headers(parse.urlparse(url)) assert request.headers['Authorization'] == 'Basic dXNlcjpwYXNz' From 791921ba5bf906978105046b13b019ad0c4516fd Mon Sep 17 00:00:00 2001 From: Sarah Bird Date: Fri, 17 Mar 2017 16:44:17 -0500 Subject: [PATCH 4/5] Refactor all headers out into method, and test all --- .../webdriver/remote/remote_connection.py | 56 ++++++++++--------- .../remote/test_remote_connection.py | 55 ++++++++++++------ 2 files changed, 69 insertions(+), 42 deletions(-) diff --git a/py/selenium/webdriver/remote/remote_connection.py b/py/selenium/webdriver/remote/remote_connection.py index 7e4915fbd15c5..ddaf8b59fc828 100644 --- a/py/selenium/webdriver/remote/remote_connection.py +++ b/py/selenium/webdriver/remote/remote_connection.py @@ -63,21 +63,6 @@ def get_method(self): """ return self._method - def add_remote_connection_headers(self, parsed_url): - """ - Helper function to add appropriate headers to remote request. - - :Args: - - parsed_url - the parsed url - """ - self.add_header('Accept', 'application/json') - self.add_header('Content-Type', 'application/json;charset=UTF-8') - - if parsed_url.username: - base64string = base64.b64encode('{0.username}:{0.password}'.format(parsed_url).encode()) - self.add_header('Authorization', 'Basic {}'.format(base64string.decode())) - - return self.headers class Response(object): @@ -178,6 +163,35 @@ def reset_timeout(cls): """ cls._timeout = socket._GLOBAL_DEFAULT_TIMEOUT + @classmethod + def get_remote_connection_headers(cls, parsed_url, keep_alive=False): + """ + Get headers for remote request. + + :Args: + - parsed_url - The parsed url + - keep_alive (Boolean) - Is this a keep-alive connection (default: False) + """ + + headers = { + 'Accept': 'application/json', + 'Content-Type': 'application/json;charset=UTF-8', + 'User-Agent': 'Python http auth' + } + + if parsed_url.username: + base64string = base64.b64encode('{0.username}:{0.password}'.format(parsed_url).encode()) + headers.update({ + 'Authorization': 'Basic {}'.format(base64string.decode()) + }) + + if keep_alive: + headers.update({ + 'Connection': 'keep-alive' + }) + + return headers + def __init__(self, remote_server_addr, keep_alive=False, resolve_ip=True): # Attempt to resolve the hostname and get an IP address. self.keep_alive = keep_alive @@ -445,17 +459,9 @@ def _request(self, method, url, body=None): LOGGER.debug('%s %s %s' % (method, url, body)) parsed_url = parse.urlparse(url) + headers = self.get_remote_connection_headers(parsed_url, self.keep_alive) if self.keep_alive: - headers = {"Connection": 'keep-alive', method: parsed_url.path, - "User-Agent": "Python http auth", - "Content-type": "application/json;charset=\"UTF-8\"", - "Accept": "application/json"} - if parsed_url.username: - auth = base64.standard_b64encode(('%s:%s' % ( - parsed_url.username, - parsed_url.password)).encode('ascii')).decode('ascii').replace('\n', '') - headers["Authorization"] = "Basic %s" % auth if body and method != 'POST' and method != 'PUT': body = None try: @@ -488,7 +494,7 @@ def _request(self, method, url, body=None): else: request = Request(url, data=body.encode('utf-8'), method=method) - request.add_remote_connection_headers(parsed_url) + request.headers.update(headers) if password_manager: opener = url_request.build_opener(url_request.HTTPRedirectHandler(), diff --git a/py/test/unit/selenium/webdriver/remote/test_remote_connection.py b/py/test/unit/selenium/webdriver/remote/test_remote_connection.py index 9a7240af39823..34c03e361e915 100644 --- a/py/test/unit/selenium/webdriver/remote/test_remote_connection.py +++ b/py/test/unit/selenium/webdriver/remote/test_remote_connection.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. + try: from urllib import parse except ImportError: # above is available in py3+, below is py2.7 @@ -22,43 +23,63 @@ from selenium.webdriver.remote.remote_connection import ( RemoteConnection, - Request ) -def test_add_remote_connection_headers_adds_auth_header(): +def test_get_remote_connection_headers_defaults(): + url = 'http://remote' + headers = RemoteConnection.get_remote_connection_headers(parse.urlparse(url)) + assert 'Authorization' not in headers.keys() + assert 'Connection' not in headers.keys() + assert headers.get('Accept') == 'application/json' + assert headers.get('Content-Type') == 'application/json;charset=UTF-8' + assert headers.get('User-Agent') == 'Python http auth' + + +def test_get_remote_connection_headers_adds_auth_header_if_pass(): url = 'http://user:pass@remote' - request = Request(url) - request.add_remote_connection_headers(parse.urlparse(url)) - assert request.headers['Authorization'] == 'Basic dXNlcjpwYXNz' + headers = RemoteConnection.get_remote_connection_headers(parse.urlparse(url)) + assert headers.get('Authorization') == 'Basic dXNlcjpwYXNz' + + +def test_get_remote_connection_headers_adds_keep_alive_if_requested(): + url = 'http://remote' + headers = RemoteConnection.get_remote_connection_headers(parse.urlparse(url), keep_alive=True) + assert headers.get('Connection') == 'keep-alive' class MockResponse: code = 200 headers = [] + def read(self): return b"{}" + def close(self): pass + def getheader(self, *args, **kwargs): pass -def test_remote_connection_calls_add_remote_connection_headers(mocker): +def test_remote_connection_adds_connection_headers_from_get_remote_connection_headers(mocker): + test_headers = {'FOO': 'bar'} + + # Stub out the get_remote_connection_headers method to return something testable + mocker.patch( + 'selenium.webdriver.remote.remote_connection.RemoteConnection.get_remote_connection_headers' + ).return_value = test_headers + # Stub out response try: mock_open = mocker.patch('urllib.request.OpenerDirector.open') except ImportError: mock_open = mocker.patch('urllib2.OpenerDirector.open') - mock_open.return_value = MockResponse() - # Add mock to test that RemoteConnection._request calls - # add_remote_connection_headers method. - mock = mocker.patch( - 'selenium.webdriver.remote.remote_connection.Request.add_remote_connection_headers' - ) - url = 'http://user:pass@remote' - command = 'status' - RemoteConnection(url, resolve_ip=False).execute(command, {}) - expected_url = parse.urlparse("%s/%s" % (url, command)) - mock.assert_called_once_with(expected_url) + def assert_header_added(request, timeout): + assert request.headers == test_headers + return MockResponse() + + mock_open.side_effect = assert_header_added + + RemoteConnection('http://remote', resolve_ip=False).execute('status', {}) From b15b6531c1f84a4c1cca2031f272b02cef61f721 Mon Sep 17 00:00:00 2001 From: Sarah Bird Date: Fri, 17 Mar 2017 16:45:36 -0500 Subject: [PATCH 5/5] Linting --- py/selenium/webdriver/remote/remote_connection.py | 1 - .../unit/selenium/webdriver/remote/test_remote_connection.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/py/selenium/webdriver/remote/remote_connection.py b/py/selenium/webdriver/remote/remote_connection.py index ddaf8b59fc828..a37a6174d8711 100644 --- a/py/selenium/webdriver/remote/remote_connection.py +++ b/py/selenium/webdriver/remote/remote_connection.py @@ -64,7 +64,6 @@ def get_method(self): return self._method - class Response(object): """ Represents an HTTP response. diff --git a/py/test/unit/selenium/webdriver/remote/test_remote_connection.py b/py/test/unit/selenium/webdriver/remote/test_remote_connection.py index 34c03e361e915..ddf30da858064 100644 --- a/py/test/unit/selenium/webdriver/remote/test_remote_connection.py +++ b/py/test/unit/selenium/webdriver/remote/test_remote_connection.py @@ -77,7 +77,7 @@ def test_remote_connection_adds_connection_headers_from_get_remote_connection_he mock_open = mocker.patch('urllib2.OpenerDirector.open') def assert_header_added(request, timeout): - assert request.headers == test_headers + assert request.headers == test_headers return MockResponse() mock_open.side_effect = assert_header_added