Skip to content

fix(node:url): Respect options parameter in url.format #24402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Kartikkala wants to merge 1 commit into from
+87 −36
Closed

fix(node:url): Respect options parameter in url.format #24402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 64 additions & 13 deletions src/js/node/url.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ const [domainToASCII, domainToUnicode] = $cpp("NodeURL.cpp", "Bun::createNodeURL
const { urlToHttpOptions } = require("internal/url");
const { validateString } = require("internal/validators");

interface URLFormatOptions {
auth?: boolean | string | number | null;
unicode?: boolean | number | null;
search?: boolean | number | string | null;
fragment?: boolean | number | null;
}

function Url() {
this.protocol = null;
this.slashes = null;
Expand Down Expand Up @@ -460,9 +467,13 @@ function getHostname(self, rest, hostname: string, url) {
return rest;
}

function isExplicitlyFalse(value){
return value != undefined && (!value || value === "");
}

// format a parsed object into a url string
declare function urlFormat(urlObject: string | URL | Url): string;
function urlFormat(urlObject: unknown) {
declare function urlFormat(urlObject: string | URL | Url, options?: URLFormatOptions): string;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
function urlFormat(urlObject: unknown, options?: unknown) {
/*
* ensure it's an object, and not a string url.
* If it's an obj, this is a no-op.
Expand All @@ -476,14 +487,36 @@ function urlFormat(urlObject: unknown) {
throw $ERR_INVALID_ARG_TYPE("urlObject", ["Object", "string"], urlObject);
}

if (options !== undefined && typeof options !== "object") {
throw $ERR_INVALID_ARG_TYPE("options", ["Object"], options);
}
Comment on lines +490 to +492

@coderabbitai coderabbitai Bot Nov 5, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider validating individual option properties.

While the current validation ensures options is an object, it doesn't validate the types of individual properties (e.g., that options.unicode is a boolean). If invalid property types could cause issues downstream, consider adding property-level validation.

🤖 Prompt for AI Agents 
In src/js/node/url.ts around lines 486 to 488, the code only checks that options
is an object but not that its individual properties are of expected types; add
property-level validation after the existing object check to guard against bad
downstream inputs — specifically, for each supported option (e.g., unicode,
auth, base, etc.) verify its presence is the right type (for booleans use typeof
=== "boolean", for strings typeof === "string", for objects use typeof ===
"object" and null-checks or Array.isArray where appropriate), throw
$ERR_INVALID_ARG_TYPE with the option name and expected type when a property has
the wrong type, and ensure optional properties are only validated when defined
so existing behavior remains unchanged.


if (!(urlObject instanceof Url)) {
return Url.prototype.format.$call(urlObject);
return Url.prototype.format.$call(urlObject, options);
}
return urlObject.format();
return urlObject.format(options);
}

Url.prototype.format = function format() {
var auth: string = this.auth || "";
Url.prototype.format = function format(options?: URLFormatOptions) {
options = options || {};

// Determine the auth string to use, reconstruct if necessary.
let authToUse = this.auth;
if ((authToUse === null || authToUse === undefined) && !isExplicitlyFalse(options.auth)) {
if (this.username || this.password) {
authToUse = this.username + (this.password ? ":" + this.password : "");
}
}

let auth: string = "";
if (typeof options.auth === "string") {
auth = options.auth;
} else if (isExplicitlyFalse(options.auth)) {
auth = "";
} else {
auth = authToUse || "";
}
Comment thread
Kartikkala marked this conversation as resolved.

if (auth) {
auth = encodeURIComponent(auth);
auth = auth.replace(/%3A/i, ":");
Expand All @@ -496,13 +529,21 @@ Url.prototype.format = function format() {
host = "",
query = "";

if (this.host) {
host = auth + this.host;
} else if (this.hostname) {
host = auth + (this.hostname.indexOf(":") === -1 ? this.hostname : "[" + this.hostname + "]");
// Convert to unicode if options.unicode is truthy
let hostnameToUse = this.hostname;
if (options.unicode && this.hostname) {
try {
hostnameToUse = domainToUnicode(hostnameToUse);
} catch {}
}

if (this.hostname) {
host = auth + (hostnameToUse.indexOf(":") === -1 ? hostnameToUse : "[" + hostnameToUse + "]");
if (this.port) {
host += ":" + this.port;
}
} else if (this.host) {
host = auth + this.host;
}

if (this.query && typeof this.query === "object" && Object.keys(this.query).length) {
Expand All @@ -511,6 +552,11 @@ Url.prototype.format = function format() {

var search = this.search || (query && "?" + query) || "";

// Apply options.search explicitly only if it's falsy
if (isExplicitlyFalse(options.search)) {
search = "";
}

if (protocol && protocol.substr(-1) !== ":") {
protocol += ":";
}
Expand All @@ -528,9 +574,15 @@ Url.prototype.format = function format() {
host = "";
}

if (hash && hash.charAt(0) !== "#") {
hash = "#" + hash;
// Apply options.fragment explicitly only if it is falsy
if (isExplicitlyFalse(options.fragment)) {
hash = "";
} else {
if (hash && hash.charAt(0) !== "#") {
hash = "#" + hash;
}
}
Comment thread
Kartikkala marked this conversation as resolved.

if (search && search.charAt(0) !== "?") {
search = "?" + search;
}
Expand All @@ -542,7 +594,6 @@ Url.prototype.format = function format() {

return protocol + host + pathname + search + hash;
};

function urlResolve(source: string | URL | Url, relative: string | URL | Url) {
return urlParse(source, false, true).resolve(relative);
}
Expand Down
46 changes: 23 additions & 23 deletions test/js/node/url/url-format-whatwg.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ describe("url.format", () => {

// TODO: Support these.
//
// assert.strictEqual(url.format(myURL), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");
assert.strictEqual(url.format(myURL), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");

// assert.strictEqual(url.format(myURL, {}), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");
assert.strictEqual(url.format(myURL, {}), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");

// TODO: Support this kind of assert.throws.
// {
Expand All @@ -34,44 +34,44 @@ describe("url.format", () => {

// TODO: Support these.
//
// assert.strictEqual(url.format(myURL, { auth: 1 }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");
assert.strictEqual(url.format(myURL, { auth: 1 }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");

// assert.strictEqual(url.format(myURL, { auth: {} }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");
assert.strictEqual(url.format(myURL, { auth: {} }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");

// assert.strictEqual(url.format(myURL, { fragment: false }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b");
assert.strictEqual(url.format(myURL, { fragment: false }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b");

// assert.strictEqual(url.format(myURL, { fragment: "" }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b");
assert.strictEqual(url.format(myURL, { fragment: "" }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b");

// assert.strictEqual(url.format(myURL, { fragment: 0 }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b");
assert.strictEqual(url.format(myURL, { fragment: 0 }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b");

// assert.strictEqual(url.format(myURL, { fragment: 1 }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");
assert.strictEqual(url.format(myURL, { fragment: 1 }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");

// assert.strictEqual(url.format(myURL, { fragment: {} }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");
assert.strictEqual(url.format(myURL, { fragment: {} }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");

// assert.strictEqual(url.format(myURL, { search: false }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a#c");
assert.strictEqual(url.format(myURL, { search: false }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a#c");

// assert.strictEqual(url.format(myURL, { search: "" }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a#c");
assert.strictEqual(url.format(myURL, { search: "" }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a#c");

// assert.strictEqual(url.format(myURL, { search: 0 }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a#c");
assert.strictEqual(url.format(myURL, { search: 0 }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a#c");

// assert.strictEqual(url.format(myURL, { search: 1 }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");
assert.strictEqual(url.format(myURL, { search: 1 }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");

// assert.strictEqual(url.format(myURL, { search: {} }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");
assert.strictEqual(url.format(myURL, { search: {} }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");

// assert.strictEqual(url.format(myURL, { unicode: true }), "http://user:pass@理容ナカムラ.com/a?a=b#c");
assert.strictEqual(url.format(myURL, { unicode: true }), "http://user:pass@理容ナカムラ.com/a?a=b#c");

// assert.strictEqual(url.format(myURL, { unicode: 1 }), "http://user:pass@理容ナカムラ.com/a?a=b#c");
assert.strictEqual(url.format(myURL, { unicode: 1 }), "http://user:pass@理容ナカムラ.com/a?a=b#c");

// assert.strictEqual(url.format(myURL, { unicode: {} }), "http://user:pass@理容ナカムラ.com/a?a=b#c");
assert.strictEqual(url.format(myURL, { unicode: {} }), "http://user:pass@理容ナカムラ.com/a?a=b#c");

// assert.strictEqual(url.format(myURL, { unicode: false }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");
assert.strictEqual(url.format(myURL, { unicode: false }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");

// assert.strictEqual(url.format(myURL, { unicode: 0 }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");
assert.strictEqual(url.format(myURL, { unicode: 0 }), "http://user:pass@xn--lck1c3crb1723bpq4a.com/a?a=b#c");

// assert.strictEqual(
// url.format(new URL("http://user:pass@xn--0zwm56d.com:8080/path"), { unicode: true }),
// "http://user:pass@测试.com:8080/path",
// );
assert.strictEqual(
url.format(new URL("http://user:pass@xn--0zwm56d.com:8080/path"), { unicode: true }),
"http://user:pass@测试.com:8080/path",
);

assert.strictEqual(url.format(new URL("tel:123")), url.format(new URL("tel:123"), { unicode: true }));
});
Expand Down