FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

(v6.x backport) url: fix surrogate handling in encodeAuth() by TimothyGu · Pull Request #11387 · nodejs/node · GitHub

/ node Public

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

Closed
TimothyGu wants to merge 1 commit into
nodejs:v6.x-stagingfrom
TimothyGu:url-surr-v6
Closed

(v6.x backport) url: fix surrogate handling in encodeAuth()#11387
TimothyGu wants to merge 1 commit into
nodejs:v6.x-stagingfrom
TimothyGu:url-surr-v6

Conversation

TimothyGu commented Feb 14, 2017
edited
Loading

Copy link
Copy Markdown
Member

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 added url Issues and PRs related to the legacy built-in url module. v6.x labels Feb 14, 2017
TimothyGu mentioned this pull request Feb 21, 2017

Copy link
Copy Markdown
Contributor

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

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

Copy link
Copy Markdown
Member Author

Copy link
Copy Markdown
Member Author

@MylesBorins PTAL...

MylesBorins pushed a commit that referenced this pull request Mar 8, 2017
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

Copy link
Copy Markdown
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 closed this Mar 8, 2017
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
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 mentioned this pull request Mar 9, 2017
TimothyGu deleted the url-surr-v6 branch March 14, 2017 18:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

url Issues and PRs related to the legacy built-in url module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL