Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(idn): encode url except domain #68

Merged
merged 2 commits into from Nov 5, 2019

Conversation

@curbengh
Copy link
Contributor

curbengh commented Sep 8, 2019

Compatibility with hexo v4 (hexojs/hexo#3708), while maintaining backward compatibility with v3, #74 is not compatible with v3 nor IDN.
Fixes hexojs/hexo-generator-feed#63

Supersede #66, #74

test/index.js Outdated Show resolved Hide resolved
@coveralls

This comment has been minimized.

Copy link

coveralls commented Sep 8, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 66c708f on curbengh:idn-handling into 57c2106 on hexojs:master.

@tomap

This comment has been minimized.

Copy link
Contributor

tomap commented Sep 8, 2019

Your title is wrong. Idn should not be encoded to punycode. Ever. Just the path should be encoded.

@curbengh curbengh changed the title fix(idn): idn should be encoded to punycode fix(idn): encode path except domain Sep 8, 2019
@curbengh curbengh force-pushed the curbengh:idn-handling branch from 0ee9d1e to 1313ba4 Sep 8, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Sep 8, 2019

Your title is wrong. Idn should not be encoded to punycode. Ever. Just the path should be encoded.

Updated title and commit message.

I think you might have confused percent-encoding with punycode. IDN must not be encoded to percent-encoding, but it can be encoded to punycode even though it's not required.


btw, don't merge this until we can figure out the failing test.

@curbengh curbengh changed the title fix(idn): encode path except domain fix(idn): encode url except domain Sep 8, 2019
@curbengh curbengh changed the title fix(idn): encode url except domain WIP fix(idn): encode url except domain Sep 11, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Sep 11, 2019

@curbengh curbengh force-pushed the curbengh:idn-handling branch from 1313ba4 to 0a3d41d Sep 24, 2019
@curbengh curbengh changed the title WIP fix(idn): encode url except domain fix(idn): encode url except domain Sep 24, 2019
@curbengh curbengh requested a review from hexojs/core Sep 24, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Sep 25, 2019

Fixed failing test. Ready to be reviewed.

@SukkaW SukkaW mentioned this pull request Oct 21, 2019
@curbengh curbengh force-pushed the curbengh:idn-handling branch from 0a3d41d to 97b99be Nov 4, 2019
@curbengh curbengh force-pushed the curbengh:idn-handling branch from 97b99be to 66c708f Nov 4, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Nov 4, 2019

@hexojs/core
Ready for review.

@curbengh curbengh requested review from hexojs/core and removed request for hexojs/core Nov 4, 2019
@SukkaW
SukkaW approved these changes Nov 4, 2019
@curbengh curbengh merged commit 6a5559d into hexojs:master Nov 5, 2019
3 checks passed
3 checks passed
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage remained the same at 100.0%
Details
@curbengh curbengh deleted the curbengh:idn-handling branch Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.