Use decodeURIComponent in recognize method #67
Conversation
| @@ -30,6 +30,16 @@ test("A unicode route recognizes", function() { | |||
| equal(router.recognize("/uniçø∂∑"), null); | |||
| }); | |||
|
|
|||
| test("A unicode with special char route recognizes", function() { | |||
There was a problem hiding this comment.
This isn't a unicode test, this is RFC 3986 special cases. Write tests for all of them: *, !:
There was a problem hiding this comment.
encodeURIComponent("?#[]@!:$&'()*+,/=;") returns
"%3F%23%5B%5D%40!%3A%24%26'()*%2B%2C%2F%3D%3B"
The characters that gets encoded is: ? # [ ] @ : $ & ' + , / =;
so ! ' ( ) * are characters that does not get encoded.
|
I haven't looked too deeply, but seems like it might be related to or a duplicate of #55. Can you check? |
|
@rwjblue Not really related, although both PR address the issue encoding and decoding the special cases characters. |
|
|
||
| var encoded = "/some/" + encodeURIComponent("reserved?#@:$&+,/=;characters"); | ||
| resultsMatch(router.recognize(encoded), [{ handler: handler, params: {}, isDynamic: false }]); | ||
| equal(router.recognize("/some/eserved?#@:$&+,/=;"), null); |
There was a problem hiding this comment.
Is the misspelling of reserved intentional? Not sure what this is testing exactly...
|
This PR only tests what happens inside of the fragment part of a URL, which the route-recognizer doesn't attempt to deal with. You should add another assert that doesn't have a > new URL("http://localhost/?@:$&+,/=;characters").search
"?@:$&+,/=;characters"
> new URL("http://localhost/?#@:$&+,/=;characters").hash
"#@:$&+,/=;characters" |
|
Superseded by #55 which is fixing all of the encoding issues in a more holistic manner. |
This change would allow matching of URI with encoded characters for ":", "/", ";", and "?". (Since decodeURI does not decode these characters, recognize would not match this.).
Also, this keeps the usage of encodeURIComponent/decodeURIComponent consistent. (This is the only instance of decodeURI used)