leave more characters unescaped for segments

[Guria]

I am using path-to-regexp in url-mapper which allows to map almost any json compatible object to url with path and query. The key feature is preserving type of values, so assert.equal(parse(stringify(object)), object).
Routes defined with path-to-regexp format, keys of object defined in route stringified with path-to-regexp, rest of keys goes to query part with help of urlon.

Tonic demo.

url-mapper allows map only simple values (strings, numbers and booleans) to path parameters. To allow preserving type for numbers and booleans they are prepended with colon character. Here is where issue starts: path-to-regexp encodes segments with encodeURIComponent function and converts colons to '%3A', which makes url to look more cryptic in path part.

Tonic demo

This PR tries to address this problem by treat ;,:@&=+$-_.!~*() characters as safe to be part of segment. According to MDN:

To avoid unexpected requests to the server, you should call encodeURIComponent on any user-entered parameters that will be passed as part of a URI.

It means there is no need to use encodeURIComponent to encode segment part, because it's too strict. I have introduced extended version on encodeURI with escaping /?#'" characters and added some tests as well.

I am suppose, that it should be treated as major change because it potentially could break matching and expectations.

[blakeembrey]

@Guria So what's the core issue here? It was a lot of text, but the expectation is that escaping is looser (to make it prettier with the colon)? I have no strong opinions, but I'd like to get another set of eyes on it first.

[Guria]

@blakeembrey core issue was here: https://tonicdev.com/guria/5715e701d286771100079fb2

[Guria]

I'd like to get another set of eyes on it first.

Of course, no hurry here. We have a recommendation on how to deal with it to avoid %3A to appear in url. It is nice to have feature, so I don't mind that it will take some time to prove it harmless.

Also I was think about keep current escaping behaviour and introduce less strict as option. That's way it could be introduced without any harm to existing users.

[maxfrigge]

I personally think that readability in urls is a good thing - even if it just creates joy amongst developers :)

Don't really see why it should break matching though?

[blakeembrey]

@maxfrigge Any change to a public facing API is a breaking change, someone may be relying on (however unlikely) the fact that : becomes %3A currently. An option allows this to be a minor opt-change, however.

@Guria You could also manage stringify yourself, for now - it's pretty simple to do and the current code doing it was very trivial to implement - just loop over the parsed string and concat together throwing errors on invalid data.

[blakeembrey]

@dougwilson Any thoughts from you here?

I'm not particularly keen on merging, it feels like a simple application left to user-land for now. That said, it is a very trivial fix and the only possible bugs I can think it may introduce on users is if they are parsing generated URLs (it'd be possible to confuse the URL parser if it was previously a relative URL segment at the beginning by putting a colon in the segment).

[Guria]

Ohh, thanks for pointing that out. Seems I really can use own tokensToFunction implementation. Though it will be almost completely copypasted.

So no we have several options here:

• accept fix as is to be available for every user. may require major release bump.
• rewrite fix to be optional: pathToRegexp.compile(route, { softEscaping: true })
• leave path-to-regexp as is and patched stringify function factory on my side: softTokensToFunction(parse(route))

I can start from 3rd option now, but will leave a pr open in case you will decide that this fix might be useful for other users too.

[dougwilson]

@blakeembrey so I have seen various people (online and in person) who do not like the encodeURIComponent behavior, simply because it's less aesthetically pleasing. Of course, the proposed reduction in this PR is technically valid (I believe) because when considered in the full URL context, those entities are only suggested to be encoded. Encoding them helps prevent ambiguity when you only have a fragment of the URL (which you pointed out), at the expensive of less aesthetics.

That said, I would suggest keeping the current safe behavior, but do not see a reason why adding an option for this new behavior would be bad (since it's optional, and more importantly, opt-in). I would leave the ultimate decision to you, but that is my opinion :)

[blakeembrey]

Merged as an option. Use { pretty: true } to get prettier URL outputs. Thanks @Guria! 😄