Repository navigation
Conversation
Web IDL converts an assigned value to the attribute's type before the setter steps run. The URL component setters passed `this.#context.href` as the first argument to `bindingUrl.update()`, evaluating it before the assigned value's string conversion. A `toString()` that mutates the URL during conversion therefore had its effects overwritten by the stale href captured beforehand. Hoist the conversion above the href read in the protocol, username, password, host, hostname, port, pathname, search and hash setters, matching the existing `href` setter. Fixes: nodejs#67345 Co-authored-by: generaltony007 <144252170+GeneralTony007@users.noreply.github.com>
Regression test: every URL component setter must apply to the URL state after value conversion, not to stale state captured before it. Fixes: nodejs#67345 Co-authored-by: generaltony007 <144252170+GeneralTony007@users.noreply.github.com>
|
Review requested:
|
|
Welcome to Node.js, and thank you for your first contribution! Before review, please take a moment to read:
Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal. Caution AgentScan found account activity patterns that may be consistent with automation. This is a heuristic, not proof that this pull request was opened by an agent or violates policy. AI-assisted contributions are permitted, but automated tooling must not open pull requests without advance approval, and contributors must personally understand, test, verify, and take responsibility for every submitted change. See the AgentScan analysis, AI use policy, and automation policy for additional context. |
|
See Commit message guidelines according to the Pull requests documentation. Your commit must contain the This is repeated also in AGENTS.md where it says:
|
PRs are required to be fully tested before submission: See Pull requests > Step 6: Test
|
Edit: removed
Fixes: https://github.com/nodejs/node/issues/67345Web IDL converts an assigned value to the attribute's type before the setter steps run. The URL component setters passed
this.#context.hrefas the first argument tobindingUrl.update(), so it was evaluated before the assigned value's string conversion. AtoString()that mutates the URL during conversion therefore had its effects overwritten by the stale href captured beforehand:The change hoists the conversion above the href read in the
protocol,username,password,host,hostname,port,pathname,searchandhashsetters, matching the existinghrefsetter, which already converts first.Adds
test/parallel/test-whatwg-url-setters-coercion-order.jscovering all nine setters (WPT'ssetters_tests.jsonhas no mutation-during-conversion cases).Local verification: the new test fails on the unpatched code (all nine setters) and passes with the fix; I additionally ran a differential check of 25 ordinary URL operations (assignments, port edge cases, URLSearchParams sync, percent-encoding) with identical outputs before and after the change. Note my sandbox can't build Node, so I verified the patched
lib/internal/url.jsby loading it through--expose-internalson v22.23.4 against the realadabinding - CI should confirm on a full build.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes - sandbox here can't build Node; new test verified via the--expose-internalsroute described above, CI run appreciated