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

Plus-sign in S3 key causes 404 #88

Closed
robdasilva opened this issue Dec 16, 2019 · 3 comments · Fixed by #223
Closed

Plus-sign in S3 key causes 404 #88

robdasilva opened this issue Dec 16, 2019 · 3 comments · Fixed by #223

Comments

@robdasilva
Copy link

Trying to fetch an image for an object in S3 with a '+' in its key causes 404.

Example:

GET https://<cname>.imgix.net/E+P-003_D.jpeg // -> 404
GET https://<cname>.imgix.net/E%2BP-003_D.jpeg // -> 200

I suspect, it's due to encodeURI in https://github.com/imgix/imgix-core-js/blob/master/src/imgix-core-js.js#L106. As the S3 key is a path, the module uses encodeURI, which accepts '+' symbols and does not encode them. S3, however, interprets those as spaces.

@sherwinski
Copy link
Contributor

Hey @RSchweizer,
Thanks for bringing this up. Unfortunately there isn't a straightforward solution for this as encoding the + runs the risk of breaking setups that don't use S3 sources. For now the quickest solution would be to separately encode the + after generating the URL:

client.buildURL(path, params).replace(/\+/g, '%2B')

I know it's not a satisfying answer, but know that we are currently investigating a better solution for this. Feel free to comment back if you have any other questions. Thanks

@robdasilva
Copy link
Author

Hi @sherwinski,

Thank you! That's also the quick fix, I came up with yesterday :)

Let me know, if I can help with anything in finding a more elegant solution.

@ericdeansanchez
Copy link
Contributor

@RSchweizer hey 👋 thanks for opening this issue and bringing this to our attention. I am glad that you and @sherwinski have found a similar solution :)

Moving forward, we're already thinking harder about how/when encoding should/shouldn't take place as well as handling edge cases. I've created an internal document tracking this issue and we are already working on a more elegant solution.

I am going to close this out for now, but please feel free to comment/pull/open-an-issue again if you have any questions, comments, or concerns.

@sherwinski sherwinski linked a pull request Jan 28, 2021 that will close this issue
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 a pull request may close this issue.

3 participants