From 861f6ea3fac236780c850c7879224f7e84561cf5 Mon Sep 17 00:00:00 2001 From: Lars Hupfeldt Date: Sat, 23 Feb 2019 18:18:43 +0100 Subject: [PATCH] Improved handling of trailing and leading '/' when joinig parts of URLs --- jenkins_api.py | 3 --- rest_api_wrapper.py | 14 ++++++++++---- test/coverage_rc.tenjin | 6 ++++++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/jenkins_api.py b/jenkins_api.py index ac5fe0c..219bafb 100755 --- a/jenkins_api.py +++ b/jenkins_api.py @@ -355,9 +355,6 @@ def invoke(self, securitytoken, build_params, cause, description): # Make location relative parts = urllib.parse.urlsplit(response.headers['location']) location = urllib.parse.urlunsplit(['', ''] + list(parts[2:])) - # Quick hack to ensure no double '//', pendig cleanup - if location.startswith('/'): - location = location[1:] else: # This hack is not correct because Jenkins may shange scheme from https to http :( location = response.headers['location'][len(self.jenkins.direct_uri):] diff --git a/rest_api_wrapper.py b/rest_api_wrapper.py index 11b7656..aae4b05 100644 --- a/rest_api_wrapper.py +++ b/rest_api_wrapper.py @@ -1,4 +1,5 @@ import sys +import os.path from .api_base import AuthError, ClientError @@ -20,6 +21,10 @@ def encode(text, encoding): return text +def _join_url(start, end): + return os.path.join(start.rstrip('/'), end.lstrip('/')) + + class ResourceNotFound(Exception): pass @@ -61,7 +66,7 @@ def _check_response(response, good_responses=(200,)): def _get(self, url, params): import requests try: - return self._check_response(self.session.get(self.direct_uri + url, params=params)) + return self._check_response(self.session.get(_join_url(self.direct_uri, url), params=params)) except requests.ConnectionError as ex: raise ConnectionError(ex) @@ -69,10 +74,10 @@ def get_content(self, url, **params): return self._get(url, params=params).content def get_json(self, url="", **params): - return self._get(url + "/api/json", params=params).json() + return self._get(_join_url(url, "api/json"), params=params).json() def post(self, url, payload=None, headers=None, allow_redirects=False, **params): - response = self.session.post(self.direct_uri + url, headers=headers, data=payload, allow_redirects=allow_redirects, params=params) + response = self.session.post(_join_url(self.direct_uri, url), headers=headers, data=payload, allow_redirects=allow_redirects, params=params) return self._check_response(response, (200, 201)) def headers(self, allow_redirects=True): @@ -92,6 +97,7 @@ def deco(self, *args, **kwargs): class RestkitRestApi(object): + """Note: This is no longer supported or tested, but may still work.""" def __init__(self, direct_uri, username, password, **kwargs): super(RestkitRestApi, self).__init__() @@ -114,7 +120,7 @@ def get_content(self, url, **params): @_check_restkit_response def get_json(self, path="", headers=None, params_dict=None, **params): - response = self.session.get(path=path + "/api/json", headers=headers, params_dict=params_dict, **params) + response = self.session.get(path=_join_url(path, "api/json"), headers=headers, params_dict=params_dict, **params) return self.json.loads(response.body_string()) @_check_restkit_response diff --git a/test/coverage_rc.tenjin b/test/coverage_rc.tenjin index 70578e0..7c404b5 100644 --- a/test/coverage_rc.tenjin +++ b/test/coverage_rc.tenjin @@ -11,14 +11,20 @@ exclude_lines = # This is hopefully covered by a subprocess call, but it won't be collected if __name__ == "__main__": + # The alternative (original) rest api is no longer tested + def _check_restkit_response + class RestkitRestApi + # Parts of jenkins_api not used when hudson if self.jenkins.is_jenkins + if head_response.get\("X-Jenkins"\) # Parts of jenkins_api not used when jenkins else: # Hudson self.is_jenkins = False + if head_response.get\("X-Hudson"\)