From 0f3d7673f36f150f38abed71cae3f250c1bee32b Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Mon, 31 Aug 2015 14:47:13 -0700 Subject: url-quote user-provided param before inserting in URL. --- docker/client.py | 85 ++++++++++++++++++++++++++-------------------------- docker/clientbase.py | 18 ++++++++--- 2 files changed, 57 insertions(+), 46 deletions(-) diff --git a/docker/client.py b/docker/client.py index b1f72e9..88bc50d 100644 --- a/docker/client.py +++ b/docker/client.py @@ -41,7 +41,7 @@ class Client(clientbase.ClientBase): 'stderr': stderr and 1 or 0, 'stream': stream and 1 or 0, } - u = self._url("/containers/{0}/attach".format(container)) + u = self._url("/containers/{0}/attach", container) response = self._post(u, params=params, stream=stream) return self._get_result(container, stream, response) @@ -58,7 +58,7 @@ class Client(clientbase.ClientBase): if ws: return self._attach_websocket(container, params) - u = self._url("/containers/{0}/attach".format(container)) + u = self._url("/containers/{0}/attach", container) return self._get_raw_response_socket(self.post( u, None, params=self._attach_params(params), stream=True)) @@ -275,8 +275,9 @@ class Client(clientbase.ClientBase): @check_resource def diff(self, container): - return self._result(self._get(self._url("/containers/{0}/changes". - format(container))), True) + return self._result( + self._get(self._url("/containers/{0}/changes", container)), True + ) def events(self, since=None, until=None, filters=None, decode=None): if isinstance(since, datetime): @@ -326,7 +327,7 @@ class Client(clientbase.ClientBase): 'Cmd': cmd } - url = self._url('/containers/{0}/exec'.format(container)) + url = self._url('/containers/{0}/exec', container) res = self._post_json(url, data=data) return self._result(res, True) @@ -337,7 +338,7 @@ class Client(clientbase.ClientBase): ) if isinstance(exec_id, dict): exec_id = exec_id.get('Id') - res = self._get(self._url("/exec/{0}/json".format(exec_id))) + res = self._get(self._url("/exec/{0}/json", exec_id)) return self._result(res, True) def exec_resize(self, exec_id, height=None, width=None): @@ -347,7 +348,7 @@ class Client(clientbase.ClientBase): exec_id = exec_id.get('Id') params = {'h': height, 'w': width} - url = self._url("/exec/{0}/resize".format(exec_id)) + url = self._url("/exec/{0}/resize", exec_id) res = self._post(url, params=params) self._raise_for_status(res) @@ -362,27 +363,28 @@ class Client(clientbase.ClientBase): 'Detach': detach } - res = self._post_json(self._url('/exec/{0}/start'.format(exec_id)), - data=data, stream=stream) + res = self._post_json( + self._url('/exec/{0}/start', exec_id), data=data, stream=stream + ) return self._get_result_tty(stream, res, tty) @check_resource def export(self, container): - res = self._get(self._url("/containers/{0}/export".format(container)), - stream=True) + res = self._get( + self._url("/containers/{0}/export", container), stream=True + ) self._raise_for_status(res) return res.raw @check_resource def get_image(self, image): - res = self._get(self._url("/images/{0}/get".format(image)), - stream=True) + res = self._get(self._url("/images/{0}/get", image), stream=True) self._raise_for_status(res) return res.raw @check_resource def history(self, image): - res = self._get(self._url("/images/{0}/history".format(image))) + res = self._get(self._url("/images/{0}/history", image)) return self._result(res, True) def images(self, name=None, quiet=False, all=False, viz=False, @@ -496,7 +498,7 @@ class Client(clientbase.ClientBase): raise errors.DeprecatedMethod( 'insert is not available for API version >=1.12' ) - api_url = self._url("/images/{0}/insert".format(image)) + api_url = self._url("/images/{0}/insert", image) params = { 'url': url, 'path': path @@ -506,21 +508,18 @@ class Client(clientbase.ClientBase): @check_resource def inspect_container(self, container): return self._result( - self._get(self._url("/containers/{0}/json".format(container))), - True) + self._get(self._url("/containers/{0}/json", container)), True + ) @check_resource def inspect_image(self, image): return self._result( - self._get( - self._url("/images/{0}/json".format(image.replace('/', '%2F'))) - ), - True + self._get(self._url("/images/{0}/json", image)), True ) @check_resource def kill(self, container, signal=None): - url = self._url("/containers/{0}/kill".format(container)) + url = self._url("/containers/{0}/kill", container) params = {} if signal is not None: params['signal'] = signal @@ -583,7 +582,7 @@ class Client(clientbase.ClientBase): if tail != 'all' and (not isinstance(tail, int) or tail <= 0): tail = 'all' params['tail'] = tail - url = self._url("/containers/{0}/logs".format(container)) + url = self._url("/containers/{0}/logs", container) res = self._get(url, params=params, stream=stream) return self._get_result(container, stream, res) return self.attach( @@ -596,7 +595,7 @@ class Client(clientbase.ClientBase): @check_resource def pause(self, container): - url = self._url('/containers/{0}/pause'.format(container)) + url = self._url('/containers/{0}/pause', container) res = self._post(url) self._raise_for_status(res) @@ -605,7 +604,7 @@ class Client(clientbase.ClientBase): @check_resource def port(self, container, private_port): - res = self._get(self._url("/containers/{0}/json".format(container))) + res = self._get(self._url("/containers/{0}/json", container)) self._raise_for_status(res) json_ = res.json() s_port = str(private_port) @@ -692,7 +691,7 @@ class Client(clientbase.ClientBase): if not tag: repository, tag = utils.parse_repository_tag(repository) registry, repo_name = auth.resolve_repository_name(repository) - u = self._url("/images/{0}/push".format(repository)) + u = self._url("/images/{0}/push", repository) params = { 'tag': tag } @@ -725,14 +724,15 @@ class Client(clientbase.ClientBase): @check_resource def remove_container(self, container, v=False, link=False, force=False): params = {'v': v, 'link': link, 'force': force} - res = self._delete(self._url("/containers/" + container), - params=params) + res = self._delete( + self._url("/containers/{0}", container), params=params + ) self._raise_for_status(res) @check_resource def remove_image(self, image, force=False, noprune=False): params = {'force': force, 'noprune': noprune} - res = self._delete(self._url("/images/" + image), params=params) + res = self._delete(self._url("/images/{0}", image), params=params) self._raise_for_status(res) @check_resource @@ -741,7 +741,7 @@ class Client(clientbase.ClientBase): raise errors.InvalidVersion( 'rename was only introduced in API version 1.17' ) - url = self._url("/containers/{0}/rename".format(container)) + url = self._url("/containers/{0}/rename", container) params = {'name': name} res = self._post(url, params=params) self._raise_for_status(res) @@ -749,21 +749,22 @@ class Client(clientbase.ClientBase): @check_resource def resize(self, container, height, width): params = {'h': height, 'w': width} - url = self._url("/containers/{0}/resize".format(container)) + url = self._url("/containers/{0}/resize", container) res = self._post(url, params=params) self._raise_for_status(res) @check_resource def restart(self, container, timeout=10): params = {'t': timeout} - url = self._url("/containers/{0}/restart".format(container)) + url = self._url("/containers/{0}/restart", container) res = self._post(url, params=params) self._raise_for_status(res) def search(self, term): - return self._result(self._get(self._url("/images/search"), - params={'term': term}), - True) + return self._result( + self._get(self._url("/images/search"), params={'term': term}), + True + ) @check_resource def start(self, container, binds=None, port_bindings=None, lxc_conf=None, @@ -829,7 +830,7 @@ class Client(clientbase.ClientBase): ) start_config = self.create_host_config(**start_config_kwargs) - url = self._url("/containers/{0}/start".format(container)) + url = self._url("/containers/{0}/start", container) res = self._post_json(url, data=start_config) self._raise_for_status(res) @@ -839,13 +840,13 @@ class Client(clientbase.ClientBase): raise errors.InvalidVersion( 'Stats retrieval is not supported in API < 1.17!') - url = self._url("/containers/{0}/stats".format(container)) + url = self._url("/containers/{0}/stats", container) return self._stream_helper(self._get(url, stream=True), decode=decode) @check_resource def stop(self, container, timeout=10): params = {'t': timeout} - url = self._url("/containers/{0}/stop".format(container)) + url = self._url("/containers/{0}/stop", container) res = self._post(url, params=params, timeout=(timeout + (self.timeout or 0))) @@ -858,14 +859,14 @@ class Client(clientbase.ClientBase): 'repo': repository, 'force': 1 if force else 0 } - url = self._url("/images/{0}/tag".format(image)) + url = self._url("/images/{0}/tag", image) res = self._post(url, params=params) self._raise_for_status(res) return res.status_code == 201 @check_resource def top(self, container): - u = self._url("/containers/{0}/top".format(container)) + u = self._url("/containers/{0}/top", container) return self._result(self._get(u), True) def version(self, api_version=True): @@ -874,13 +875,13 @@ class Client(clientbase.ClientBase): @check_resource def unpause(self, container): - url = self._url('/containers/{0}/unpause'.format(container)) + url = self._url('/containers/{0}/unpause', container) res = self._post(url) self._raise_for_status(res) @check_resource def wait(self, container, timeout=None): - url = self._url("/containers/{0}/wait".format(container)) + url = self._url("/containers/{0}/wait", container) res = self._post(url, timeout=timeout) self._raise_for_status(res) json_ = res.json() diff --git a/docker/clientbase.py b/docker/clientbase.py index ce52ffa..90dba63 100644 --- a/docker/clientbase.py +++ b/docker/clientbase.py @@ -88,11 +88,21 @@ class ClientBase(requests.Session): def _delete(self, url, **kwargs): return self.delete(url, **self._set_request_timeout(kwargs)) - def _url(self, path, versioned_api=True): + def _url(self, pathfmt, resource_id=None, versioned_api=True): + if resource_id and not isinstance(resource_id, six.string_types): + raise ValueError( + 'Expected a resource ID string but found {0} ({1}) ' + 'instead'.format(resource_id, type(resource_id)) + ) + elif resource_id: + resource_id = six.moves.urllib.parse.quote_plus(resource_id) + if versioned_api: - return '{0}/v{1}{2}'.format(self.base_url, self._version, path) + return '{0}/v{1}{2}'.format( + self.base_url, self._version, pathfmt.format(resource_id) + ) else: - return '{0}{1}'.format(self.base_url, path) + return '{0}{1}'.format(self.base_url, pathfmt.format(resource_id)) def _raise_for_status(self, response, explanation=None): """Raises stored :class:`APIError`, if one occurred.""" @@ -136,7 +146,7 @@ class ClientBase(requests.Session): @check_resource def _attach_websocket(self, container, params=None): - url = self._url("/containers/{0}/attach/ws".format(container)) + url = self._url("/containers/{0}/attach/ws", container) req = requests.Request("POST", url, params=self._attach_params(params)) full_url = req.prepare().url full_url = full_url.replace("http://", "ws://", 1) -- cgit v1.2.1 From 3d884f9a3c47d12dac7596b790638813580233ac Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Mon, 31 Aug 2015 15:01:40 -0700 Subject: Test URL construction --- tests/test.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test.py b/tests/test.py index 1bf8c55..9cf94a1 100644 --- a/tests/test.py +++ b/tests/test.py @@ -144,6 +144,28 @@ class DockerClientTest(Cleanup, base.BaseTestCase): 'Version parameter must be a string or None. Found float' ) + def test_url_valid_resource(self): + url = self.client._url('/hello/{0}/world', 'somename') + self.assertEqual( + url, '{0}{1}'.format(url_prefix, 'hello/somename/world') + ) + + url = self.client._url('/hello/{0}/world', '/some?name') + self.assertEqual( + url, '{0}{1}'.format(url_prefix, 'hello/%2Fsome%3Fname/world') + ) + + def test_url_invalid_resource(self): + with pytest.raises(ValueError): + self.client._url('/hello/{0}/world', ['sakuya', 'izayoi']) + + def test_url_no_resource(self): + url = self.client._url('/simple') + self.assertEqual(url, '{0}{1}'.format(url_prefix, 'simple')) + + url = self.client._url('/simple', None) + self.assertEqual(url, '{0}{1}'.format(url_prefix, 'simple')) + ######################### # INFORMATION TESTS # ######################### -- cgit v1.2.1