Merge "Swift name usage cleanup. Unicode support and slash prohibition."

11 files changed, 114 insertions, 41 deletions

diff --git a/horizon/horizon/api/swift.py b/horizon/horizon/api/swift.py
index e9b93fde5..9af3aa9c6 100644
--- a/horizon/horizon/api/swift.py
+++ b/horizon/horizon/api/swift.py
@@ -22,6 +22,7 @@ import logging
 
 import cloudfiles
 from django.conf import settings
+from django.utils.translation import ugettext as _
 
 from horizon import exceptions
 from horizon.api.base import url_for
@@ -100,6 +101,18 @@ def swift_get_objects(request, container_name, prefix=None, marker=None):
 
 def swift_copy_object(request, orig_container_name, orig_object_name,
                       new_container_name, new_object_name):
+    try:
+        # FIXME(gabriel): Cloudfiles currently fails at unicode in the
+        # copy_to method, so to provide a better experience we check for
+        # unicode here and pre-empt with an error message rather than
+        # letting the call fail.
+        str(orig_container_name)
+        str(orig_object_name)
+        str(new_container_name)
+        str(new_object_name)
+    except UnicodeEncodeError:
+        raise exceptions.HorizonException(_("Unicode is not currently "
+                                            "supported for object copy."))
     container = swift_api(request).get_container(orig_container_name)
 
     if swift_object_exists(request, new_container_name, new_object_name):
@@ -109,10 +122,10 @@ def swift_copy_object(request, orig_container_name, orig_object_name,
     return orig_obj.copy_to(new_container_name, new_object_name)
 
 
-def swift_upload_object(request, container_name, object_name, object_data):
+def swift_upload_object(request, container_name, object_name, object_file):
     container = swift_api(request).get_container(container_name)
     obj = container.create_object(object_name)
-    obj.write(object_data)
+    obj.send(object_file)
     return obj
 
 
diff --git a/horizon/horizon/dashboards/nova/containers/forms.py b/horizon/horizon/dashboards/nova/containers/forms.py
index 3ee121bea..308dd1f45 100644
--- a/horizon/horizon/dashboards/nova/containers/forms.py
+++ b/horizon/horizon/dashboards/nova/containers/forms.py
@@ -21,8 +21,9 @@
 import logging
 
 from django import shortcuts
-from django.core.urlresolvers import reverse
 from django.contrib import messages
+from django.core import validators
+from django.core.urlresolvers import reverse
 from django.utils.translation import ugettext as _
 
 from horizon import api
@@ -33,8 +34,16 @@ from horizon import forms
 LOG = logging.getLogger(__name__)
 
 
+no_slash_validator = validators.RegexValidator(r'^(?u)[^/]+$',
+                                               _("Slash is not an allowed "
+                                                 "character."),
+                                               code="noslash")
+
+
 class CreateContainer(forms.SelfHandlingForm):
-    name = forms.CharField(max_length="255", label=_("Container Name"))
+    name = forms.CharField(max_length="255",
+                           label=_("Container Name"),
+                           validators=[no_slash_validator])
 
     def handle(self, request, data):
         try:
@@ -46,7 +55,9 @@ class CreateContainer(forms.SelfHandlingForm):
 
 
 class UploadObject(forms.SelfHandlingForm):
-    name = forms.CharField(max_length="255", label=_("Object Name"))
+    name = forms.CharField(max_length="255",
+                           label=_("Object Name"),
+                           validators=[no_slash_validator])
     object_file = forms.FileField(label=_("File"))
     container_name = forms.CharField(widget=forms.HiddenInput())
 
@@ -56,7 +67,7 @@ class UploadObject(forms.SelfHandlingForm):
             obj = api.swift_upload_object(request,
                                           data['container_name'],
                                           data['name'],
-                                          object_file.read())
+                                          object_file)
             obj.metadata['orig-filename'] = object_file.name
             obj.sync_metadata()
             messages.success(request, _("Object was successfully uploaded."))
@@ -67,11 +78,11 @@ class UploadObject(forms.SelfHandlingForm):
 
 
 class CopyObject(forms.SelfHandlingForm):
-    new_container_name = forms.ChoiceField(
-        label=_("Container to store object in"))
-
+    new_container_name = forms.ChoiceField(label=_("Destination container"),
+                                           validators=[no_slash_validator])
     new_object_name = forms.CharField(max_length="255",
-                                      label=_("New object name"))
+                                      label=_("Destination object name"),
+                                      validators=[no_slash_validator])
     orig_container_name = forms.CharField(widget=forms.HiddenInput())
     orig_object_name = forms.CharField(widget=forms.HiddenInput())
 
