search

sindresorhus / normalize-url

Normalize a URL
MIT License
840 stars 122 forks source link

Normalize encoding #44

Closed Ricardo-Silva91 closed 5 years ago

Ricardo-Silva91 commented 7 years ago

Fixes #43: '@' symbol not being decoded from '%40' used the decodeURIComponent() method. Maybe it's too general but, since the code still passes all previous tests, I suppose it's preferable to just replacing the '@' character with str.replace(/%40/, '@'), as it can prevent future similar issues.

stevenvachon commented 7 years ago

decodeURIComponent() can result in issues as it's been around, mostly untouched (supposedly) since JavaScript's beginnings. It might be best to wait to fix this along with #45. ~If not, this PR looks fine otherwise.~~

stevenvachon commented 7 years ago

I just noticed something:

normalizeUrl('http://host/?var1=va%26lue&var2=value');

results in

http://host/?var1=va&lue&var2=value

which is a different URL.

Ricardo-Silva91 commented 7 years ago

Hello again. The latest commit has a fix for the bug @stevenvachon noticed (character '&' inside a parameter splits the parameter in 2). With this hotfix, the complexity of the function increased to 23, not sure if that's a problem. I also added a test for the bug.

sindresorhus commented 5 years ago

Closing as we no longer decode the URL.