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

Backport WHATWG URL to 6.x #17365

Closed
wants to merge 4 commits into from
Closed

Conversation

TimothyGu
Copy link
Member

Includes all commits related to the WHATWG URL parser up until now. To ease backporting, many changes to url documentation not directly related to the new parser are also included. For compatibility concerns, the new url.format(whatwgURLObj) overload and WHATWG URL support in fs and http are not backported.

PRs #11464, #9246, #13362, which the current WHATWG URL implementation requires, are backported in separate commits as they are logically separate, even if the commits themselves were made after the WHATWG parser landed and contained changes to it.

Fixes: nodejs/Release#208

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

url

TimothyGu and others added 4 commits November 27, 2017 21:26
- Add IsInvalidated() method
- Add capacity() method for finding out the actual capacity, not the
  current size, of the buffer
- Make IsAllocated() work for invalidated buffers
- Allow multiple calls to AllocateSufficientStorage() and Invalidate()
- Assert buffer is malloc'd in Release()
- Assert buffer has not been invalidated in AllocateSufficientStorage()
- Add more descriptive comments describing the purpose of the methods
- Add cctest for MaybeStackBuffer

PR-URL: nodejs#11464
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#9246
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Remove custom tests for invalid IDNA domains in url-idna.js in favor of
the more comprehensive official set.

PR-URL: nodejs#13362
Refs: whatwg/url#309
Refs: web-platform-tests/wpt#5976
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Implements WHATWG URL support. Example:

```
var u = new url.URL('http://example.org');
```

Many, many other commits improving the implementation have been squashed
into this backport PR. They are not listed separately here for brevity.

PR-URL: nodejs#7448
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. v6.x labels Nov 28, 2017
@TimothyGu TimothyGu added c++ Issues and PRs that require attention from people who are familiar with C++. doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. whatwg-url Issues and PRs related to the WHATWG URL implementation. i18n-api Issues and PRs related to the i18n implementation. labels Nov 28, 2017
@MylesBorins
Copy link
Contributor

Wow!!!

@nodejs/LTS any concerns around this? We will likely have to wait until the next minor release for this

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

Very nice.

@addaleax addaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Nov 30, 2017
@TimothyGu
Copy link
Member Author

Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

LGTM with a green CI

@MylesBorins
Copy link
Contributor

MylesBorins commented Nov 30, 2017 via email

@MylesBorins
Copy link
Contributor

@TimothyGu
Copy link
Member Author

@MylesBorins
Copy link
Contributor

MylesBorins pushed a commit that referenced this pull request Jan 18, 2018
- Add IsInvalidated() method
- Add capacity() method for finding out the actual capacity, not the
  current size, of the buffer
- Make IsAllocated() work for invalidated buffers
- Allow multiple calls to AllocateSufficientStorage() and Invalidate()
- Assert buffer is malloc'd in Release()
- Assert buffer has not been invalidated in AllocateSufficientStorage()
- Add more descriptive comments describing the purpose of the methods
- Add cctest for MaybeStackBuffer

Backport-PR-URL: #17365
PR-URL: #11464
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

ARM failures are unrelated

landed in 3c4bb3c...3639d0c

MylesBorins pushed a commit that referenced this pull request Jan 18, 2018
Backport-PR-URL: #17365
PR-URL: #9246
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Jan 18, 2018
Remove custom tests for invalid IDNA domains in url-idna.js in favor of
the more comprehensive official set.

Backport-PR-URL: #17365
PR-URL: #13362
Refs: whatwg/url#309
Refs: web-platform-tests/wpt#5976
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 18, 2018
Implements WHATWG URL support. Example:

```
var u = new url.URL('http://example.org');
```

Many, many other commits improving the implementation have been squashed
into this backport PR. They are not listed separately here for brevity.

Backport-PR-URL: #17365
PR-URL: #7448
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
@TimothyGu TimothyGu deleted the v6.x-url branch February 1, 2018 00:53
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
- Add IsInvalidated() method
- Add capacity() method for finding out the actual capacity, not the
  current size, of the buffer
