page.js
page.js copied to clipboard
visionmedia
param with slash
Hi, I encountered an issue in my search page.
I have router like : /search/:keyword/, when keyword including a slash, eg: /seach/markdown%20%2F%20wiki, the regexp will match the item before slash.
I find there is a decodeURIComponent call at https://github.com/visionmedia/page.js/blob/master/page.js#L437.
What's the concern to decode the url when doing match? Can we just remove it? :D
@kethinov haha! Github works pretty well for me, but if GitHub will be blocked I have VPN :D
The URL decoding function (decodeURIComponent) is a convenience that frees the user from having to put in place custom URL decoding logic. This is handy, for example, when submitting a form with input fields that contain whitespace and/or special charaters (which will be encoded in x-www-form-urlencoded format by the browser) that matches any active page.js route. Without the URL decoding function, the user would have to decode the query parameters at the application level.
This issue can be fixed by double-encoding the keyword parameter in the route, or by making the URL decoding functionality opt-in in the page.js start function. The way I see it, it's more useful to keep the URL decoding functionality around, because in most cases automatic URL decoding is preferable. The best solution seems to be adding an option to opt out of URL decoding when the router is started (by default, URL decoding would be enabled). For example:
page.start({decode: false});
would disable automatic URL decoding at router startup. I'll push the change in soon, hang in there!
@hheiskanen double-encoding would work fine for now.Thanks for your detail explanation, I will wait for the update.
I just created a PR that makes URL decoding opt-out: https://github.com/visionmedia/page.js/pull/189
I modified my PR such that opting out of URL decoding is no longer necessary to fix the route matching issue. URL decoding logic will not affect route matching logic, and only dictates whether URL path components (query string, pathname, hash) and route params are decoded or not.
@hheiskanen thanks for the updating. I update page.js from 1.4.1 to 1.5.0 from bower registry. But the double-encoding do not work now. Also url like /search/markdown%20%2F%20wiki couldn't fire the router /search/:keyword/:page?, Could you help me out with this? thanks again.
@daddybh yes, I also noticed a route matching issue that was caused by an earlier change to decode all URL paths before the route matching function gets to run. Essentially, in version 1.5.0, each URL is decoded twice as part of the route matching logic: first when adding a new history entry, and a second time when doing the actual route matching. My earlier pull request will fix this issue once it's merged. In the meantime, I suggest you downgrade back to 1.4.1 or pile on more encoding layers. Once the PR is merged, double encoding should work again.
@shuvalov-anton An external plugin is not necessary, as long as the user doesn't mind double-encoding problematic URL's (because route matching strips one encoding layer away). I think it's ok to stick with the current behavior, or try to remove the decoding function call at https://github.com/visionmedia/page.js/blob/master/page.js#L437 altogether. However, merging https://github.com/visionmedia/page.js/pull/189 is needed to make double-encoding work again without the need to manually decode URL path components in the user's code.
@shuvalov-anton I added some tests that cover the basic URL decoding cases in https://github.com/visionmedia/page.js/pull/194.
@daddybh Now that the previous PR is merged, your original issue with slashes still persists, but can be worked around by double-encoding. I hope this will do for you -- removing the decodeURIComponent call at https://github.com/visionmedia/page.js/blob/master/index.js#L442 is a little trickier than it seems and causes route matching to get stuck in a loop with some paths. I recommend we keep it around for now, and use double-encoding when a route contains slashes.
@hheiskanen I downgraded to version 1.4.1 for now. Double-encoding work fine in most cases, except keyword include a percent sign %, when I double-decode the url, there is a Uncaught URIError URI malformed, I still looking at this issue right now. I will give it a try tomorrow of the latest update. Thanks for the update.
@shuvalov-anton I just upgrade to 1.6.0, but still have problem after I removed all double-encoding codes.
I found out there is a native decodeURIComonent call at https://github.com/visionmedia/page.js/blob/master/page.js#L484.
Since pathname have been decoded by function decodeURLEncodedURIComponent, can we remove it now ?
@daddybh you still need to double-encode URL's that contain slashes -- the new decodeURLComponents option only decodes the URL segments as contained in the page.js Context object, but this does not affect route matching. If the URL decoding was removed before hitting the native decodeURIComponent at https://github.com/visionmedia/page.js/blob/master/page.js#L484 route matching would get stuck in a loop with some routes that contain special characters. Fixing this might require a fundamental rewrite of the route matching logic.
Has decodeURLComponents: false regressed? I have a route /foo/:id and if the URL is /foo/xy%2Fz it doesn't match, but /foo/xyz/ does
+1 for allowing URI encoded URIs as parameters in URLs. I'm currently implementing a W3C specification which uses URI encoded URIs as IDs in URLs and this is proving to be a bit of a problem.
E.g. https://mythingdirectory.com/things/https%3A%2F%2Fmythingserver.com%2Fthings%2Fthing1 needs to match /things/:id
As I understand it currently the only way to achieve this with page.js is to double encode the ID with encodeURIComponent(encodeURIComponent(id)) but that means the URLs on the front end have to be different from the URLs on the back end API, which is very confusing!
