diff --git a/HISTORY.rst b/HISTORY.rst index 6f7c1d0d11..0fa3d4e6db 100644 --- a/HISTORY.rst +++ b/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) ================== diff --git a/mayan/apps/metadata/api.py b/mayan/apps/metadata/api.py index ed641f7a14..3848d0822d 100644 --- a/mayan/apps/metadata/api.py +++ b/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) diff --git a/mayan/apps/metadata/tests/literals.py b/mayan/apps/metadata/tests/literals.py index d35c2c0e84..feff034f88 100644 --- a/mayan/apps/metadata/tests/literals.py +++ b/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' diff --git a/mayan/apps/metadata/tests/test_views.py b/mayan/apps/metadata/tests/test_views.py index db88b27fe4..c0c2d5a787 100644 --- a/mayan/apps/metadata/tests/test_views.py +++ b/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], } diff --git a/mayan/apps/metadata/tests/test_wizard_steps.py b/mayan/apps/metadata/tests/test_wizard_steps.py index 3f4b51cac8..a0cbfca898 100644 --- a/mayan/apps/metadata/tests/test_wizard_steps.py +++ b/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 + ) diff --git a/mayan/apps/sources/models.py b/mayan/apps/sources/models.py index c9dabe23e2..eb6464cd90 100644 --- a/mayan/apps/sources/models.py +++ b/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 _ diff --git a/mayan/apps/sources/views.py b/mayan/apps/sources/views.py index 8372ab19df..e4484cceb4 100644 --- a/mayan/apps/sources/views.py +++ b/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, diff --git a/mayan/apps/sources/wizards.py b/mayan/apps/sources/wizards.py index 98da80454b..61802119dc 100644 --- a/mayan/apps/sources/wizards.py +++ b/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)