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

ACAS-761: Fix download urls for experiment files containing # special character #1146

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

bffrost
Copy link
Collaborator

@bffrost bffrost commented Mar 28, 2024

Description

I dug into this a little more and found that there are some subtle differences between encodeURI which is meant to encode an entire URL / URI and encodeURIComponent which is meant to encode a query string or path component in a URL. For more info: https://stackoverflow.com/questions/75980/when-are-you-supposed-to-use-escape-instead-of-encodeuri-encodeuricomponent

I found that a simple switch to encoding only the fileValue with encodeURIComponent fixes the issue with # characters.

Related Issue

https://schrodinger.atlassian.net/browse/ACAS-761

How Has This Been Tested?

  • Tested uploading an experiment file whose filename contained # and confirmed I could download it again
  • Also tested a horrible file with name: Generic Special Chars ! @ # $ % ^ & * ( ) - _ = + [ ] { } | \ / < > ' " ? ,.xlsx. This filename gets clobbered during the upload, and gets saved to the ACAS filestore as : < > ' %22 ? ,.xlsx, which is a related but separate bug. Before this fix, that file could not be downloaded from the experiment browser, and after the fix it can be downloaded successfully.

@bffrost bffrost requested a review from brianbolt March 28, 2024 22:46
Copy link
Contributor

@brianbolt brianbolt left a comment

Choose a reason for hiding this comment

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

LGTM.

Just pointing out additionally that any ACAS data viewers like LiveDesign would also need employ its own mechanism of escaping the path as it is actually stored in the DB with # character.

@bffrost bffrost merged commit 05d2ca1 into release/2024.2.x Mar 28, 2024
2 checks passed
@bffrost bffrost deleted the ACAS-761 branch March 28, 2024 23:02
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.

2 participants