Skip to content

Commit 8dc084a

Browse files
robobunClaude BotclaudeJarred-Sumner
authored
fix(fetch): ignore proxy object without url property (#25414)
## Summary - When a URL object is passed as the proxy option, or when a proxy object lacks a "url" property, ignore it instead of throwing an error - This fixes a regression introduced in 1.3.4 where libraries like taze that pass URL objects as proxy values would fail ## Test plan - Added test: "proxy as URL object should be ignored (no url property)" - passes a URL object directly as proxy - Updated test: "proxy object without url is ignored (regression #25413)" - proxy object with headers but no url - Updated test: "proxy object with null url is ignored (regression #25413)" - proxy object where url is null - All 29 proxy tests pass Fixes #25413 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Bot <[email protected]> Co-authored-by: Claude <[email protected]> Co-authored-by: Jarred Sumner <[email protected]>
1 parent 2028e21 commit 8dc084a

File tree

2 files changed

+81
-60
lines changed

2 files changed

+81
-60
lines changed

‎src/bun.js/webcore/fetch.zig‎

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -667,51 +667,51 @@ pub fn Bun__fetch_(
667667
break :extract_proxy buffer;
668668
}
669669
// Handle object format: proxy: { url: "http://proxy.example.com:8080", headers?: Headers }
670+
// If the proxy object doesn't have a 'url' property, ignore it.
671+
// This handles cases like passing a URL object directly as proxy (which has 'href' not 'url').
670672
if (proxy_arg.isObject()) {
671673
// Get the URL from the proxy object
672-
const proxy_url_arg = try proxy_arg.get(globalThis, "url");
673-
if (proxy_url_arg == null or proxy_url_arg.?.isUndefinedOrNull()) {
674-
const err = ctx.toTypeError(.INVALID_ARG_VALUE, "fetch() proxy object requires a 'url' property", .{});
675-
is_error = true;
676-
return JSPromise.dangerouslyCreateRejectedPromiseValueWithoutNotifyingVM(globalThis, err);
677-
}
678-
if (proxy_url_arg.?.isString() and try proxy_url_arg.?.getLength(ctx) > 0) {
679-
var href = try jsc.URL.hrefFromJS(proxy_url_arg.?, globalThis);
680-
if (href.tag == .Dead) {
681-
const err = ctx.toTypeError(.INVALID_ARG_VALUE, "fetch() proxy URL is invalid", .{});
682-
is_error = true;
683-
return JSPromise.dangerouslyCreateRejectedPromiseValueWithoutNotifyingVM(globalThis, err);
684-
}
685-
defer href.deref();
686-
const buffer = try std.fmt.allocPrint(allocator, "{s}{f}", .{ url_proxy_buffer, href });
687-
url = ZigURL.parse(buffer[0..url.href.len]);
688-
if (url.isFile()) {
689-
url_type = URLType.file;
690-
} else if (url.isBlob()) {
691-
url_type = URLType.blob;
692-
}
674+
if (try proxy_arg.get(globalThis, "url")) |proxy_url_arg| {
675+
if (!proxy_url_arg.isUndefinedOrNull()) {
676+
if (proxy_url_arg.isString() and try proxy_url_arg.getLength(ctx) > 0) {
677+
var href = try jsc.URL.hrefFromJS(proxy_url_arg, globalThis);
678+
if (href.tag == .Dead) {
679+
const err = ctx.toTypeError(.INVALID_ARG_VALUE, "fetch() proxy URL is invalid", .{});
680+
is_error = true;
681+
return JSPromise.dangerouslyCreateRejectedPromiseValueWithoutNotifyingVM(globalThis, err);
682+
}
683+
defer href.deref();
684+
const buffer = try std.fmt.allocPrint(allocator, "{s}{f}", .{ url_proxy_buffer, href });
685+
url = ZigURL.parse(buffer[0..url.href.len]);
686+
if (url.isFile()) {
687+
url_type = URLType.file;
688+
} else if (url.isBlob()) {
689+
url_type = URLType.blob;
690+
}
693691

694-
proxy = ZigURL.parse(buffer[url.href.len..]);
695-
allocator.free(url_proxy_buffer);
696-
url_proxy_buffer = buffer;
697-
698-
// Get the headers from the proxy object (optional)
699-
if (try proxy_arg.get(globalThis, "headers")) |headers_value| {
700-
if (!headers_value.isUndefinedOrNull()) {
701-
if (headers_value.as(FetchHeaders)) |fetch_hdrs| {
702-
proxy_headers = Headers.from(fetch_hdrs, allocator, .{}) catch |err| bun.handleOom(err);
703-
} else if (try FetchHeaders.createFromJS(ctx, headers_value)) |fetch_hdrs| {
704-
defer fetch_hdrs.deref();
705-
proxy_headers = Headers.from(fetch_hdrs, allocator, .{}) catch |err| bun.handleOom(err);
692+
proxy = ZigURL.parse(buffer[url.href.len..]);
693+
allocator.free(url_proxy_buffer);
694+
url_proxy_buffer = buffer;
695+
696+
// Get the headers from the proxy object (optional)
697+
if (try proxy_arg.get(globalThis, "headers")) |headers_value| {
698+
if (!headers_value.isUndefinedOrNull()) {
699+
if (headers_value.as(FetchHeaders)) |fetch_hdrs| {
700+
proxy_headers = Headers.from(fetch_hdrs, allocator, .{}) catch |err| bun.handleOom(err);
701+
} else if (try FetchHeaders.createFromJS(ctx, headers_value)) |fetch_hdrs| {
702+
defer fetch_hdrs.deref();
703+
proxy_headers = Headers.from(fetch_hdrs, allocator, .{}) catch |err| bun.handleOom(err);
704+
}
705+
}
706706
}
707+
708+
break :extract_proxy url_proxy_buffer;
709+
} else {
710+
const err = ctx.toTypeError(.INVALID_ARG_VALUE, "fetch() proxy.url must be a non-empty string", .{});
711+
is_error = true;
712+
return JSPromise.dangerouslyCreateRejectedPromiseValueWithoutNotifyingVM(globalThis, err);
707713
}
708714
}
709-
710-
break :extract_proxy url_proxy_buffer;
711-
} else {
712-
const err = ctx.toTypeError(.INVALID_ARG_VALUE, "fetch() proxy.url must be a non-empty string", .{});
713-
is_error = true;
714-
return JSPromise.dangerouslyCreateRejectedPromiseValueWithoutNotifyingVM(globalThis, err);
715715
}
716716
}
717717
}

‎test/js/bun/http/proxy.test.ts‎

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -500,29 +500,32 @@ describe("proxy object format with headers", () => {
500500
}
501501
});
502502

503-
test("proxy object without url throws error", async () => {
504-
await expect(
505-
fetch(httpServer.url, {
506-
method: "GET",
507-
proxy: {
508-
headers: { "X-Test": "value" },
509-
} as any,
510-
keepalive: false,
511-
}),
512-
).rejects.toThrow("fetch() proxy object requires a 'url' property");
503+
test("proxy object without url is ignored (regression #25413)", async () => {
504+
// When proxy object doesn't have a 'url' property, it should be ignored
505+
// This ensures compatibility with libraries that pass URL objects as proxy
506+
const response = await fetch(httpServer.url, {
507+
method: "GET",
508+
proxy: {
509+
headers: { "X-Test": "value" },
510+
} as any,
511+
keepalive: false,
512+
});
513+
expect(response.ok).toBe(true);
514+
expect(response.status).toBe(200);
513515
});
514516

515-
test("proxy object with null url throws error", async () => {
516-
await expect(
517-
fetch(httpServer.url, {
518-
method: "GET",
519-
proxy: {
520-
url: null,
521-
headers: { "X-Test": "value" },
522-
} as any,
523-
keepalive: false,
524-
}),
525-
).rejects.toThrow("fetch() proxy object requires a 'url' property");
517+
test("proxy object with null url is ignored (regression #25413)", async () => {
518+
// When proxy.url is null, the proxy object should be ignored
519+
const response = await fetch(httpServer.url, {
520+
method: "GET",
521+
proxy: {
522+
url: null,
523+
headers: { "X-Test": "value" },
524+
} as any,
525+
keepalive: false,
526+
});
527+
expect(response.ok).toBe(true);
528+
expect(response.status).toBe(200);
526529
});
527530

528531
test("proxy object with empty string url throws error", async () => {
@@ -699,4 +702,22 @@ describe("proxy object format with headers", () => {
699702
await once(proxyServerWithCapture, "close");
700703
}
701704
});
705+
706+
test("proxy as URL object should be ignored (no url property)", async () => {
707+
// This tests the regression from #25413
708+
// When a URL object is passed as proxy, it should be ignored (no error)
709+
// because URL objects don't have a "url" property - they have "href"
710+
const proxyUrl = new URL(httpProxyServer.url);
711+
712+
// Passing a URL object as proxy should NOT throw an error
713+
// It should just be ignored since there's no "url" string property
714+
const response = await fetch(httpServer.url, {
715+
method: "GET",
716+
proxy: proxyUrl as any,
717+
keepalive: false,
718+
});
719+
// The request should succeed (without proxy, since URL object is ignored)
720+
expect(response.ok).toBe(true);
721+
expect(response.status).toBe(200);
722+
});
702723
});

0 commit comments

Comments
 (0)