-
Notifications
You must be signed in to change notification settings - Fork 43
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
When should percent decoding occur by default with paths? #42
Comments
Since an Elm application produces
Current elm/url does not round trip.
If we use the auto encode and auto decode versions
We can try it out in this tiny app https://elm-url-test.netlify.app (source)
The function names would best be renamed to avoid this problem
|
using strings as-is, vuejs mostly round trips properly <router-link to="/path/foo?query=foo">Go to foo</router-link>
<router-link to="/path/👍?query=👍">Go to 👍</router-link>
<router-link :to="{ name: 'pathRoute', params: { id: '!@#$%^&*()' }}">Go to !@#$%^&*()</router-link>
<router-link to="/path/{}[]<>;/?query=/{}[]<>;/">Go to {}[]<>;/</router-link>
<router-link to="/path/日本?query=日本">Go to 日本</router-link>
<router-link to="/path/خحجث?query=خحجث">Go to خحجث</router-link> the generated href attribute values are escaped
|
using strings as-is, react router mostly round trips properly. <li><Link to={{ pathname: "/path/foo", search: "?query=foo" }}>foo</Link></li>
<li><Link to={{ pathname: "/path/👍", search: "?query=👍" }}>👍</Link></li>
<li><Link to={{ pathname: "/path/!@#$%^&*()", search: "?query=!@#$%^&*()" }}>!@#$%^&*()</Link></li>
<li><Link to={{ pathname: "/path/" + encodeURI("!@#$%^&*()"), search: "?query=!@#$%^&*()" }}>!@#$%^&*()</Link> fixed</li>
<li><Link to={{ pathname: "/path/{}[]<>;", search: "?query={}[]<>;" }}>{}[]<>;</Link></li>
<li><Link to={{ pathname: "/path/日本", search: "?query=日本" }}>日本</Link></li>
<li><Link to={{ pathname: "/path/خحجث", search: "?query=خحجث" }}>خحجث</Link></li> https://codesandbox.io/s/react-router-url-parameters-forked-sno3x?file=/example.js Array.from(document.getElementsByTagName('a')).forEach(function(a) { console.log(a.href) }) running the above js in codesandbox's own console (bottom right), you'll notice most of the
|
So autoencoding [very sorry this came in as multiple comments. I didn't know how much time I had or needed] |
Problem
Some issues on this repo are saying that more decoding should happen on path segments by default:
Issue #20 also says that this decoding should not occur in all cases, referencing this issue that points to implementations in many languages that do not call
percentDecode
in a naive way.What should be done?
Current Status
As mentioned in #16, it is currently possible to use
Url.Parser.custom
to create custom path parsers that have whatever behavior you personally think is best. For example,custom "STRING" Url.percentDecode
would do very aggressive percent decoding.So nobody is blocked on this topic. It is a question of defaults, and any change should be considered a breaking change that triggers a MAJOR version bump.
Goal
Make a table of scenarios to try to find the ideal defaults for a broad range of people. The default options for path parsing and building are:
We currently do (1) but maybe it'd be good to make a table to show the various options. Right now I think it would be ideal for someone interested in this topic to:
Hopefully this will reveal more information / some sort of consensus. I think being thorough about this is very important, since any change here could break a lot of peoples' code in ways that might be very hard to detect. Please share your efforts here or on the Elm Discourse.
The text was updated successfully, but these errors were encountered: