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.

Sign up for GitHub

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

Jump to bottom

(v6.x backport) url: fix surrogate handling in encodeAuth() #11387

Closed
wants to merge 1 commit into from
Closed

(v6.x backport) url: fix surrogate handling in encodeAuth() #11387

wants to merge 1 commit into from

Conversation

TimothyGu
Copy link
Member

@TimothyGu TimothyGu commented Feb 14, 2017 •
edited
Loading

Currently, the legacy URL stringifier miscalculates the offset of an extra surrogate, causing the high surrogate to be included unescaped:

> url.format({ auth: '\uD83D\uDE00', hostname: 'a' })
'//%F0%9F%98%80�@a'

Backport of #11161.

CI: https://ci.nodejs.org/job/node-test-pull-request/6413/

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

url

@nodejs-github-bot nodejs-github-bot added url Issues and PRs related to the legacy built-in url module. v6.x labels Feb 14, 2017
@TimothyGu TimothyGu mentioned this pull request Feb 21, 2017
Merged
@MylesBorins
Copy link
Contributor

@TimothyGu can you please rebase this and preserve the original commit message from #11161

@TimothyGu
url: fix surrogate handling in encodeAuth()
8100f59 
Currently, the legacy URL stringifier miscalculates the offset of an
extra surrogate, causing the high surrogate to be included unescaped:

    > url.format({ auth: '\uD83D\uDE00', hostname: 'a' })
    '//%F0%9F%98%80�@A'

PR-URL: nodejs#11387
Backport-of: nodejs#11161
@TimothyGu
Copy link
Member Author

@MylesBorins, done.

CI: https://ci.nodejs.org/job/node-test-pull-request/6698/

@TimothyGu
Copy link
Member Author

@MylesBorins PTAL...

MylesBorins pushed a commit that referenced this pull request Mar 8, 2017
@TimothyGu @MylesBorins
url: fix surrogate handling in encodeAuth()
4fee955 
Currently, the legacy URL stringifier miscalculates the offset of an
extra surrogate, causing the high surrogate to be included unescaped:

    > url.format({ auth: '\uD83D\uDE00', hostname: 'a' })
    '//%F0%9F%98%80�@A'

PR-URL: #11387
Backport-of: #11161
@MylesBorins
Copy link
Contributor

looks like a single failure on arm

https://ci.nodejs.org/job/node-test-binary-arm/6510/RUN_SUBSET=3,label=pi1-raspbian-wheezy/tapTestReport/test.tap-179/

not ok 179 sequential/test-regress-GH-897

landed in 4fee955

@MylesBorins MylesBorins closed this Mar 8, 2017
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
@TimothyGu @MylesBorins
url: fix surrogate handling in encodeAuth()
9e6fcbb 
Currently, the legacy URL stringifier miscalculates the offset of an
extra surrogate, causing the high surrogate to be included unescaped:

    > url.format({ auth: '\uD83D\uDE00', hostname: 'a' })
    '//%F0%9F%98%80�@A'

PR-URL: #11387
Backport-of: #11161
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
Merged
@TimothyGu TimothyGu deleted the url-surr-v6 branch March 14, 2017 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TimothyGu @MylesBorins @nodejs-github-bot