Add improvements to the metadata URL encoding and decoding to support ampersand characters as part of the metadata value. GitLab issue #529. Thanks to Mark Maglana @relaxdiego for the report.

Signed-off-by: Roberto Rosario <roberto.rosario.gonzalez@gmail.com>

HISTORY.rst

@@ -5,6 +5,9 @@
 * Add new material from the Wiki to the documentation.
 * Add data migrations to the sources app migraton 0019 to ensure all labels
   are unique before performing the schema migations.
+* Add improvements to the metadata URL encoding and decoding to support
+  ampersand characters as part of the metadata value. GitLab issue 
+  #529. Thanks to Mark Maglana @relaxdiego for the report.
 
 3.1.7 (2018-10-14)
 ==================

mayan/apps/metadata/api.py

@@ -3,7 +3,7 @@ from __future__ import unicode_literals
 from furl import furl
 
 from django.shortcuts import get_object_or_404
-from django.utils.six.moves.urllib.parse import unquote_plus
+from django.utils.encoding import force_bytes
 
 from .models import DocumentMetadata, MetadataType
 
@@ -19,7 +19,7 @@ def decode_metadata_from_querystring(querystring=None):
     metadata_list = []
     if querystring:
         # Match out of order metadata_type ids with metadata values from request
-        for key, value in furl(querystring).args.items():
+        for key, value in furl(force_bytes(querystring)).args.items():
             if 'metadata' in key:
                 index, element = key[8:].split('_')
                 metadata_dict[element][index] = value
@@ -79,13 +79,8 @@ def save_metadata(metadata_dict, document, create=False, _user=None):
             # TODO: Maybe return warning to caller?
             document_metadata = None
 
-    # Handle 'plus sign as space' in the url
-
-    # unquote_plus handles utf-8?!?
-    # http://stackoverflow.com/questions/4382875/handling-iri-in-django
-    # .decode('utf-8')
     if document_metadata:
-        document_metadata.value = unquote_plus(metadata_dict['value'])
+        document_metadata.value = metadata_dict['value']
         document_metadata.save(_user=_user)
 
 

mayan/apps/metadata/tests/literals.py

@@ -18,5 +18,6 @@ TEST_METADATA_TYPE_NAME_EDITED = 'test metadata type name edited'
 TEST_METADATA_VALUE = 'test value'
 TEST_METADATA_VALUE_EDITED = 'test value edited'
 TEST_METADATA_VALUE_UNICODE = 'español'
+TEST_METADATA_VALUE_WITH_AMPERSAND = 'first value & second value'
 TEST_PARSED_VALID_DATE = '2001-01-01'
 TEST_VALID_DATE = '2001-1-1'

mayan/apps/metadata/tests/test_views.py

@@ -318,7 +318,7 @@ class DocumentMetadataTestCase(GenericDocumentViewTestCase):
             metadata_type=metadata_type_2
         )
 
