Zaid-Ajaj / Feliz.Router

A router component for React and Elmish that is focused, powerful and extremely easy to use.
MIT License
78 stars 16 forks source link

Ampersand characters in query values need better handling #3

Closed rmunn closed 5 years ago

rmunn commented 5 years ago

URL-decoding query strings in onUrlChanged before passing them to user code seems very nice, but there's one scenario you may not have considered: ampersands in query values. E.g., according to the documentation, the URL https://candystore.example.com/placeOrder?candyName=M%26M&quantity=5 would parse to ["placeOrder", "?candyName=M&M&quantity=5"], which would then confuse the Javascript URLSearchParams constructor when the end user tries to use the Route.Query active pattern. (I tested it on Google Chrome, and the result was that candyName had the value "M", and there was another key called M with the value "").

Since URLSearchParams already handles percent-decoding, I think it's better to not decode the query string in onUrlChanged, but just pass it unchanged to the user code, which can then safely use Route.Query to ensure a valid match against something like ["candyName", name] which will then place the string "M&M" into name.

Zaid-Ajaj commented 5 years ago

Aah I see, it looks like the query string parameter is decoded twice, nice catch! I will look into it soon

Zaid-Ajaj commented 5 years ago

Fixed! please re-open if the issue persists