Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Fix runtime router handling of encoded query parameters #1620

Merged
merged 1 commit into from Dec 14, 2020

Conversation

bermi
Copy link
Contributor

@bermi bermi commented Oct 30, 2020

Hello Sapper team! Thanks for creating this fantastic framework.

I found an issue where a link such as

<a href="/?multiline=multi%0Aline">link</a>

will cause runtime routes to parse the query parameters as

{ multiline: "multi" }

instead of

{ multiline: "multi\nline" }

It looks like sapper@0.6.0 removed the dependency from URLSearchParams with a custom RegExp to accommodate IE11. Using URLSearchParams prevents this and potentially future query parsing bugs from happening, so this PR tries to use URLSearchParams when available and fixes the old RegExp to allow.

To fix current query parser adding the RegExp/dotAll /s would be sufficient but unfortunately, it does not work on IE11 or FF Android. So the [\s\S]* RegExp tick is used as opposed to .*.

Since URLSearchParams uses iterables, I also had to add dom.iterable to the tsconfig.json file to get the tests passing. Since I am new to TypeScript I am not sure what the implication might be for the final sapper client-side bundle.

I have tested the IE11 code by disabling the URLSearchParams block. Are IE11 tests executed automatically by the CI pipeline before pushing a release?

Copy link
Contributor

@ehrencrona ehrencrona left a comment

Choose a reason for hiding this comment

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

This makes a lot of sense to me!

I'm fairly sure adding dom.iterable to the tsconfig.json is harmless.

@benmccann
Copy link
Member

There are several potentially related bugs open in the tracker: #1179 #1174 #1002. Would this PR fix any of them?

This seems like a good fix. I'm slightly hesitant to merge it because it will make Sapper's code diverge more from SvelteKit's. I expect SvelteKit would not hit this bug because it uses UrlSearchParams. But I think overall it's worth it because it adds a test and it'd be nice to eventually port that test to SvelteKit to prevent a regression there

@benmccann benmccann changed the title Runtime router fails parsing query parameters with encoded line feeds/carriage returns ↵. Fix runtime router handling of encoded query parameters Dec 14, 2020
@benmccann benmccann merged commit 07b97ef into sveltejs:master Dec 14, 2020
@bermi bermi deleted the parsing-query-params branch December 17, 2020 15:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants