[v18.17] Same URLs are different

[regseb]

Version

v18.17.0

Platform

Linux regseblaptop 5.19.0-46-generic #47~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Wed Jun 21 15:35:31 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

import assert from "node:assert/strict";

const link = "https://baz.org/";
const url = new URL(link);

console.log("===== FIRST =====")
assert.deepEqual(url, new URL(link));

console.log("===== CONSOLE =====")
console.log(url);

console.log("===== SECOND =====")
assert.deepEqual(url, new URL(link));

How often does it reproduce? Is there a required condition?

The error occurs since Node.js v18.17.0.

What is the expected behavior? Why is that the expected behavior?

===== FIRST =====
===== CONSOLE =====
URL{
 href: 'https://baz.org/',
 origin: 'https://baz.org',
 protocol: 'https:',
 username: '',
 password: '',
 host: 'baz.org',
 hostname: 'baz.org',
 port: '',
 pathname: '/',
 search: '',
 searchParams: URLSearchParams{},
 hash: ''
}
===== SECOND =====

What do you see instead?

===== FIRST =====
===== CONSOLE =====
URL{
 href: 'https://baz.org/',
 origin: 'https://baz.org',
 protocol: 'https:',
 username: '',
 password: '',
 host: 'baz.org',
 hostname: 'baz.org',
 port: '',
 pathname: '/',
 search: '',
 searchParams: URLSearchParams{},
 hash: ''
}
===== SECOND =====
node:internal/process/esm_loader:97
 internalBinding('errors').triggerUncaughtException(
 ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected ... Lines skipped

+ <ref *1> URL{
- URL{
 [Symbol(context)]: URLContext{
 hash_start: 4294967295,
...
 search_start: 4294967295,
 username_end: 8
+ },
+ [Symbol(query)]: URLSearchParams{
+ [Symbol(context)]: [Circular *1],
+ [Symbol(query)]: []
 }
 }
 at file:///home/regseb/testcase/index.js:13:8
 at ModuleJob.run (node:internal/modules/esm/module_job:194:25){
 generatedMessage: true,
 code: 'ERR_ASSERTION',
 actual: <ref *1> URL{
 [Symbol(context)]: URLContext{
 href: 'https://baz.org/',
 protocol_end: 6,
 username_end: 8,
 host_start: 8,
 host_end: 15,
 pathname_start: 15,
 search_start: 4294967295,
 hash_start: 4294967295,
 port: 4294967295,
 scheme_type: 2
 },
 [Symbol(query)]: URLSearchParams{
 [Symbol(query)]: [],
 [Symbol(context)]: [Circular *1]
 }
 },
 expected: URL{
 [Symbol(context)]: URLContext{
 href: 'https://baz.org/',
 protocol_end: 6,
 username_end: 8,
 host_start: 8,
 host_end: 15,
 pathname_start: 15,
 search_start: 4294967295,
 hash_start: 4294967295,
 port: 4294967295,
 scheme_type: 2
 }
 },
 operator: 'deepStrictEqual'
}

Node.js v18.17.0

Additional information

• Node.js v18.17.0 comes with the latest version of the URL parser, Ada.
• The difference is found only after console.log().

[debadree25]

cc @nodejs/url

[anonrig]

Calling console.log lazy loads the query parameter and initializes the searchParams causing this indifference when using deepEqual. Unfortunately, I don’t have a solution to this problem without seriously impacting the performance.

[regseb]

I tested with Node.js v20.5.0 and I have no problem.

const url = new URL("http://foo.org");

console.log(Object.getOwnPropertySymbols(url));
url.searchParams;
console.log(Object.getOwnPropertySymbols(url));

Node.js v18.17.0	[ Symbol(context) ] [ Symbol(context), Symbol(query) ]
Node.js v20.5.0	[] []
Chromium 114	[] []

@anonrig Will this problem happen on v20? Or will it only be on v18?

[anonrig]

We use private properties in Node 20 because it is a breaking change coming from using symbols as a private property. That’s why the output is different in Node 18.

[anonrig]

By the way, if you want to do a equality check between url instances, compare their href attributes. That would be the fastest and most direct way.

[aduh95]

Unfortunately, I don’t have a solution to this problem without seriously impacting the performance.

Do you mean using a WeakMap? How bad are we talking about?

[anonrig]

Unfortunately, I don’t have a solution to this problem without seriously impacting the performance.

Do you mean using a WeakMap? How bad are we talking about?

The root cause of this inequality causes from console.log(url) where it calls searchParams getter (ref: https://github.com/nodejs/node/blob/main/lib/internal/url.js#L809). When searchParams getter is run, if it's not initialized, it initializes URLSearchParams and assigns that variable. But if you have 2 different URL's, in which URLSearchParams is initialized, and in another, it is not, this inequality occurs.

Available solutions:

• not trigger searchParams getter on inspect, with the loss of context in inspect function.
• remove the lazy approach of URLSearchParams on searchParams getter, which would impact the execution time of new URL() since it would also call URLSearchParams constructor.

[aduh95]

Wouldn't another solution to have a WeakMap to store the query rather than adding a Symbol to the URL instance? Instead of having something like

const searchParams = Symbol('query');

class URL{
 get searchParams() {
 if (!isURL(this))
 throw new ERR_INVALID_THIS('URL');
 if (this[searchParams] == null) {
 this[searchParams] = new URLSearchParams(this.search);
 this[searchParams][context] = this;
 }
 return this[searchParams];
 }
}

we could have something like:

const searchParams = new SafeWeakMap();
class URL{
 get searchParams() {
 if (!isURL(this))
 throw new ERR_INVALID_THIS('URL');
 const cachedValue = searchParams.get(this)
 if (cachedValue != null) {
 return cachedValue;
 }
 const value = new URLSearchParams(this.search);
 value[context] = this;
 searchParams.set(this, value);
 return value;
 }
}

It'd still be lazy but wouldn't affect the deep equality.

[anonrig]

Might work. Can you open a pull request to v18.x-staging?