Skip to content

fix(encodeURL): correct URL search parameter encoding#413

Merged
yoshinorin merged 2 commits intohexojs:masterfrom
uiolee:searchparam
Mar 22, 2025
Merged

fix(encodeURL): correct URL search parameter encoding#413
yoshinorin merged 2 commits intohexojs:masterfrom
uiolee:searchparam

Conversation

@uiolee
Copy link
Member

@uiolee uiolee commented Feb 8, 2025

check list

  • Add test cases for the changes.
  • Passed the CI test.

Description

fix #411

Additional information

@coveralls
Copy link

coveralls commented Feb 8, 2025

Coverage Status

coverage: 96.875%. remained the same
when pulling e6c067e on uiolee:searchparam
into 1ad96ca on hexojs:master.

@axiac
Copy link

axiac commented Feb 12, 2025

The code currently uses four sets of URL handling components:

The functions/classes in each of these groups work fine together but combining them across groups produces unexpected (and sometimes invalid) results because their functionalities do not overlap.

What's the purpose of this decoding and encoding?
It only introduces confusion and bugs.

I would avoid the operations that should cancel each other (encodeURI(decodeURI(...))) and stick to a single set of functions/classes to manipulate the URLs.
Since url.parse() and url.format() are deprecated and encodeURI()/decodeURI() deal with the whole URL at once and do not play well with decodeURIComponent()/querystring.unescape(), I would suggest to use only class URL to validate, parse and possibly re-encode the input URL correctly.
Its documentation describes what it does and it seems to cover all transformations and fixes that function encodeURL() does when it attempts to fix a badly encoded URL.

@uiolee
Copy link
Member Author

uiolee commented Feb 14, 2025

@axiac

Thanks for your advice.

I also don't know the purpose of some of the code.
They may solve certain problems.
It may also be a historical issue, at least hexo was born before WHATWG URL were added to node.

However, hexo-util is referenced by other packages, so it has to be changed with caution

@SukkaW SukkaW requested review from a team and SukkaW and removed request for SukkaW February 17, 2025 01:13
Copy link
Member

@renbaoshuo renbaoshuo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@yoshinorin yoshinorin merged commit 19a1151 into hexojs:master Mar 22, 2025
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

encodeURL() rewrites the query string incorrectly

6 participants