Fix encoding consistency with sorted and non-sorted URL parameters

[ludofischer]

Fix #149

Currently normalize-url encodes URL parameters when sortQueryParameters is true and leaves them as they are when not. This behaviour is confusing as a user would not expect sorting the parameters to affect the encoding. This PR tries to ensure the search parameters are always decoded, since normalize-url already always decodes the pathname and does not intend to sanitize the URL;

normalize-url/index.js

Line 160 in 6ea4038

urlObject.pathname = decodeURI(urlObject.pathname);

[sindresorhus]

This really sounds like a WHATWG URL bug. It should always be consistent about encoding whether or not .sort() is called. If you can only reproduce this in Node.js, I recommend opening an issue on https://github.com/nodejs/node, otherwise https://github.com/whatwg/url

[sindresorhus]

Since the search components are decoded when .sort() is not called, this will potentially decode encoded character where the encoding is meant to be preserved. You could potentially only decode when !options.sortQueryParameters, but I think the encoding inconsistency is a bug in the URL implementation and it would be risky to depend on it staying like that.

[ludofischer]

This really sounds like a WHATWG URL bug. It should always be consistent about encoding whether or not .sort() is called. If you can only reproduce this in Node.js, I recommend opening an issue on https://github.com/nodejs/node, otherwise https://github.com/whatwg/url

I can reproduce it in Firefox and Chrome too! And it seems the issue is well known: whatwg/url#491:

And any attempts to manipulate it will change the contents of your URL's query string in unintended ways

[ludofischer]

See also whatwg/url#478 where they decide to keep the current searchParams behaviour.
Given that searchParams modifies the input by design and the working group does not seem to want to change it, would it be preferable to avoid using it altogether?

[sindresorhus]

Given that searchParams modifies the input by design and the working group does not seem to want to change it, would it be preferable to avoid using it altogether?

There's no easy way to avoid it as we need to keep the sort support and I don't want to bring in another library just for that.

[sindresorhus]

You could potentially only decode when !options.sortQueryParameters

I guess this is the best option then.

[ludofischer]

You could potentially only decode when !options.sortQueryParameters

I guess this is the best option then.

I've pushed an additional commit to check if the parameters have been sorted before decoding.

[psamusev]

The current behavior of
urlObject.pathname = decodeURI(urlObject.pathname);
doesn't work as expected. Since URL automatically always encode any value that we set, according to https://developer.mozilla.org/en-US/docs/Web/API/URL

URLs are encoded according to the rules found in [RFC 3986](https://datatracker.ietf.org/doc/html/rfc3986)

[sindresorhus]

@psamusev It's better to open a new issue instead of commenting on an old pull request. Submitting a failing test would be even better.