Remove the urlquote to arguments passed to reverse

In the container dashboard, I removed the urlquote to all
arguments passed to the reverse function since the reverse
function runs the urlquote on the arguments.

Change-Id: I4186ded9a2f52b6e56a12e8c521cb33fb3633a48
Closes-Bug: 1347734
Co-Authored-By: Masco Kaliyamoorthy <Masco.kaliyamoorthy@enovance.com>
This commit is contained in:
George Peristerakis 2014-07-23 10:35:42 -04:00
parent 19a89053a9
commit 36ef6d80c9
4 changed files with 109 additions and 83 deletions

View File

@ -0,0 +1,30 @@
# Copyright 2012 Nebula, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import six
from django.core.urlresolvers import reverse as django_reverse
from django.utils.http import urlquote # noqa
from django import VERSION # noqa
def reverse(viewname, urlconf=None, args=None, kwargs=None, prefix=None,
current_app=None):
if VERSION < (1, 6):
if args:
args = [urlquote(x) for x in args]
if kwargs:
kwargs = dict([(x, urlquote(y)) for x, y in six.iteritems(kwargs)])
return django_reverse(viewname, urlconf, args, kwargs, prefix,
current_app)

View File

@ -13,7 +13,6 @@
# under the License.
import logging
from django.core.urlresolvers import reverse
from django import shortcuts
from django import template
from django.template import defaultfilters as filters
@ -24,6 +23,7 @@ from django.utils.translation import ugettext_lazy as _
from horizon import exceptions
from horizon import messages
from horizon import tables
from horizon.utils.urlresolvers import reverse # noqa
from openstack_dashboard import api
from openstack_dashboard.api import swift
@ -47,8 +47,7 @@ class ViewContainer(tables.LinkAction):
def get_link_url(self, datum=None):
obj_id = self.table.get_object_id(datum)
args = (http.urlquote(obj_id),)
return reverse(self.url, args=args)
return reverse(self.url, args=(obj_id,))
class MakePublicContainer(tables.Action):
@ -164,8 +163,7 @@ class CreatePseudoFolder(tables.LinkAction):
else:
container_name = self.table.kwargs['container_name']
subfolders = self.table.kwargs.get('subfolder_path', '')
args = (http.urlquote(bit) for bit in
(container_name, subfolders) if bit)
args = (bit for bit in (container_name, subfolders) if bit)
return reverse(self.url, args=args)
def allowed(self, request, datum=None):
@ -190,13 +188,12 @@ class UploadObject(tables.LinkAction):
# Usable for both the container and object tables
if getattr(datum, 'container', datum):
# This is a container
container_name = http.urlquote(datum.name)
container_name = datum.name
else:
# This is a table action, and we already have the container name
container_name = self.table.kwargs['container_name']
subfolders = self.table.kwargs.get('subfolder_path', '')
args = (http.urlquote(bit) for bit in
(container_name, subfolders) if bit)
args = (bit for bit in (container_name, subfolders) if bit)
return reverse(self.url, args=args)
def allowed(self, request, datum=None):
@ -216,7 +213,7 @@ def get_size_used(container):
def get_container_link(container):
return reverse("horizon:project:containers:index",
args=(http.urlquote(wrap_delimiter(container.name)),))
args=(wrap_delimiter(container.name),))
class ContainerAjaxUpdateRow(tables.Row):
@ -298,8 +295,7 @@ class ViewObject(tables.LinkAction):
def get_link_url(self, obj):
container_name = self.table.kwargs['container_name']
return reverse(self.url, args=(http.urlquote(container_name),
http.urlquote(obj.name)))
return reverse(self.url, args=(container_name, obj.name))
class UpdateObject(tables.LinkAction):
@ -312,8 +308,7 @@ class UpdateObject(tables.LinkAction):
def get_link_url(self, obj):
container_name = self.table.kwargs['container_name']
return reverse(self.url, args=(http.urlquote(container_name),
http.urlquote(obj.name)))
return reverse(self.url, args=(container_name, obj.name))
class DeleteObject(tables.DeleteAction):
@ -349,8 +344,7 @@ class CopyObject(tables.LinkAction):
def get_link_url(self, obj):
container_name = self.table.kwargs['container_name']
return reverse(self.url, args=(http.urlquote(container_name),
http.urlquote(obj.name)))
return reverse(self.url, args=(container_name, obj.name))
class DownloadObject(tables.LinkAction):
@ -362,8 +356,7 @@ class DownloadObject(tables.LinkAction):
def get_link_url(self, obj):
container_name = self.table.kwargs['container_name']
return reverse(self.url, args=(http.urlquote(container_name),
http.urlquote(obj.name)))
return reverse(self.url, args=(container_name, obj.name))
def allowed(self, request, object):
return object.bytes and object.bytes > 0
@ -410,8 +403,8 @@ def get_size(obj):
def get_link_subfolder(subfolder):
container_name = subfolder.container_name
return reverse("horizon:project:containers:index",
args=(http.urlquote(wrap_delimiter(container_name)),
http.urlquote(wrap_delimiter(subfolder.name))))
args=(wrap_delimiter(container_name),
wrap_delimiter(subfolder.name)))
class ObjectsTable(tables.DataTable):

