From 5ccef9cd06978f303b77de0feba19e4e85d5ec27 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 12 May 2020 14:57:14 +0200 Subject: [PATCH] Treat endpoints with trailing slashes the same way as without them We've been historically using endpoints without trailing slashes in our API. Apparently, some libraries (like gophercloud) are quite opinionated about it (see the story), so let's handle both. The implementation could be simpler if we just added trailing slash to all routes, but it would cause redirects for current users. Change-Id: Icbd971a8e792f93f9c3fa66ba29bec055dcdee32 Story: #2007660 Task: #39749 --- ironic_inspector/main.py | 18 ++++++++++++------ ironic_inspector/test/unit/test_main.py | 10 ++++++++-- .../trailing-slashes-93c2466b71829ec1.yaml | 5 +++++ 3 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/trailing-slashes-93c2466b71829ec1.yaml diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index d6dad4263..8862e4eaa 100644 --- a/ironic_inspector/main.py +++ b/ironic_inspector/main.py @@ -172,8 +172,9 @@ def add_version_headers(res): def create_link_object(urls): links = [] for url in urls: - links.append({"rel": "self", - "href": os.path.join(flask.request.url_root, url)}) + links.append({ + "rel": "self", + "href": os.path.join(flask.request.url_root, url).rstrip('/')}) return links @@ -181,7 +182,7 @@ def generate_resource_data(resources): data = [] for resource in resources: item = {} - item['name'] = str(resource).split('/')[-1] + item['name'] = str(resource).rstrip('/').split('/')[-1] item['links'] = create_link_object([str(resource)[1:]]) data.append(item) return data @@ -226,6 +227,11 @@ def api(path, is_public_api=False, rule=None, verb_to_rule_map=None, and strings to format the 'rule' string with :param kwargs: all the rest kwargs are passed to flask app.route """ + # Force uniform behavior with regards to trailing slashes + if not path.endswith('/'): + path = path + '/' + flask_kwargs['strict_slashes'] = False + def outer(func): @_app.route(path, **flask_kwargs) @convert_exceptions @@ -264,7 +270,7 @@ def api_root(): @api('/', rule='introspection:version', is_public_api=True, methods=['GET']) def version_root(version): - pat = re.compile(r'^\/%s\/[^\/]*?$' % version) + pat = re.compile(r'^\/%s\/[^\/]*?/?$' % version) resources = [] for url in _app.url_map.iter_rules(): @@ -272,7 +278,7 @@ def version_root(version): resources.append(url) if not resources: - raise utils.Error(_('Version not found.'), code=404) + raise utils.Error(_('Version %s not found.') % version, code=404) return flask.jsonify(resources=generate_resource_data(resources)) @@ -392,7 +398,7 @@ def api_introspection_reapply(node_id): def rule_repr(rule, short): result = rule.as_dict(short=short) result['links'] = [{ - 'href': flask.url_for('api_rule', uuid=result['uuid']), + 'href': flask.url_for('api_rule', uuid=result['uuid']).rstrip('/'), 'rel': 'self' }] return result diff --git a/ironic_inspector/test/unit/test_main.py b/ironic_inspector/test/unit/test_main.py index dc4643314..23dde5912 100644 --- a/ironic_inspector/test/unit/test_main.py +++ b/ironic_inspector/test/unit/test_main.py @@ -582,8 +582,8 @@ class TestApiVersions(BaseAPITest): @mock.patch.object(main._app.url_map, "iter_rules", autospec=True) def test_version_endpoint(self, mock_rules): mock_rules.return_value = ["/v1/endpoint1", "/v1/endpoint2/", - "/v1/endpoint1/", - "/v2/endpoint1", "/v1/endpoint3", + "/v1/endpoint1//", + "/v2/endpoint1", "/v1/endpoint3/", "/v1/endpoint2//subpoint"] endpoint = "/v1" res = self.app.get(endpoint) @@ -606,6 +606,12 @@ class TestApiVersions(BaseAPITest): ]} self.assertEqual(expected, json_data) + def test_version_endpoint_with_slash(self): + endpoint = "/v1/" + res = self.app.get(endpoint) + self.assertEqual(200, res.status_code) + self._check_version_present(res) + def test_version_endpoint_invalid(self): endpoint = "/v-1" res = self.app.get(endpoint) diff --git a/releasenotes/notes/trailing-slashes-93c2466b71829ec1.yaml b/releasenotes/notes/trailing-slashes-93c2466b71829ec1.yaml new file mode 100644 index 000000000..dee47ad25 --- /dev/null +++ b/releasenotes/notes/trailing-slashes-93c2466b71829ec1.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes accessing API endpoints with trailing slashes. Now they're treated + the same way as without slashes, although the latter remain canonical URLs.