Fix handling of urlencoded '+'

[sprohaska]

Previously, '%2B' was incorrectly converted to space. The reason was
that page.js's Context() calls decodeURLEncodedURIComponent() when
initializing pathname and Route.match() calls
decodeURLEncodedURIComponent() again on the matched params after
splitting pathname. Since decodeURLEncodedURIComponent() replaces
plus with space and the path is already decoded when calling it on the
params, pluses were incorrectly converted to spaces.

page's Route.match() calls decodeURIComponent() to decode pathname
before applying the path regex, so there is no need to decode the entire
path when initializing the Context().

page.js's behavior seems odd. But I haven't fully understood whether it
is a bug, so this fixes it by disabling page.js's
decodeURLEncodedURIComponent() via the config object.

The tests are updated to exercise the param matching with a few
urlencoded special characters.

[arunoda]

Why you need put encoded string here? I think that should be done by the router.

[sprohaska]

I think you are right that the router should encode it. See longer comment below.

[arunoda]

I'm quite not sure about here. Need some voice from others as well. @elidoran @delgermurun

[delgermurun]

What is the problem? What is exact behavior?

[elidoran]

pagejs appears to decode twice as @sprohaska describes. See pagejs line 491 and line 497.

I don't know why they would decode it twice. The first time always happens and the second happens unless you disable it. The second one also explicitly changes pluses to spaces before decoding (here).

I'm curious to know why they do that. Perhaps raise this issue with pagejs and see what they have to say?

It seems like outright disabling it would alter behavior and possibly break backwards compatibility. How about providing a way for a user to specify FlowRouter should disable the extra decoding, and leave it enabled by default.

Also, when I do the calls listed below I get an error: URIError: URI malformed. Should it encode it for us?

• FlowRouter.go('/app/abc +@%')
• FlowRouter.go('/app/abc%20%2B%40%25')
• FlowRouter.go('/app/:param', {param:'abc%20%2B%40%25'})
• FlowRouter.go('/app/home?test=abc +@%')

Without the % and %25 they work fine and all return abc @ (plus sign is replaced with a space).

When I do FlowRouter.go('/app/home?test=abc%20%2B%40%25') it works and the browser shows the encoded version. FlowRouter.getQueryParam('test') returns the decoded version "abc +@%".

So, this seems to require some review to determine what the behavior should actually be.

[sprohaska]

I agree that a good solution may require clarification of the behavior in pagejs.

I'm a bit unsure what I'd exactly expect from flow router. It seems reasonable that whatever I type in the browser's address field should be put into the params as urldecoded strings and I can just use them without any further decoding.

I'm less sure what I'd expect from FlowRouter.go(). One option would be that FlowRouter.go(path) behaves as if I typed path into the browser's address field, which means that it would not urlencode path. But for symmetry with route matching, FlowRouter.go('/app/:param', {param: String}), should probably urlencode the String, since it places decoded strings into params when matching a path, so it should encode them when constructing a path. In combination it would mean that flow router would not encode any direct path argument (assuming that it is already encoded) but would encode strings that are passed as params. I'm unsure whether this is a sensible choice.

Any thoughts?

[sprohaska]

I've updated the commit to implement the urlencoding in FlowRouter.path() as described in my previous comment.

[sprohaska]

Note that encoding of params may be unintuitive for tail params as in /:foo/:bar+. :bar captures all tail directory levels, so /a/b/c/d is matched with :bar = b/c/d. FlowRouter.path('/:foo/:bar+', {foo: 'x', bar: 'b/c/d'}) will not return the original path but /x/b%2Fc%2Fd, which is correct but looks a bit odd.

I'm unsure whether this is acceptable. I'm not fully convinced, but also do not yet have a better proposal.

[arunoda]

I had to do add a few modifications to this.
See this: 750321c

[sprohaska]

It looks correct and works, but it creates ugly URLs. I'm wondering whether flow router could provide an opt-in option to disable page.js URL encoding. It would preserve backward compatibility but also provide nicer URLs by avoiding the page.js weirdness.

I haven't thought about it in more detail. Maybe I propose something when I find more time.