@@ -95,6 +106,9 @@ class CopyObject(forms.SelfHandlingForm):
             vals = {"container": new_container, "obj": new_object}
             messages.success(request, _('Object "%(obj)s" copied to container '
                                         '"%(container)s".') % vals)
+        except exceptions.HorizonException, exc:
+            messages.error(request, exc)
+            return shortcuts.redirect(object_index, orig_container)
         except:
             redirect = reverse(object_index, args=(orig_container,))
             exceptions.handle(request,
diff --git a/horizon/horizon/dashboards/nova/containers/tests.py b/horizon/horizon/dashboards/nova/containers/tests.py
index 95e9ec3a3..71b9be6fc 100644
--- a/horizon/horizon/dashboards/nova/containers/tests.py
+++ b/horizon/horizon/dashboards/nova/containers/tests.py
@@ -22,6 +22,7 @@ import tempfile
 
 from cloudfiles.errors import ContainerNotEmpty
 from django import http
+from django.core.files.uploadedfile import InMemoryUploadedFile
 from django.core.urlresolvers import reverse
 from mox import IsA
 
@@ -50,12 +51,12 @@ class ContainerViewTests(test.TestCase):
         self.assertEqual(len(resp_containers), len(containers))
 
     def test_delete_container(self):
-        container = self.containers.get(name="container_two")
+        container = self.containers.get(name=u"container_two\u6346")
         self.mox.StubOutWithMock(api, 'swift_delete_container')
         api.swift_delete_container(IsA(http.HttpRequest), container.name)
         self.mox.ReplayAll()
 
-        action_string = "containers__delete__%s" % container.name
+        action_string = u"containers__delete__%s" % container.name
         form_data = {"action": action_string}
         req = self.factory.post(CONTAINER_INDEX_URL, form_data)
         table = ContainersTable(req, self.containers.list())
@@ -70,7 +71,7 @@ class ContainerViewTests(test.TestCase):
                                    container.name).AndRaise(exc)
         self.mox.ReplayAll()
 
-        action_string = "containers__delete__%s" % container.name
+        action_string = u"containers__delete__%s" % container.name
         form_data = {"action": action_string}
         req = self.factory.post(CONTAINER_INDEX_URL, form_data)
         table = ContainersTable(req, self.containers.list())
@@ -130,7 +131,7 @@ class ObjectViewTests(test.TestCase):
         api.swift_upload_object(IsA(http.HttpRequest),
                                 container.name,
                                 obj.name,
-                                OBJECT_DATA).AndReturn(obj)
+                                IsA(InMemoryUploadedFile)).AndReturn(obj)
         self.mox.StubOutWithMock(obj, 'sync_metadata')
         obj.sync_metadata()
         self.mox.ReplayAll()
@@ -149,6 +150,19 @@ class ObjectViewTests(test.TestCase):
                             args=[container.name])
         self.assertRedirectsNoFollow(res, index_url)
 
+        # Test invalid filename
+        formData['name'] = "contains/a/slash"
+        res = self.client.post(upload_url, formData)
+        self.assertNoMessages()
+        self.assertContains(res, "Slash is not an allowed character.")
+
+        # Test invalid container name
+        #formData['container_name'] = "contains/a/slash"
+        #formData['name'] = "no_slash"
+        #res = self.client.post(upload_url, formData)
+        #self.assertNoMessages()
+        #self.assertContains(res, "Slash is not an allowed character.")
+
     def test_delete(self):
         container = self.containers.first()
         obj = self.objects.first()
@@ -201,8 +215,8 @@ class ObjectViewTests(test.TestCase):
         self.assertTemplateUsed(res, 'nova/objects/copy.html')
 
     def test_copy(self):
-        container_1 = self.containers.get(name="container_one")
-        container_2 = self.containers.get(name="container_two")
+        container_1 = self.containers.get(name=u"container_one\u6346")
+        container_2 = self.containers.get(name=u"container_two\u6346")
         obj = self.objects.first()
 
         self.mox.StubOutWithMock(api, 'swift_get_containers')
diff --git a/horizon/horizon/dashboards/nova/containers/views.py b/horizon/horizon/dashboards/nova/containers/views.py
index 6cfc3c4e2..4b31ec5df 100644
--- a/horizon/horizon/dashboards/nova/containers/views.py
+++ b/horizon/horizon/dashboards/nova/containers/views.py
@@ -122,7 +122,8 @@ def object_download(request, container_name, object_name):
                           _("Unable to retrieve object."),
                           redirect=redirect)
     response = http.HttpResponse()