View File

@ -19,7 +19,6 @@
import tempfile
from django.core.files.uploadedfile import InMemoryUploadedFile # noqa
from django.core.urlresolvers import reverse
from django import http
from django.utils import http as utils_http
@ -32,16 +31,37 @@ from openstack_dashboard.dashboards.project.containers import views
from openstack_dashboard.test import helpers as test
from horizon import exceptions
from horizon.utils.urlresolvers import reverse # noqa
CONTAINER_NAME_1 = u"container one%\u6346"
CONTAINER_NAME_2 = u"container_two\u6346"
CONTAINER_NAME_1_QUOTED = utils_http.urlquote(CONTAINER_NAME_1)
CONTAINER_NAME_2_QUOTED = utils_http.urlquote(CONTAINER_NAME_2)
INVALID_CONTAINER_NAME_1 = utils_http.urlquote(CONTAINER_NAME_1_QUOTED)
INVALID_CONTAINER_NAME_2 = utils_http.urlquote(CONTAINER_NAME_2_QUOTED)
CONTAINER_INDEX_URL = reverse('horizon:project:containers:index')
INVALID_PATHS = []
def invalid_paths():
if not INVALID_PATHS:
for x in (CONTAINER_NAME_1_QUOTED, CONTAINER_NAME_2_QUOTED):
y = reverse('horizon:project:containers:index',
args=(tables.wrap_delimiter(x), ))
INVALID_PATHS.append(y)
for x in (CONTAINER_NAME_1, CONTAINER_NAME_2):
INVALID_PATHS.append(CONTAINER_INDEX_URL + x)
return INVALID_PATHS
class SwiftTests(test.TestCase):
def _test_invalid_paths(self, response):
for x in invalid_paths():
self.assertNotContains(response, x)
@test.create_stubs({api.swift: ('swift_get_containers',)})
def test_index_no_container_selected(self):
containers = self.containers.list()
@ -114,8 +134,7 @@ class SwiftTests(test.TestCase):
'method': forms.CreateContainer.__name__}
res = self.client.post(
reverse('horizon:project:containers:create'), formData)
args = (utils_http.urlquote(tables.wrap_delimiter(
container.name)),)
args = (tables.wrap_delimiter(container.name),)
url = reverse('horizon:project:containers:index', args=args)
self.assertRedirectsNoFollow(res, url)
@ -174,6 +193,7 @@ class SwiftTests(test.TestCase):
form_action = ' action="%s%s/" ' % (CONTAINER_INDEX_URL,
CONTAINER_NAME_1_QUOTED)
self.assertContains(res, form_action, count=2)
self._test_invalid_paths(res)
@test.create_stubs({api.swift: ('swift_upload_object',)})
def test_upload(self):
@ -197,9 +217,8 @@ class SwiftTests(test.TestCase):
res = self.client.get(upload_url)
self.assertTemplateUsed(res, 'project/containers/upload.html')
res = self.client.get(upload_url)
self.assertContains(res, 'enctype="multipart/form-data"')
self._test_invalid_paths(res)
formData = {'method': forms.UploadObject.__name__,
'container_name': container.name,
@ -207,7 +226,7 @@ class SwiftTests(test.TestCase):
'object_file': temp_file}
res = self.client.post(upload_url, formData)
args = (utils_http.urlquote(tables.wrap_delimiter(container.name)),)
args = (tables.wrap_delimiter(container.name),)
index_url = reverse('horizon:project:containers:index', args=args)
self.assertRedirectsNoFollow(res, index_url)
@ -230,6 +249,8 @@ class SwiftTests(test.TestCase):
res = self.client.get(upload_url)
self.assertContains(res, 'enctype="multipart/form-data"')
self.assertNotContains(res, INVALID_CONTAINER_NAME_1)
self.assertNotContains(res, INVALID_CONTAINER_NAME_2)
formData = {'method': forms.UploadObject.__name__,
'container_name': container.name,
@ -237,7 +258,7 @@ class SwiftTests(test.TestCase):
'object_file': None}
res = self.client.post(upload_url, formData)
args = (utils_http.urlquote(tables.wrap_delimiter(container.name)),)
args = (tables.wrap_delimiter(container.name),)
index_url = reverse('horizon:project:containers:index', args=args)
self.assertRedirectsNoFollow(res, index_url)
@ -258,6 +279,7 @@ class SwiftTests(test.TestCase):
res = self.client.get(create_pseudo_folder_url)
self.assertTemplateUsed(res,
'project/containers/create_pseudo_folder.html')
self._test_invalid_paths(res)
formData = {'method': forms.CreatePseudoFolder.__name__,
'container_name': container.name,
@ -273,7 +295,7 @@ class SwiftTests(test.TestCase):
def test_delete(self):
container = self.containers.first()
obj = self.objects.first()
args = (utils_http.urlquote(tables.wrap_delimiter(container.name)),)
args = (tables.wrap_delimiter(container.name),)
index_url = reverse('horizon:project:containers:index', args=args)
api.swift.swift_delete_object(IsA(http.HttpRequest),
container.name,
@ -292,7 +314,7 @@ class SwiftTests(test.TestCase):
def test_delete_pseudo_folder(self):
container = self.containers.first()
folder = self.folder.first()
args = (utils_http.urlquote(tables.wrap_delimiter(container.name)),)
args = (tables.wrap_delimiter(container.name),)
index_url = reverse('horizon:project:containers:index', args=args)
api.swift.swift_delete_object(IsA(http.HttpRequest),
container.name,
@ -324,6 +346,9 @@ class SwiftTests(test.TestCase):
res = self.client.get(download_url)
self.assertEqual(res.content, obj.data)
self.assertTrue(res.has_header('Content-Disposition'))
self.assertNotContains(res, INVALID_CONTAINER_NAME_1)
self.assertNotContains(res, INVALID_CONTAINER_NAME_2)
# Check that the returned Content-Disposition filename is well
# surrounded by double quotes and with commas removed
expected_name = '"%s"' % obj.name.replace(
@ -343,6 +368,8 @@ class SwiftTests(test.TestCase):
args=[self.containers.first().name,
self.objects.first().name]))
self.assertTemplateUsed(res, 'project/containers/copy.html')
self.assertNotContains(res, INVALID_CONTAINER_NAME_1)
self.assertNotContains(res, INVALID_CONTAINER_NAME_2)
@test.create_stubs({api.swift: ('swift_get_containers',
'swift_copy_object')})
@ -368,7 +395,7 @@ class SwiftTests(test.TestCase):
copy_url = reverse('horizon:project:containers:object_copy',
args=[container_1.name, obj.name])
res = self.client.post(copy_url, formData)
args = (utils_http.urlquote(tables.wrap_delimiter(container_2.name)),)
args = (tables.wrap_delimiter(container_2.name),)
index_url = reverse('horizon:project:containers:index', args=args)
self.assertRedirectsNoFollow(res, index_url)
@ -417,9 +444,8 @@ class SwiftTests(test.TestCase):
res = self.client.get(update_url)
self.assertTemplateUsed(res, 'project/containers/update.html')
res = self.client.get(update_url)
self.assertContains(res, 'enctype="multipart/form-data"')
self._test_invalid_paths(res)
formData = {'method': forms.UpdateObject.__name__,
'container_name': container.name,
@ -427,7 +453,7 @@ class SwiftTests(test.TestCase):
'object_file': temp_file}
res = self.client.post(update_url, formData)
args = (utils_http.urlquote(tables.wrap_delimiter(container.name)),)
args = (tables.wrap_delimiter(container.name),)
index_url = reverse('horizon:project:containers:index', args=args)
self.assertRedirectsNoFollow(res, index_url)
@ -443,16 +469,15 @@ class SwiftTests(test.TestCase):
res = self.client.get(update_url)
self.assertTemplateUsed(res, 'project/containers/update.html')
res = self.client.get(update_url)
self.assertContains(res, 'enctype="multipart/form-data"')
self._test_invalid_paths(res)
formData = {'method': forms.UpdateObject.__name__,
'container_name': container.name,
'name': obj.name}
res = self.client.post(update_url, formData)
args = (utils_http.urlquote(tables.wrap_delimiter(container.name)),)
args = (tables.wrap_delimiter(container.name),)
index_url = reverse('horizon:project:containers:index', args=args)
self.assertRedirectsNoFollow(res, index_url)
@ -473,6 +498,8 @@ class SwiftTests(test.TestCase):
self.assertTemplateUsed(res,
'project/containers/container_detail.html')
self.assertContains(res, container.name, 1, 200)
self.assertNotContains(res, INVALID_CONTAINER_NAME_1)
self.assertNotContains(res, INVALID_CONTAINER_NAME_2)
@test.create_stubs({api.swift: ('swift_get_object', )})
def test_view_object(self):
@ -492,6 +519,7 @@ class SwiftTests(test.TestCase):
self.assertTemplateUsed(
res, 'project/containers/object_detail.html')
self.assertContains(res, obj.name, 1, 200)
self._test_invalid_paths(res)
def test_wrap_delimiter(self):
expected = {
@ -502,13 +530,3 @@ class SwiftTests(test.TestCase):
}
for name, expected_name in expected.items():
self.assertEqual(tables.wrap_delimiter(name), expected_name)
def test_for_url(self):
expected = {
'containerA': 'containerA/',
'containerB%': 'containerB%25/', # urlquote() must be called
'containerC%/': 'containerC%25/',
'containerD%/objectA%': 'containerD%25/objectA%25/'
}
for name, expected_name in expected.items():
self.assertEqual(views.for_url(name), expected_name)

View File

@ -22,17 +22,17 @@ Views for managing Swift containers.
import os
from django.core.urlresolvers import reverse
from django import http
from django.utils.functional import cached_property # noqa
from django.utils import http as utils_http
from django.utils.translation import ugettext_lazy as _
import django.views.generic
from django.views import generic
from horizon import browsers
from horizon import exceptions
from horizon import forms
from horizon.utils import memoized
from horizon.utils.urlresolvers import reverse # noqa
from openstack_dashboard import api
from openstack_dashboard.api import swift
from openstack_dashboard.dashboards.project.containers \
@ -42,16 +42,6 @@ from openstack_dashboard.dashboards.project.containers \
from openstack_dashboard.dashboards.project.containers import tables
def for_url(container_name):
"""Build a URL friendly container name.
Add Swift delimiter if necessary.
The name can contain '%' (bug 1231904).
"""
container_name = tables.wrap_delimiter(container_name)
return utils_http.urlquote(container_name)
class ContainerView(browsers.ResourceBrowserView):
browser_class = project_browsers.ContainerBrowser
template_name = "project/containers/index.html"
@ -142,10 +132,11 @@ class CreateView(forms.ModalFormView):
if parent:
container, slash, remainder = parent.partition(
swift.FOLDER_DELIMITER)
args = (for_url(container), for_url(remainder))
args = (tables.wrap_delimiter(container),
tables.wrap_delimiter(remainder))
return reverse(self.success_url, args=args)
else:
container = for_url(self.request.POST['name'])
container = tables.wrap_delimiter(self.request.POST['name'])
return reverse(self.success_url, args=[container])
def get_initial(self):
@ -182,9 +173,9 @@ class UploadView(forms.ModalFormView):
success_url = "horizon:project:containers:index"
def get_success_url(self):
container_name = for_url(self.request.POST['container_name'])
path = for_url(self.request.POST.get('path', ''))
args = (container_name, path)
container = tables.wrap_delimiter(self.request.POST['container_name'])
path = tables.wrap_delimiter(self.request.POST.get('path', ''))
args = (container, path)
return reverse(self.success_url, args=args)
def get_initial(self):
@ -193,8 +184,7 @@ class UploadView(forms.ModalFormView):
def get_context_data(self, **kwargs):
context = super(UploadView, self).get_context_data(**kwargs)
container_name = utils_http.urlquote(self.kwargs["container_name"])
context['container_name'] = container_name
context['container_name'] = self.kwargs["container_name"]
return context
@ -226,9 +216,10 @@ class CopyView(forms.ModalFormView):
success_url = "horizon:project:containers:index"
def get_success_url(self):
new_container_name = for_url(self.request.POST['new_container_name'])
path = for_url(self.request.POST.get('path', ''))
args = (new_container_name, path)
container = tables.wrap_delimiter(
self.request.POST['new_container_name'])
path = tables.wrap_delimiter(self.request.POST.get('path', ''))
args = (container, path)
return reverse(self.success_url, args=args)
def get_form_kwargs(self):
@ -261,14 +252,12 @@ class CopyView(forms.ModalFormView):
def get_context_data(self, **kwargs):
context = super(CopyView, self).get_context_data(**kwargs)
container_name = utils_http.urlquote(self.kwargs["container_name"])
context['container_name'] = container_name
context['container_name'] = self.kwargs["container_name"]
context['object_name'] = self.kwargs["object_name"]
return context
class ContainerDetailView(forms.ModalFormMixin,
django.views.generic.TemplateView):
class ContainerDetailView(forms.ModalFormMixin, generic.TemplateView):
template_name = 'project/containers/container_detail.html'
@memoized.memoized_method
@ -290,8 +279,7 @@ class ContainerDetailView(forms.ModalFormMixin,
return context
class ObjectDetailView(forms.ModalFormMixin,
django.views.generic.TemplateView):
class ObjectDetailView(forms.ModalFormMixin, generic.TemplateView):
template_name = 'project/containers/object_detail.html'
@memoized.memoized_method
@ -320,9 +308,9 @@ class UpdateObjectView(forms.ModalFormView):
success_url = "horizon:project:containers:index"
def get_success_url(self):
container_name = for_url(self.request.POST['container_name'])
path = for_url(self.request.POST.get('path', ''))
args = (container_name, path)
container = tables.wrap_delimiter(self.request.POST['container_name'])
path = tables.wrap_delimiter(self.request.POST.get('path', ''))
args = (container, path)
return reverse(self.success_url, args=args)
def get_initial(self):
@ -332,10 +320,7 @@ class UpdateObjectView(forms.ModalFormView):
def get_context_data(self, **kwargs):
context = super(UpdateObjectView, self).get_context_data(**kwargs)
context['container_name'] = utils_http.urlquote(
self.kwargs["container_name"])
context['subfolder_path'] = utils_http.urlquote(
self.kwargs["subfolder_path"])
context['object_name'] = utils_http.urlquote(
self.kwargs["object_name"])
context['container_name'] = self.kwargs["container_name"]
context['subfolder_path'] = self.kwargs["subfolder_path"]
context['object_name'] = self.kwargs["object_name"]
return context