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

Handling of 'multiValueQuery' not respecting encoded characters #277

Closed
vandrade-git opened this issue Feb 3, 2025 · 5 comments
Closed
Assignees

Comments

@vandrade-git
Copy link
Contributor

When trying to handle multiple query parameters with encoded characters, the decoding is done before the split resulting in some weird behavior.
My example is the encoding of commas.

Normal case:
q=test-1,test-2 -> q: [ 'test-1', 'test-2']

Un-encoded comma case (seems correct):
q=test-1,test,2 -> q: [ 'test-1', 'test', '2']

Encoded comma case (seems incorrect):
q=test-1,test%2C2 -> q: [ 'test-1', 'test', '2'] when I would expect it to be q: [ 'test-1', 'test,2']

Looking at the source I can see the logic that explains why this is happening:

Current behavior:
decodeURIComponent('test-1,test%2C2').split(',')

My expected behavior:
'test-1,test%2C2'.split(',').map((c) => decodeURIComponent(c))

Is this the intended behavior or a strange edge case?

@naorpeled naorpeled self-assigned this Feb 4, 2025
@naorpeled
Copy link
Collaborator

Hey @vandrade-git,
interesting 🤔

Wdyt making this a configurable option?
And if so, would you want to open a PR for it?

@vandrade-git
Copy link
Contributor Author

Hi @naorpeled

I don't mind creating a PR to address this although I am not sure how being configurable would work in this case. Is your idea something similar to the 'serializer' configuration option?

@naorpeled
Copy link
Collaborator

naorpeled commented Feb 6, 2025

Hi @naorpeled

I don't mind creating a PR to address this although I am not sure how being configurable would work in this case. Is your idea something similar to the 'serializer' configuration option?

exactly, and I'd call it something like queryStringSerializer

what do you think?

@vandrade-git
Copy link
Contributor Author

After heavy testing looking for a solution I realized that everything in lambda-api is working correctly.
The problem is caused by how 'Function URLs' work (in Localstack at least). When using ALB everything works correctly so this report is invalid.
I am closing the issue.

@vandrade-git vandrade-git closed this as not planned Won't fix, can't repro, duplicate, stale Feb 9, 2025
@naorpeled
Copy link
Collaborator

naorpeled commented Feb 9, 2025

After heavy testing looking for a solution I realized that everything in lambda-api is working correctly. The problem is caused by how 'Function URLs' work (in Localstack at least). When using ALB everything works correctly so this report is invalid. I am closing the issue.

Alrighty, if you need anything else feel free to ping me 🙏
BTW, if you have a bit of time I'd love to hear you thoughts regarding the new type safety rework look and feel that is described in this PR's description

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

No branches or pull requests

2 participants