-        response = self.post(
+        self.post(
             'metadata:metadata_add', args=(self.document.pk,), data={
                 'metadata_type': [self.metadata_type.pk, metadata_type_2.pk],
             }

mayan/apps/metadata/tests/test_wizard_steps.py

@@ -14,7 +14,9 @@ from sources.tests.literals import (
     TEST_SOURCE_LABEL, TEST_SOURCE_UNCOMPRESS_N,
 )
 
-from .literals import TEST_METADATA_VALUE_UNICODE
+from .literals import (
+    TEST_METADATA_VALUE_UNICODE, TEST_METADATA_VALUE_WITH_AMPERSAND
+)
 from .mixins import MetadataTypeTestMixin
 
 
@@ -33,7 +35,7 @@ class DocumentUploadMetadataTestCase(MetadataTypeTestMixin, GenericDocumentViewT
             metadata_type=self.metadata_type, required=True
         )
 
-    def test_unicode_interactive_with_unicode_metadata(self):
+    def test_upload_interactive_with_unicode_metadata(self):
         url = furl(reverse('sources:upload_interactive'))
         url.args['metadata0_id'] = self.metadata_type.pk
         url.args['metadata0_value'] = TEST_METADATA_VALUE_UNICODE
@@ -41,8 +43,9 @@ class DocumentUploadMetadataTestCase(MetadataTypeTestMixin, GenericDocumentViewT
         self.grant_access(
             permission=permission_document_create, obj=self.document_type
         )
+
         # Upload the test document
-        with open(TEST_SMALL_DOCUMENT_PATH, 'rb') as file_descriptor:
+        with open(TEST_SMALL_DOCUMENT_PATH, mode='rb') as file_descriptor:
             response = self.post(
                 path=url, data={
                     'document-language': 'eng', 'source-file': file_descriptor,
@@ -55,3 +58,26 @@ class DocumentUploadMetadataTestCase(MetadataTypeTestMixin, GenericDocumentViewT
             Document.objects.first().metadata.first().value,
             TEST_METADATA_VALUE_UNICODE
         )
+
+    def test_upload_interactive_with_ampersand_metadata(self):
+        url = furl(reverse('sources:upload_interactive'))
+        url.args['metadata0_id'] = self.metadata_type.pk
+        url.args['metadata0_value'] = TEST_METADATA_VALUE_WITH_AMPERSAND
+
+        self.grant_access(
+            permission=permission_document_create, obj=self.document_type
+        )
+        # Upload the test document
+        with open(TEST_SMALL_DOCUMENT_PATH, mode='rb') as file_descriptor:
+            response = self.post(
+                path=url, data={
+                    'document-language': 'eng', 'source-file': file_descriptor,
+                    'document_type_id': self.document_type.pk,
+                }
+            )
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(Document.objects.count(), 1)
+        self.assertEqual(
+            Document.objects.first().metadata.first().value,
+            TEST_METADATA_VALUE_WITH_AMPERSAND
+        )

mayan/apps/sources/models.py

@@ -14,7 +14,7 @@ from django.core.files import File
 from django.core.files.base import ContentFile
 from django.db import models, transaction
 from django.utils.encoding import (
-    force_bytes, force_str, force_text, python_2_unicode_compatible
+    force_bytes, force_text, python_2_unicode_compatible
 )
 from django.utils.timezone import now
 from django.utils.translation import ugettext_lazy as _

mayan/apps/sources/views.py

@@ -2,12 +2,14 @@ from __future__ import absolute_import, unicode_literals
 
 import logging
 
+from furl import furl
+
 from django.contrib import messages
 from django.http import HttpResponseRedirect, JsonResponse
 from django.shortcuts import get_object_or_404
 from django.template import RequestContext
 from django.urls import reverse, reverse_lazy
-from django.utils.encoding import force_text, uri_to_iri
+from django.utils.encoding import force_text
 from django.utils.translation import ugettext_lazy as _
 
 from acls.models import AccessControlList
@@ -191,6 +193,7 @@ class UploadBaseView(MultiFormView):
 
 
 class UploadInteractiveView(UploadBaseView):
+
     def dispatch(self, request, *args, **kwargs):
         self.subtemplates_list = []
 
@@ -253,6 +256,10 @@ class UploadInteractiveView(UploadBaseView):
             except Exception as exception:
                 messages.error(self.request, exception)
 
+            querystring = furl()
+            querystring.args.update(self.request.GET)
+            querystring.args.update(self.request.POST)
+
             try:
                 task_source_handle_upload.apply_async(
                     kwargs=dict(
@@ -263,11 +270,7 @@ class UploadInteractiveView(UploadBaseView):
                             filename=force_text(shared_uploaded_file)
                         ),
                         language=forms['document_form'].cleaned_data.get('language'),
-                        querystring=uri_to_iri(
-                            '?{}&{}'.format(
-                                self.request.GET.urlencode(), self.request.POST.urlencode()
-                            )
-                        ),
+                        querystring=querystring,
                         shared_uploaded_file_id=shared_uploaded_file.pk,
                         source_id=self.source.pk,
                         user_id=user_id,

mayan/apps/sources/wizards.py

@@ -1,11 +1,12 @@
 from __future__ import unicode_literals
 
+from furl import furl
+
 from django.apps import apps
 from django.contrib import messages
 from django.http import HttpResponseRedirect
 from django.urls import reverse
 from django.utils.decorators import classonlymethod
-from django.utils.http import urlencode
 from django.utils.translation import ugettext_lazy as _
 
 from formtools.wizard.views import SessionWizardView
@@ -185,11 +186,9 @@ class DocumentCreateWizard(SessionWizardView):
         for step in WizardStep.get_all():
             query_dict.update(step.done(wizard=self) or {})
 
-        url = '?'.join(
-            [
-                reverse('sources:upload_interactive'),
-                urlencode(query_dict, doseq=True)
-            ]
-        )
+        url = furl(reverse('sources:upload_interactive'))
+        # Use equal and not .update() to get the same result as using
+        # urlencode(doseq=True)
+        url.args = query_dict
 
         return HttpResponseRedirect(url)