From 0dc485eb28fb05eafad4dc78c646d917d8d4024a Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 31 Mar 2023 09:04:03 -0400 Subject: [PATCH] url: drop ICU requirement for parsing hostnames PR-URL: https://github.com/nodejs/node/pull/47339 Backport-PR-URL: https://github.com/nodejs/node/pull/48345 Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Rich Trott --- deps/ada/ada.gyp | 19 ++----------------- doc/api/url.md | 25 +++++++++++++++---------- lib/internal/idna.js | 9 ++------- test/benchmark/test-benchmark-url.js | 14 +------------- test/wpt/status/url.json | 21 --------------------- 5 files changed, 20 insertions(+), 68 deletions(-) diff --git a/deps/ada/ada.gyp b/deps/ada/ada.gyp index 1171e8750755e1..55cea0033e4fbd 100644 --- a/deps/ada/ada.gyp +++ b/deps/ada/ada.gyp @@ -1,6 +1,7 @@ { 'variables': { 'v8_enable_i18n_support%': 1, + 'ada_sources': [ 'ada.cpp' ], }, 'targets': [ { @@ -10,23 +11,7 @@ 'direct_dependent_settings': { 'include_dirs': ['.'], }, - 'sources': ['ada.cpp'], - 'conditions': [ - ['v8_enable_i18n_support==0', { - 'defines': ['ADA_HAS_ICU=0'], - }], - ['v8_enable_i18n_support==1', { - 'dependencies': [ - '<(icu_gyp_path):icui18n', - '<(icu_gyp_path):icuuc', - ], - }], - ['OS=="win" and v8_enable_i18n_support==1', { - 'dependencies': [ - '<(icu_gyp_path):icudata', - ], - }], - ] + 'sources': [ '<@(ada_sources)' ] }, ] } diff --git a/doc/api/url.md b/doc/api/url.md index 315f01aaafbb71..b82f9e1dd38452 100644 --- a/doc/api/url.md +++ b/doc/api/url.md @@ -129,6 +129,13 @@ return `true`. #### `new URL(input[, base])` + + * `input` {string} The absolute or relative input URL to parse. If `input` is relative, then `base` is required. If `input` is absolute, the `base` is ignored. If `input` is not a string, it is [converted to a string][] first. @@ -172,9 +179,6 @@ const myURL = new URL('https://測試'); // https://xn--g6w251d/ ``` -This feature is only available if the `node` executable was compiled with -[ICU][] enabled. If not, the domain names are passed through unchanged. - In cases where it is not known in advance if `input` is an absolute URL and a `base` is provided, it is advised to validate that the `origin` of the `URL` object is what is expected. @@ -1029,6 +1033,10 @@ for (const [name, value] of params) { added: - v7.4.0 - v6.13.0 +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/47339 + description: ICU requirement is removed. --> * `domain` {string} @@ -1039,9 +1047,6 @@ invalid domain, the empty string is returned. It performs the inverse operation to [`url.domainToUnicode()`][]. -This feature is only available if the `node` executable was compiled with -[ICU][] enabled. If not, the domain names are passed through unchanged. - ```mjs import url from 'node:url'; @@ -1070,6 +1075,10 @@ console.log(url.domainToASCII('xn--iñvalid.com')); added: - v7.4.0 - v6.13.0 +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/47339 + description: ICU requirement is removed. --> * `domain` {string} @@ -1080,9 +1089,6 @@ domain, the empty string is returned. It performs the inverse operation to [`url.domainToASCII()`][]. -This feature is only available if the `node` executable was compiled with -[ICU][] enabled. If not, the domain names are passed through unchanged. - ```mjs import url from 'node:url'; @@ -1725,7 +1731,6 @@ console.log(myURL.origin); // Prints https://xn--1xa.example.com ``` -[ICU]: intl.md#options-for-building-nodejs [Punycode]: https://tools.ietf.org/html/rfc5891#section-4.4 [WHATWG URL]: #the-whatwg-url-api [WHATWG URL Standard]: https://url.spec.whatwg.org/ diff --git a/lib/internal/idna.js b/lib/internal/idna.js index 8591226d104d3a..566f8590d8755c 100644 --- a/lib/internal/idna.js +++ b/lib/internal/idna.js @@ -1,9 +1,4 @@ 'use strict'; -if (internalBinding('config').hasIntl) { - const { toASCII, toUnicode } = internalBinding('icu'); - module.exports = { toASCII, toUnicode }; -} else { - const { domainToASCII, domainToUnicode } = require('internal/url'); - module.exports = { toASCII: domainToASCII, toUnicode: domainToUnicode }; -} +const { domainToASCII, domainToUnicode } = require('internal/url'); +module.exports = { toASCII: domainToASCII, toUnicode: domainToUnicode }; diff --git a/test/benchmark/test-benchmark-url.js b/test/benchmark/test-benchmark-url.js index f4eb4efa234599..664e7c4d8dc827 100644 --- a/test/benchmark/test-benchmark-url.js +++ b/test/benchmark/test-benchmark-url.js @@ -1,18 +1,6 @@ 'use strict'; -const common = require('../common'); - -// TODO(@anonrig): Remove this check when Ada removes ICU requirement. -if (!common.hasIntl) { - // A handful of the benchmarks fail when ICU is not included. - // ICU is responsible for ignoring certain inputs from the hostname - // and without it, it is not possible to validate the correctness of the input. - // DomainToASCII method in Unicode specification states which characters are - // ignored and/or remapped. Doing this outside of the scope of DomainToASCII, - // would be a violation of the WHATWG URL specification. - // Please look into: https://unicode.org/reports/tr46/#ProcessingStepMap - common.skip('missing Intl'); -} +require('../common'); const runBenchmark = require('../common/benchmark'); diff --git a/test/wpt/status/url.json b/test/wpt/status/url.json index 76a56633c580bd..712ad96680a323 100644 --- a/test/wpt/status/url.json +++ b/test/wpt/status/url.json @@ -1,13 +1,8 @@ { - "toascii.window.js": { - "requires": ["small-icu"] - }, "percent-encoding.window.js": { - "requires": ["small-icu"], "skip": "TODO: port from .window.js" }, "historical.any.js": { - "requires": ["small-icu"], "fail": { "expected": [ "URL: no structured serialize/deserialize support", @@ -15,26 +10,10 @@ ] } }, - "urlencoded-parser.any.js": { - "requires": ["small-icu"] - }, - "url-constructor.any.js": { - "requires": ["small-icu"] - }, - "url-origin.any.js": { - "requires": ["small-icu"] - }, - "url-setters.any.js": { - "requires": ["small-icu"] - }, "url-setters-a-area.window.js": { "skip": "already tested in url-setters.any.js" }, - "IdnaTestV2.window.js": { - "requires": ["small-icu"] - }, "javascript-urls.window.js": { - "required": ["small-icu"], "skip": "requires document.body reference" } }