Replace os.path.join() for URLs
Since os.path.join() is OS-dependent, it should not be used for creating URLs. This patch replaces the use of os.path.join() in nova/api/openstack with common.url_join(), which uses the more correct "/".join(), while preserving the behavior of removing duplicate slashes inside the URL and adding a trailing slash with an empty final element. It also adds some tests to ensure that the generate_href() method in nova/api/openstack/compute/views/versions.py works after the refactoring to use common.url_join() Closes-Bug: 1489627 Change-Id: I32948dd1fcf0839b34e446d9e4b08f9c39d17c8f
This commit is contained in:
parent
60c5c27980
commit
7a09f727f3
|
@ -16,7 +16,6 @@
|
||||||
import collections
|
import collections
|
||||||
import functools
|
import functools
|
||||||
import itertools
|
import itertools
|
||||||
import os
|
|
||||||
import re
|
import re
|
||||||
|
|
||||||
from oslo_config import cfg
|
from oslo_config import cfg
|
||||||
|
@ -299,8 +298,7 @@ def remove_trailing_version_from_href(href):
|
||||||
LOG.debug('href %s does not contain version' % href)
|
LOG.debug('href %s does not contain version' % href)
|
||||||
raise ValueError(_('href %s does not contain version') % href)
|
raise ValueError(_('href %s does not contain version') % href)
|
||||||
|
|
||||||
new_path = '/'.join(url_parts)
|
new_path = url_join(*url_parts)
|
||||||
|
|
||||||
parsed_url = list(parsed_url)
|
parsed_url = list(parsed_url)
|
||||||
parsed_url[2] = new_path
|
parsed_url[2] = new_path
|
||||||
return urlparse.urlunsplit(parsed_url)
|
return urlparse.urlunsplit(parsed_url)
|
||||||
|
@ -398,6 +396,21 @@ def check_snapshots_enabled(f):
|
||||||
return inner
|
return inner
|
||||||
|
|
||||||
|
|
||||||
|
def url_join(*parts):
|
||||||
|
"""Convenience method for joining parts of a URL
|
||||||
|
|
||||||
|
Any leading and trailing '/' characters are removed, and the parts joined
|
||||||
|
together with '/' as a separator. If last element of 'parts' is an empty
|
||||||
|
string, the returned URL will have a trailing slash.
|
||||||
|
"""
|
||||||
|
parts = parts or [""]
|
||||||
|
clean_parts = [part.strip("/") for part in parts if part]
|
||||||
|
if not parts[-1]:
|
||||||
|
# Empty last element should add a trailing slash
|
||||||
|
clean_parts.append("")
|
||||||
|
return "/".join(clean_parts)
|
||||||
|
|
||||||
|
|
||||||
class ViewBuilder(object):
|
class ViewBuilder(object):
|
||||||
"""Model API responses as dictionaries."""
|
"""Model API responses as dictionaries."""
|
||||||
|
|
||||||
|
@ -427,27 +440,27 @@ class ViewBuilder(object):
|
||||||
params = request.params.copy()
|
params = request.params.copy()
|
||||||
params["marker"] = identifier
|
params["marker"] = identifier
|
||||||
prefix = self._update_compute_link_prefix(request.application_url)
|
prefix = self._update_compute_link_prefix(request.application_url)
|
||||||
url = os.path.join(prefix,
|
url = url_join(prefix,
|
||||||
self._get_project_id(request),
|
self._get_project_id(request),
|
||||||
collection_name)
|
collection_name)
|
||||||
return "%s?%s" % (url, urlparse.urlencode(params))
|
return "%s?%s" % (url, urlparse.urlencode(params))
|
||||||
|
|
||||||
def _get_href_link(self, request, identifier, collection_name):
|
def _get_href_link(self, request, identifier, collection_name):
|
||||||
"""Return an href string pointing to this object."""
|
"""Return an href string pointing to this object."""
|
||||||
prefix = self._update_compute_link_prefix(request.application_url)
|
prefix = self._update_compute_link_prefix(request.application_url)
|
||||||
return os.path.join(prefix,
|
return url_join(prefix,
|
||||||
self._get_project_id(request),
|
self._get_project_id(request),
|
||||||
collection_name,
|
collection_name,
|
||||||
str(identifier))
|
str(identifier))
|
||||||
|
|
||||||
def _get_bookmark_link(self, request, identifier, collection_name):
|
def _get_bookmark_link(self, request, identifier, collection_name):
|
||||||
"""Create a URL that refers to a specific resource."""
|
"""Create a URL that refers to a specific resource."""
|
||||||
base_url = remove_trailing_version_from_href(request.application_url)
|
base_url = remove_trailing_version_from_href(request.application_url)
|
||||||
base_url = self._update_compute_link_prefix(base_url)
|
base_url = self._update_compute_link_prefix(base_url)
|
||||||
return os.path.join(base_url,
|
return url_join(base_url,
|
||||||
self._get_project_id(request),
|
self._get_project_id(request),
|
||||||
collection_name,
|
collection_name,
|
||||||
str(identifier))
|
str(identifier))
|
||||||
|
|
||||||
def _get_collection_links(self,
|
def _get_collection_links(self,
|
||||||
request,
|
request,
|
||||||
|
|
|
@ -13,8 +13,6 @@
|
||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
import os.path
|
|
||||||
|
|
||||||
import webob
|
import webob
|
||||||
|
|
||||||
from nova.api.openstack import common
|
from nova.api.openstack import common
|
||||||
|
@ -80,7 +78,8 @@ class CreateBackupController(wsgi.Controller):
|
||||||
# build location of newly-created image entity if rotation is not zero
|
# build location of newly-created image entity if rotation is not zero
|
||||||
if rotation > 0:
|
if rotation > 0:
|
||||||
image_id = str(image['id'])
|
image_id = str(image['id'])
|
||||||
image_ref = os.path.join(req.application_url, 'images', image_id)
|
image_ref = common.url_join(req.application_url, 'images',
|
||||||
|
image_id)
|
||||||
resp.headers['Location'] = image_ref
|
resp.headers['Location'] = image_ref
|
||||||
|
|
||||||
return resp
|
return resp
|
||||||
|
|
|
@ -12,7 +12,6 @@
|
||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
import os.path
|
|
||||||
import traceback
|
import traceback
|
||||||
|
|
||||||
from oslo_log import log as logging
|
from oslo_log import log as logging
|
||||||
|
@ -292,7 +291,8 @@ class AdminActionsController(wsgi.Controller):
|
||||||
# build location of newly-created image entity if rotation is not zero
|
# build location of newly-created image entity if rotation is not zero
|
||||||
if rotation > 0:
|
if rotation > 0:
|
||||||
image_id = str(image['id'])
|
image_id = str(image['id'])
|
||||||
image_ref = os.path.join(req.application_url, 'images', image_id)
|
image_ref = common.url_join(req.application_url, 'images',
|
||||||
|
image_id)
|
||||||
resp.headers['Location'] = image_ref
|
resp.headers['Location'] = image_ref
|
||||||
|
|
||||||
return resp
|
return resp
|
||||||
|
|
|
@ -15,7 +15,6 @@
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
import base64
|
import base64
|
||||||
import os
|
|
||||||
import re
|
import re
|
||||||
import sys
|
import sys
|
||||||
|
|
||||||
|
@ -1113,10 +1112,10 @@ class Controller(wsgi.Controller):
|
||||||
image_id = str(image['id'])
|
image_id = str(image['id'])
|
||||||
url_prefix = self._view_builder._update_glance_link_prefix(
|
url_prefix = self._view_builder._update_glance_link_prefix(
|
||||||
req.application_url)
|
req.application_url)
|
||||||
image_ref = os.path.join(url_prefix,
|
image_ref = common.url_join(url_prefix,
|
||||||
context.project_id,
|
context.project_id,
|
||||||
'images',
|
'images',
|
||||||
image_id)
|
image_id)
|
||||||
|
|
||||||
resp = webob.Response(status_int=202)
|
resp = webob.Response(status_int=202)
|
||||||
resp.headers['Location'] = image_ref
|
resp.headers['Location'] = image_ref
|
||||||
|
|
|
@ -14,7 +14,6 @@
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
import copy
|
import copy
|
||||||
import os
|
|
||||||
|
|
||||||
from nova.api.openstack import common
|
from nova.api.openstack import common
|
||||||
|
|
||||||
|
@ -92,8 +91,5 @@ class ViewBuilder(common.ViewBuilder):
|
||||||
else:
|
else:
|
||||||
version_number = 'v2'
|
version_number = 'v2'
|
||||||
|
|
||||||
if path:
|
path = path or ''
|
||||||
path = path.strip('/')
|
return common.url_join(self.prefix, version_number, path)
|
||||||
return os.path.join(self.prefix, version_number, path)
|
|
||||||
else:
|
|
||||||
return os.path.join(self.prefix, version_number) + '/'
|
|
||||||
|
|
|
@ -419,6 +419,22 @@ class VersionsViewBuilderTests(test.NoDBTestCase):
|
||||||
|
|
||||||
self.assertEqual(expected, actual)
|
self.assertEqual(expected, actual)
|
||||||
|
|
||||||
|
def test_generate_href_with_path(self):
|
||||||
|
path = "random/path"
|
||||||
|
base_url = "http://example.org/app/"
|
||||||
|
expected = "http://example.org/app/v2/%s" % path
|
||||||
|
builder = views.versions.ViewBuilder(base_url)
|
||||||
|
actual = builder.generate_href("v2", path)
|
||||||
|
self.assertEqual(actual, expected)
|
||||||
|
|
||||||
|
def test_generate_href_with_empty_path(self):
|
||||||
|
path = ""
|
||||||
|
base_url = "http://example.org/app/"
|
||||||
|
expected = "http://example.org/app/v2/"
|
||||||
|
builder = views.versions.ViewBuilder(base_url)
|
||||||
|
actual = builder.generate_href("v2", path)
|
||||||
|
self.assertEqual(actual, expected)
|
||||||
|
|
||||||
|
|
||||||
# NOTE(oomichi): Now version API of v2.0 covers "/"(root).
|
# NOTE(oomichi): Now version API of v2.0 covers "/"(root).
|
||||||
# So this class tests "/v2.1" only for v2.1 API.
|
# So this class tests "/v2.1" only for v2.1 API.
|
||||||
|
|
|
@ -579,6 +579,38 @@ class LinkPrefixTest(test.NoDBTestCase):
|
||||||
result)
|
result)
|
||||||
|
|
||||||
|
|
||||||
|
class UrlJoinTest(test.NoDBTestCase):
|
||||||
|
def test_url_join(self):
|
||||||
|
pieces = ["one", "two", "three"]
|
||||||
|
joined = common.url_join(*pieces)
|
||||||
|
self.assertEqual("one/two/three", joined)
|
||||||
|
|
||||||
|
def test_url_join_extra_slashes(self):
|
||||||
|
pieces = ["one/", "/two//", "/three/"]
|
||||||
|
joined = common.url_join(*pieces)
|
||||||
|
self.assertEqual("one/two/three", joined)
|
||||||
|
|
||||||
|
def test_url_join_trailing_slash(self):
|
||||||
|
pieces = ["one", "two", "three", ""]
|
||||||
|
joined = common.url_join(*pieces)
|
||||||
|
self.assertEqual("one/two/three/", joined)
|
||||||
|
|
||||||
|
def test_url_join_empty_list(self):
|
||||||
|
pieces = []
|
||||||
|
joined = common.url_join(*pieces)
|
||||||
|
self.assertEqual("", joined)
|
||||||
|
|
||||||
|
def test_url_join_single_empty_string(self):
|
||||||
|
pieces = [""]
|
||||||
|
joined = common.url_join(*pieces)
|
||||||
|
self.assertEqual("", joined)
|
||||||
|
|
||||||
|
def test_url_join_single_slash(self):
|
||||||
|
pieces = ["/"]
|
||||||
|
joined = common.url_join(*pieces)
|
||||||
|
self.assertEqual("", joined)
|
||||||
|
|
||||||
|
|
||||||
class ViewBuilderLinkTest(test.NoDBTestCase):
|
class ViewBuilderLinkTest(test.NoDBTestCase):
|
||||||
project_id = "fake"
|
project_id = "fake"
|
||||||
api_version = "2.1"
|
api_version = "2.1"
|
||||||
|
|
Loading…
Reference in New Issue