Closed
Description
Version: affects at least Node 4.4.7, 5.7.1, 6.3.0
Platform: Darwin Kernel Version 15.5.0; root:xnu-3248.50.21~8/RELEASE_X86_64 x86_64
OS: Mac OS X 10.11.5
For a reason I can't understand, a #
symbol in the search
parameter of a URL is formatted inconsistently. The first occurrence (whatever the position) seems to be url-encoded, and subsequent occurrences are not:
$ node
> require('url').format({search:'foo=1#2#3#'})
'?foo=1%232#3#'
> require('url').format({search:'foo=###'})
'?foo=%23##'
> require('url').format({search:'###'})
'?%23##'
This seems to affect Node 4-6 at least, in my testing. It's causing problems in sindresorhus/normalize-url#26.
Activity
imyller commentedon Aug 11, 2016
Yes, because at https://github.com/nodejs/node/blob/master/lib/url.js#L627
and
String.prototype.replace()
documentation states that:imyller commentedon Aug 11, 2016
If all occurrences of
#
must be replaced, the line should be:imyller commentedon Aug 11, 2016
I'd be happy to submit a PR if someone can confirm that all occurrences of
#
should be url-encoded in URL search part.georgecrawford commentedon Aug 11, 2016
Looks like this has already been noticed: https://github.com/nodejs/node/pull/2303/files#diff-bbc5176adff1bbc01acd61bfc85d049fR973. I'm not sure what the status of the various URL improvements is. There seem to be some merges, reverts, and open PRs with conflicts.
Any chance of getting a quick fix in for this?
georgecrawford commentedon Aug 11, 2016
Right - I also don't know what's desired. I just see that a number of different PRs to refactor this code seem to add the
/g
fix.imyller commentedon Aug 11, 2016
Yes, hash-URIs are tricky.
Anything After the First
#
is a considered Fragment Identifier, and it is not immediately clear what should be done with the#
's inside Fragment Identifier.RFC3986 states that:
And formatting of fragment is context dependent:
So, encoding only the first
#
might or might not be considered a bug.georgecrawford commentedon Aug 11, 2016
But I'm explicitly setting this string as the
search
property of the URL. So I'd expect this to be treated as a 'query string', and for all hashes (#
) to be url-encoded so they're not mistaken for a Fragment identifier.MylesBorins commentedon Aug 11, 2016
/cc @jasnell
jasnell commentedon Aug 11, 2016
Definitely a bug, in my opinion. I'll look at this a bit later today and see about getting a PR opened.
jasnell commentedon Aug 11, 2016
(unless you want to go ahead with a PR @imyller :-) ..)
imyller commentedon Aug 11, 2016
I'll open PR soon.
12 remaining items