Skip to content

fix(url): handle :port values in host setter#963

Merged
anonrig merged 1 commit intoada-url:mainfrom
watilde:fix/url-host-setter
Jun 29, 2025
Merged

fix(url): handle :port values in host setter#963
anonrig merged 1 commit intoada-url:mainfrom
watilde:fix/url-host-setter

Conversation

@watilde
Copy link
Contributor

@watilde watilde commented Jun 28, 2025

The current implementation incorrectly allows values like ":80" (empty host with port), resulting in foo://:80/to, while the spec requires failure if the host is missing. This update aligns behavior with the spec.

const NodeURL = require("url").URL;
const WhatwgURL = require("whatwg-url").URL;
const newHost = ':80';

const url = new NodeURL("foo://path/to");  
url.host = newHost;
console.log(url.href); // Wrong: foo://:80/to

const url2 = new WhatwgURL("foo://path/to");
url2.host = newHost;
console.log(url2.href); // Correct: foo://path/to

Refs: https://url.spec.whatwg.org/#host-state

If buffer is the empty string, host-missing validation error, return failure.

WPT tests for this case will be added in a follow-up PR.

return false;
} else if (has_dash_dot()) {
add_authority_slashes_if_needed();
// Should remove dash_dot from pathname
Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of this?

@anonrig anonrig requested a review from lemire June 29, 2025 12:55
@anonrig anonrig merged commit 523e9e2 into ada-url:main Jun 29, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants