Skip to content

Conversation

@fabianzw
Copy link
Contributor

@fabianzw fabianzw commented Mar 25, 2025

Summary

Fixed two issues in downloadAction.ts that prevented users from downloading multiple files when one of the file names included a comma or a hash symbol.

  1. downloadAction.ts:66: Changed decodeURI to decodeURIComponent
    decodeURI does not decode characters that are not part of the URI syntax. For example, a file named "A,B.txt" was requested as "A%2CB.txt".

  2. downloadAction.ts:58,61: Changed "new URL(nodes[0].source)" to "new URL(nodes[0].encodedSource)"
    In .source, hash symbols are not escaped, which confuses the logic as url.pathname does not include the hash symbol and everything after it.

Checklist

Sorry for the second PR, I forgot the signoff message in the first..

@fabianzw fabianzw requested review from a team and skjnldsv as code owners March 25, 2025 11:06
@fabianzw fabianzw requested review from sorbaugh and susnux and removed request for a team March 25, 2025 11:06
@skjnldsv
Copy link
Member

/backport to stable31

@skjnldsv
Copy link
Member

/backport to stable30

@skjnldsv
Copy link
Member

/backport to stable29

@fabianzw
Copy link
Contributor Author

fabianzw commented Mar 26, 2025

/compile
Edit: Whoops.. Doesn't work for forks, I did it manually..

Signed-off-by: Fabian Zwemke <fabian@zwemke.de>
Signed-off-by: Fabian Zwemke <fabian@zwemke.de>
Comment on lines +58 to +61
url = new URL(nodes[0].encodedSource)
let base = url.pathname
for (const node of nodes.slice(1)) {
base = longestCommonPath(base, (new URL(node.source).pathname))
base = longestCommonPath(base, (new URL(node.encodedSource).pathname))
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about this path as the URL constructor should already encode the URL so this smells like double encoding. Will test tomorrow with some more examples.

@susnux susnux added the 3. to review Waiting for reviews label Mar 26, 2025
@skjnldsv skjnldsv merged commit 96e2555 into nextcloud:master Mar 27, 2025
108 of 111 checks passed
@backportbot
Copy link

backportbot bot commented Mar 27, 2025

The backport to stable29 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable29
git pull origin stable29

# Create the new backport branch
git checkout -b backport/51684/stable29

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 9fd0d263 52cabd78

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/51684/stable29

Error: Failed to check for changes with origin/stable29: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@backportbot
Copy link

backportbot bot commented Mar 27, 2025

The backport to stable30 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable30
git pull origin stable30

# Create the new backport branch
git checkout -b backport/51684/stable30

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 9fd0d263 52cabd78

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/51684/stable30

Error: Failed to check for changes with origin/stable30: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@skjnldsv
Copy link
Member

Not backportable for 29 & 30 because #48098 created too much differences :)

@github-actions
Copy link
Contributor

github-actions bot commented Apr 9, 2025

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@nextcloud-bot nextcloud-bot mentioned this pull request Aug 19, 2025
@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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Downloads of multiple files with "," in filename fails

4 participants