Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

files: fix upload of files with special characters in filename #901

Closed
wants to merge 1 commit into from

Conversation

vgranata
Copy link
Contributor

@vgranata vgranata commented Sep 26, 2022

Why are you opening this PR?

#861

How to test / Notes for future debugging

a. Create a deposit with a file and publish it.
b. Create a document with a file (login as superuser)

Examples of filenames to test:
¿Es la unión mística un paradigma para la relación interpersonal? Una respuesta diferenciada en Edith Stein.pdf
L'âme humaine comme "intériorité".pdf
La portée anthropologique du symbole du château chez sainte Thérèse d'Avila.pdf
Le symbole du château. Une proposition d’interprétation relationnelle.pdf
Betschart - 2017 - La portée anthropologique du symbole du château ch.pdf
&%/? éàèê/üäö$£/(){}*"'`+-!$.pdf

Logged in as superuser in the Administration section:

  • Edit document’s metadata (no need to change anything just click on Edit and Save)
  • Edit file’s metadata (no need to change anything just click on Edit and Save)
  • Add files
  • Delete one or more files
  • Add new versions of a file
  • Delete one or more versions of a file
  • Check the download, preview and thumbnail links work.

An example of what could happen is in the screenshot below where the file “Le symbole du château. Une proposition d’interprétation relationnelle.pdf” disappeared from the list and the links for this file do not work any longer.
In this specific case the problem is the right single quotation mark in “d’interprétation” that during the actions above is sometimes encoded into apostrophe.

test_861

characters

Sometimes after one of the action above several fields in the files metadata disappear (type, label, links, order, restriction, thumbnail) and the files are no more visible. In the example below, the file "La portée anthropologig…e Thérèse d'Avila.pdf" is still visible but the others have disappeared.

test_861b
Screenshot 2022-10-05 at 14 21 44

Screenshot 2022-10-05 at 14 22 04

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Extracted translations?

Co-Authored-by: Valeria Granata <valeria@chaw.com>
@vgranata vgranata force-pushed the grv-861-fix-file-broken-link branch from 810ee5a to 25f5636 Compare October 4, 2022 14:02
@vgranata vgranata marked this pull request as ready for review October 5, 2022 07:22
Copy link
Contributor

@jma jma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are missing.

@@ -81,6 +83,8 @@ def get_file_links(file, record):
:param record: Record.
:returns: Dict containing the URL, the download URL and the type of link.
"""
key = quote(unicodedata.normalize('NFD', file['key']))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not do the normalization here but on the final url. A better approach is to use a decorator to do it for several functions or methods i.e. get_thumbnail

@pronguen
Copy link
Contributor

pronguen commented Oct 6, 2022

  • NPR/06.10: When I deposit this file name (&% éàèêüäö$£(){}'`+-!$.pdf), I receive a "400 Bad Request" if I click on the download button as in image below. From all other screens, the PDF can be downloaded without problem.
    image
    "_files": [
        {
          "bucket": "a15d8297-d8b8-4c34-b8b9-f0672384b5e0",
          "category": "main",
          "checksum": "md5:4c2a52395a6812cd4b8e37a2034c8dbd",
          "file_id": "c1bd626f-232c-4f7a-8cb2-8fc03798ef34",
          "key": "&% éàèêüäö$£(){}'`+-!$.pdf",
          "label": "&% éàèêüäö$£(){}'`+-!$",
          "mimetype": "application/pdf",
          "size": 2952766,
          "type": "file",
          "version_id": "13dcb2b0-dac7-46c3-a03e-065fde287180"
        }
      ]
  • NPR/06.10: Inverse problem: I could deposit a file "Le château dans le ciel.pdf" without problem, but the created document had the broken link...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants