Skip to content

Conversation

@danxuliu
Copy link
Member

Fixes #1102

The filename may contain characters that are not compatible with a URL path, so it needs to be explicitly encoded. Otherwise the upload could fail (for example, if the filename contains % followed by a letter, which would cause the server to fail to decode the path and return a 400 Bad request error) or even overwrite a different file (for example, if the filename contains #, which would cause the rest of the path to be ignored).

How to test (scenario 1)

  • Open the Files app
  • Ensure that there is no folder or file named just test
  • Create a new folder named test#folder
  • Open the new folder
  • Upload a PDF file
  • Open the PDF file
  • Add an annotation
  • Save the PDF file
  • Close the PDF file
  • Open it again

Result with this pull request

The annotation is in the file

Result without this pull request

The annotation is not in the file. If the root folder is opened again there is now a file named test in it.

How to test (scenario 2)

  • Open the Files app
  • Create a new folder named #test
  • Open the new folder
  • Upload a PDF file
  • Open the PDF file
  • Add an annotation
  • Save the PDF file

Result with this pull request

The file is saved

Result without this pull request

An error is shown and the file is not saved. The "Network" tab of the browser development tools show that the request failed with "409 Conflict" (as it tried to overwrite the root folder).

How to test (scenario 3)

  • Open the Files app
  • Create a new folder named test%folder
  • Open the new folder
  • Upload a PDF file
  • Open the PDF file
  • Add an annotation
  • Save the PDF file

Result with this pull request

The file is saved

Result without this pull request

An error is shown and the file is not saved. The "Network" tab of the browser development tools show that the request failed with "400 Bad request" (as %f in the path could not be decoded).

@danxuliu
Copy link
Member Author

/backport to stable31

@danxuliu
Copy link
Member Author

/backport to stable30

@danxuliu
Copy link
Member Author

/backport to stable29

@danxuliu danxuliu requested a review from szaimen February 21, 2025 16:51
danxuliu added 2 commits April 2, 2025 20:18
@nextcloud/paths was already an implicit dependency, but now it is made
explicit to use the functions it provides in the PDF viewer code.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The filename may contain characters that are not compatible with a URL
path, so it needs to be explicitly encoded. Otherwise the upload could
fail (for example, if the filename contains "%" followed by a letter,
which would cause the server to fail to decode the path and return a
"400 Bad request" error) or even overwrite a different file (for
example, if the filename contains "#", which would cause the rest of the
path to be ignored).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu force-pushed the fix-saving-annotations-in-paths-with-special-characters branch from 96969fd to eb63e77 Compare April 2, 2025 18:30
@danxuliu
Copy link
Member Author

danxuliu commented Apr 2, 2025

/compile /

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
Copy link
Collaborator

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

🐘

@susnux susnux merged commit f7a2909 into master Apr 23, 2025
36 checks passed
@danxuliu danxuliu deleted the fix-saving-annotations-in-paths-with-special-characters branch May 6, 2025 09:20
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PDF Comments can not be saved if path containes a #

6 participants