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

Fix for "s3 key with url like /?foo=bar with query on / path, leads to error when reading .createReadStream()" #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jplwood
Copy link

@jplwood jplwood commented Sep 22, 2020

If I have a defaultKey such as index.html, and I request a / route with a query string, e.g., /?foo=bar. If i do this, that first if statement thats supposed to replace "" with defaultKey, s3Key will be ?foo=bar and my defaultKey will not be used. The code continues and removes the query string from the s3Key making it s3Key = "". If my s3Key is passed to the s3Params as a blank string, which is passed to s3Request and finally when we call s3Request.createReadStream(), a 500 error is thrown by aws param_validator: UriParameterError: Expected uri parameter to have length >= 1, but found "" for params.Key. \n\n this fix removes the query string first since there is really nouse for it, to have the if that replaces "/" with defaultKey will work even if there is a query string.

…th a query string, e.g., /?foo=bar. If i do this, that first if statement thats supposed to replace "" with defaultKey, s3Key will be ?foo=bar and

my defaultKey will not be used. The code continues and removes the query string from the s3Key making it s3Key = "". If my s3Key is passed to the s3Params as a blank string, which is passed to s3Request and finally when we call s3Requ
est.createReadStream\(\), a 500 error is thrown by aws param_validator: UriParameterError: Expected uri parameter to have length >= 1, but found "" for params.Key. \n\n this fix removes the query string first since there is really no
use for it, to have the if that replaces "/" with defaultKey will work even if there is a query string.
@jplwood
Copy link
Author

jplwood commented Sep 22, 2020

This is a fix for the issue I opened #22

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.

1 participant