-    response['Content-Disposition'] = 'attachment; filename=%s' % filename
+    safe_name = filename.encode('utf-8')
+    response['Content-Disposition'] = 'attachment; filename=%s' % safe_name
     response['Content-Type'] = 'application/octet-stream'
     for data in object_data:
         response.write(data)
diff --git a/horizon/horizon/dashboards/nova/templates/nova/containers/create.html b/horizon/horizon/dashboards/nova/templates/nova/containers/create.html
index 31f0de34b..1facb05d8 100644
--- a/horizon/horizon/dashboards/nova/templates/nova/containers/create.html
+++ b/horizon/horizon/dashboards/nova/templates/nova/containers/create.html
@@ -7,7 +7,7 @@
 {% endblock page_header %}
 
 {% block dash_main %}
-  {% include "nova/containers/_create.html" with form=create_form %}
+  {% include "nova/containers/_create.html" %}
 {% endblock %}
 
 
diff --git a/horizon/horizon/static/horizon/js/horizon.js b/horizon/horizon/static/horizon/js/horizon.js
index 727dfbc2b..4bbdf8c0f 100644
--- a/horizon/horizon/static/horizon/js/horizon.js
+++ b/horizon/horizon/static/horizon/js/horizon.js
@@ -26,11 +26,11 @@ var Horizon = function() {
     // Bind event handlers to confirm dangerous actions.
     $("body").on("click", "form .btn-danger", function (evt) {
       horizon.datatables.confirm(this);
-      return false;
+      evt.preventDefault();
     });
 
     // Bind dismiss(x) handlers for alert messages.
-    $(".alert").alert()
+    $(".alert").alert();
 
     $.each(initFunctions, function(ind, fn) {
       fn();
diff --git a/horizon/horizon/tables/base.py b/horizon/horizon/tables/base.py
index eade30d10..3e24a769d 100644
--- a/horizon/horizon/tables/base.py
+++ b/horizon/horizon/tables/base.py
@@ -286,7 +286,7 @@ class Row(object):
                 widget = forms.CheckboxInput(check_test=False)
                 # Convert value to string to avoid accidental type conversion
                 data = widget.render('object_ids',
-                                     str(table.get_object_id(datum)))
+                                     unicode(table.get_object_id(datum)))
                 table._data_cache[column][table.get_object_id(datum)] = data
             elif column.auto == "actions":
                 data = table.render_row_actions(datum)
diff --git a/horizon/horizon/test.py b/horizon/horizon/test.py
index afb1c5749..da017eb67 100644
--- a/horizon/horizon/test.py
+++ b/horizon/horizon/test.py
@@ -162,14 +162,22 @@ class TestCase(django_test.TestCase):
         temp_req = self.client.request(**{'wsgi.input': None})
         temp_req.COOKIES = self.client.cookies
         storage = default_storage(temp_req)
+        messages = []
         # To gain early access to the messages we have to decode the
         # cookie on the test client.
         if 'messages' in self.client.cookies:
             messages = storage._decode(self.client.cookies['messages'].value)
-        elif any(kwargs.values()):
+
+        # If we don't have messages and we don't expect messages, we're done.
+        if not any(kwargs.values()) and not messages:
+            return
+
+        # If we expected messages and have none, that's a problem.
+        if any(kwargs.values()) and not messages:
             error_msg = "Messages were expected, but none were set."
             assert 0 == sum(kwargs.values()), error_msg
 
+        # Otherwise, make sure we got the expected messages.
         for msg_type, count in kwargs.items():
             msgs = [m.message for m in messages if msg_type in m.tags]
             assert len(msgs) == count, \
diff --git a/horizon/horizon/tests/api_tests/swift_tests.py b/horizon/horizon/tests/api_tests/swift_tests.py
index 9b405844e..89902041b 100644
--- a/horizon/horizon/tests/api_tests/swift_tests.py
+++ b/horizon/horizon/tests/api_tests/swift_tests.py
@@ -84,8 +84,8 @@ class SwiftApiTests(test.APITestCase):
         swift_api.get_container(container.name).AndReturn(container)
         self.mox.StubOutWithMock(container, 'create_object')
         container.create_object(obj.name).AndReturn(obj)
-        self.mox.StubOutWithMock(obj, 'write')
-        obj.write(OBJECT_DATA).AndReturn(obj)
+        self.mox.StubOutWithMock(obj, 'send')
+        obj.send(OBJECT_DATA).AndReturn(obj)
         self.mox.ReplayAll()
 
         ret_val = api.swift_upload_object(self.request,
@@ -147,24 +147,37 @@ class SwiftApiTests(test.APITestCase):
         self.assertFalse(api.swift_object_exists(*args))
 
     def test_swift_copy_object(self):
-        container = self.containers.get(name="container_one")
-        container_2 = self.containers.get(name="container_two")
+        container = self.containers.get(name=u"container_one\u6346")
+        container_2 = self.containers.get(name=u"container_two\u6346")
         obj = self.objects.first()
 
         swift_api = self.stub_swiftclient()
         self.mox.StubOutWithMock(api.swift, 'swift_object_exists')
-        swift_api.get_container(container.name).AndReturn(container)
-        api.swift.swift_object_exists(self.request,
-                                      container_2.name,
-                                      obj.name).AndReturn(False)
         self.mox.StubOutWithMock(container, 'get_object')
-        container.get_object(obj.name).AndReturn(obj)
         self.mox.StubOutWithMock(obj, 'copy_to')
-        obj.copy_to(container_2.name, obj.name)
+        # Using the non-unicode names here, see below.
+        swift_api.get_container("no_unicode").AndReturn(container)
+        api.swift.swift_object_exists(self.request,
+                                      "also no unicode",
+                                      "obj_with_no_unicode").AndReturn(False)
+        container.get_object("obj_with_no_unicode").AndReturn(obj)
+        obj.copy_to("also no unicode", "obj_with_no_unicode")
         self.mox.ReplayAll()
+
+        # Unicode fails... we'll get to a successful test in a minute
+        with self.assertRaises(exceptions.HorizonException):
+            api.swift_copy_object(self.request,
+                                  container.name,
+                                  obj.name,
+                                  container_2.name,
+                                  obj.name)
+
         # Verification handled by mox. No assertions needed.
+        container.name = "no_unicode"
+        container_2.name = "also no unicode"
+        obj.name = "obj_with_no_unicode"
         api.swift_copy_object(self.request,
-                              container.name,
-                              obj.name,
-                              container_2.name,
-                              obj.name)
+                                  container.name,
+                                  obj.name,
+                                  container_2.name,
+                                  obj.name)
diff --git a/horizon/horizon/tests/test_data/keystone_data.py b/horizon/horizon/tests/test_data/keystone_data.py
index 958ab45a3..64cd8703c 100644
--- a/horizon/horizon/tests/test_data/keystone_data.py
+++ b/horizon/horizon/tests/test_data/keystone_data.py
@@ -88,6 +88,7 @@ def data(TEST):
     user2 = users.User(users.UserManager, user_dict)
     TEST.users.add(user, user2)
     TEST.user = user  # Your "current" user
+    TEST.user.service_catalog = SERVICE_CATALOG
 
     tenant_dict = {'id': "1",
                    'name': 'test_tenant',
diff --git a/horizon/horizon/tests/test_data/swift_data.py b/horizon/horizon/tests/test_data/swift_data.py
index aa077d592..394f0a560 100644
--- a/horizon/horizon/tests/test_data/swift_data.py
+++ b/horizon/horizon/tests/test_data/swift_data.py
@@ -14,8 +14,11 @@
 
 import new
 
+from django import http
+
 from cloudfiles import container, storage_object
 
+from horizon.api import base
 from .utils import TestDataContainer
 
 
@@ -23,21 +26,27 @@ def data(TEST):
     TEST.containers = TestDataContainer()
     TEST.objects = TestDataContainer()
 
+    request = http.HttpRequest()
+    request.user = TEST.user
+
     class FakeConnection(object):
         def __init__(self):
             self.cdn_enabled = False
+            self.uri = base.url_for(request, "object-store")
+            self.token = TEST.token
+            self.user_agent = "python-cloudfiles"
 
     conn = FakeConnection()
 
-    container_1 = container.Container(conn, name="container_one")
-    container_2 = container.Container(conn, name="container_two")
+    container_1 = container.Container(conn, name=u"container_one\u6346")
+    container_2 = container.Container(conn, name=u"container_two\u6346")
     TEST.containers.add(container_1, container_2)
 
-    object_dict = {"name": "test_object",
-                   "content_type": "text/plain",
+    object_dict = {"name": u"test_object\u6346",
+                   "content_type": u"text/plain",
                    "bytes": 128,
                    "last_modified": None,
-                   "hash": "object_hash"}
+                   "hash": u"object_hash"}
     obj_dicts = [object_dict]
     for obj_dict in obj_dicts:
         swift_object = storage_object.Object(container_1,