access domains not handled properly on NodeJS 18

[breautek]

Bug Report

Problem

Unit test fails on NodeJS 18, because the values returned by URL.parse has changed in NodeJS 18.

https://github.com/apache/cordova-ios/blame/master/lib/prepare.js#L1029

As of NodeJS 18, the parsed URL now contains .hostname property that is a non-empty string, whereas older Node versions returned hosname as an empty string, when given a URL that contains a wildcard such as https://*.test.com.

Here is a summary of how the parsed URL has changed going into Node 18.

v16.14.0 -> v18.14.1

var URL = require('url')
var href = 'http://*.test';
Url {
 protocol: 'http:',
 slashes: true,
 auth: null,
 host: '',----------------> '*.test'
 port: null,
 hostname: '',------------> '*.test'
 hash: null,
 search: null,
 query: null,
 pathname: '/*.test',-----> '/'
 path: '/*.test',---------> '/'
 href: 'http:///*.test'---> 'http://*.test/'
}

As a result, the final object when testing https://github.com/apache/cordova-ios/blame/master/tests/spec/unit/prepare.spec.js#L864

fails because the key is *.server01.com instead of the expected server01.com

Version information

cordova-ios master NodeJS 18

[TylerBreau]

The code mentions URL.parse being deprecated.

function parseAllowlistUrlForATS (url, options) {
    // @todo 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead.
    const href = URL.parse(url); // eslint-disable-line

Here's a summary of the differences between URL.parse and URL.URL constructor.

v16.14.0 (URL.parse) -> v18.14.1 (new URL.URL)

var URL = require('url')
var href = 'http://*.test';
URL.parse -> new URL.URL(href)
Url {
 protocol: 'http:',
 slashes: true,-----------> undefined
 auth: null,--------------> undefined
 host: '',----------------> '*.test'
 port: null,
 hostname: '',------------> '*.test'
 hash: null,
 search: null,
 query: null,-------------> undefined
 pathname: '/*.test',-----> '/'
 path: '/*.test',---------> undefined
 href: 'http:///*.test'---> 'http://*.test/'

 v18 introduces the following properties
 origin: 'http://*.test',
 username: '',
 password: '',
 searchParams: URLSearchParams {},
}

[dpogue]

The WHATWG URL API looks like it was added to NodeJS in Node 10, so we should be safe to switch over to using that.

[breautek]

The WHATWG URL API looks like it was added to NodeJS in Node 10, so we should be safe to switch over to using that.

As long as it's consistent then it would probably be the best path forward, even if it still requires some refactoring.

Which appears to be the case:

Node 10 (Planned to be dropped, but included for reference/context)

> new URL("https://*.test.com")
URL {
  href: 'https://*.test.com/',
  origin: 'https://*.test.com',
  protocol: 'https:',
  username: '',
  password: '',
  host: '*.test.com',
  hostname: '*.test.com',
  port: '',
  pathname: '/',
  search: '',
  searchParams: URLSearchParams {},
  hash: '' }

Node 16:

new URL("https://*.test.com")
URL {
  href: 'https://*.test.com/',
  origin: 'https://*.test.com',
  protocol: 'https:',
  username: '',
  password: '',
  host: '*.test.com',
  hostname: '*.test.com',
  port: '',
  pathname: '/',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}

Node 18:

new URL("https://*.test.com")
URL {
  href: 'https://*.test.com/',
  origin: 'https://*.test.com',
  protocol: 'https:',
  username: '',
  password: '',
  host: '*.test.com',
  hostname: '*.test.com',
  port: '',
  pathname: '/',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}

But I do not understand enough of the code to know what is proper, like do we need to parse out the *. from the hostname, etc...

[NiklasMerz]

But I do not understand enough of the code to know what is proper, like do we need to parse out the *. from the hostname, etc...

Same for me. Updating the code to the URL constructor breaks 32 tests and definitely changes the logic a lot. Not that easy refactor I guess.

Do we aim for a quick fix for a patch release or permament solution? Any ideas for a pragmatic fix?