- Make IsAllocated() work for invalidated buffers
- Allow multiple calls to AllocateSufficientStorage() and Invalidate()
- Assert buffer is malloc'd in Release()
- Assert buffer has not been invalidated in AllocateSufficientStorage()
- Add more descriptive comments describing the purpose of the methods
- Add cctest for MaybeStackBuffer

Backport-PR-URL: #17365
PR-URL: #11464
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
Backport-PR-URL: #17365
PR-URL: #9246
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
Remove custom tests for invalid IDNA domains in url-idna.js in favor of
the more comprehensive official set.

Backport-PR-URL: #17365
PR-URL: #13362
Refs: whatwg/url#309
Refs: web-platform-tests/wpt#5976
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
Implements WHATWG URL support. Example:

```
var u = new url.URL('http://example.org');
```

Many, many other commits improving the implementation have been squashed
into this backport PR. They are not listed separately here for brevity.

Backport-PR-URL: #17365
PR-URL: #7448
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
- Add IsInvalidated() method
- Add capacity() method for finding out the actual capacity, not the
  current size, of the buffer
- Make IsAllocated() work for invalidated buffers
- Allow multiple calls to AllocateSufficientStorage() and Invalidate()
- Assert buffer is malloc'd in Release()
- Assert buffer has not been invalidated in AllocateSufficientStorage()
- Add more descriptive comments describing the purpose of the methods
- Add cctest for MaybeStackBuffer

Backport-PR-URL: #17365
PR-URL: #11464
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
Backport-PR-URL: #17365
PR-URL: #9246
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
Remove custom tests for invalid IDNA domains in url-idna.js in favor of
the more comprehensive official set.

Backport-PR-URL: #17365
PR-URL: #13362
Refs: whatwg/url#309
Refs: web-platform-tests/wpt#5976
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
Implements WHATWG URL support. Example:

```
var u = new url.URL('http://example.org');
```

Many, many other commits improving the implementation have been squashed
into this backport PR. They are not listed separately here for brevity.

Backport-PR-URL: #17365
PR-URL: #7448
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
- Add IsInvalidated() method
- Add capacity() method for finding out the actual capacity, not the
  current size, of the buffer
- Make IsAllocated() work for invalidated buffers
- Allow multiple calls to AllocateSufficientStorage() and Invalidate()
- Assert buffer is malloc'd in Release()
- Assert buffer has not been invalidated in AllocateSufficientStorage()
- Add more descriptive comments describing the purpose of the methods
- Add cctest for MaybeStackBuffer

Backport-PR-URL: #17365
PR-URL: #11464
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
Backport-PR-URL: #17365
PR-URL: #9246
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
Remove custom tests for invalid IDNA domains in url-idna.js in favor of
the more comprehensive official set.

Backport-PR-URL: #17365
PR-URL: #13362
Refs: whatwg/url#309
Refs: web-platform-tests/wpt#5976
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
Implements WHATWG URL support. Example:

```
var u = new url.URL('http://example.org');
```

Many, many other commits improving the implementation have been squashed
into this backport PR. They are not listed separately here for brevity.

Backport-PR-URL: #17365
PR-URL: #7448
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
nodejs-github-bot pushed a commit that referenced this pull request Nov 13, 2024
PR-URL: #55818
Fixes: #55806
Refs: #11236
Refs: #17365
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
aduh95 added a commit that referenced this pull request Nov 16, 2024
PR-URL: #55818
Fixes: #55806
Refs: #11236
Refs: #17365
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
tpoisseau pushed a commit to tpoisseau/node that referenced this pull request Nov 21, 2024
PR-URL: nodejs#55818
Fixes: nodejs#55806
Refs: nodejs#11236
Refs: nodejs#17365
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. doc Issues and PRs related to the documentations. i18n-api Issues and PRs related to the i18n implementation. lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version. test Issues and PRs related to the tests. whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants