fetch: reject non-HTTP(S) schemes before touching the network by robobun · Pull Request #33960 · oven-sh/bun

robobun · 2026-07-11T02:36:10Z

Reproduction

import * as http from "node:http";
const seen = [];
const s = http.createServer((rq, rs) => { seen.push(rq.url); rs.end("ok"); });
await new Promise(r => s.listen(0, "127.0.0.1", r));
const u = `localhost:${s.address().port}/api/x?q=1`; // scheme "localhost:", host ""
console.log(await fetch(u).then(r => r.status, e => e.name));
// bun: 200, server receives the request
// node: TypeError, no network activity
console.log(await fetch("about:blank").then(r => r.status, e => `${e.name} ${e.syscall ?? ""} ${e.hostname ?? ""}`));
// bun: issues getaddrinfo("about")
// node: TypeError

fetch() of a URL whose WHATWG scheme is not http/https leaks a DNS query for the scheme name and, if the resolver answers, opens a real connection. Node/undici and the fetch spec reject these with TypeError and zero network activity.

Cause

fetch_impl re-parses the WHATWG-normalized href with the simple bun_url::URL parser and gates on scheme:

if !url.protocol.is_empty() {
    if !(url.is_http() || url.is_https() || url.is_s3()) { /* reject */ }
}

URL::parse_protocol (src/url/lib.rs) only assigns self.protocol when the colon is followed by //. Non-special schemes keep their opaque scheme:rest form through WHATWG normalization, so protocol stays empty, the outer guard skips the check, and the rest of parse() treats scheme:rest as host:port.

ftp:example.com/ was already rejected only because ftp is a WHATWG special scheme and normalizes to ftp://example.com/.

Fix

Drop the !url.protocol.is_empty() guard. The input has already been WHATWG-validated by this point; http/https are special schemes that always normalize to scheme://host, so is_http()/is_https() are reliable without the guard. data:, file:, and registered blob: URLs are handled earlier and are unaffected.

This matches the existing unconditional check in fetch.preconnect() in the same file.

Verification

New tests in test/js/web/fetch/fetch-args.test.ts cover about:blank, javascript:alert(1), chrome:flags, foo:bar, localhost:<port>/path (asserting the local server is not hit), and new Request("about:blank"). A positive test confirms http:// and data: still succeed.

Fails on main with Received value: null (fetch resolved via network) or a DNS error; passes with this change.

[review] gate passed · iteration 1 · 2 files touched

fails on main (without fix)

ASAN without fix: 6 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/fetch/fetch-args.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (a3619b52d)

test/js/web/fetch/fetch-args.test.ts:
26 |       const prevCount = requestCount;
27 |       const err = await fetch(input).then(
28 |         () => null,
29 |         e => e,
30 |       );
31 |       expect(err).toBeInstanceOf(TypeError);
                       ^
error: expect(received).toBeInstanceOf(expected)

Expected constructor: [class TypeError extends Error]
Received value: null

      at <anonymous> (/workspace/bun/test/js/web/fetch/fetch-args.test.ts:31:19)
(fail) non-HTTP(S) URL scheme rejection > fetch("about:blank") rejects with TypeError [23.13ms]
26 |       const prevCount = requestCount;
27 |       const err = await fetch(input).then(
28 |         () => null,
29 |         e => e,
30 |       );
31 |     
... (truncated)

release without fix: all passed
bun test v1.4.0-canary.1 (2985d11b0)

test/js/web/fetch/fetch-args.test.ts:
(pass) non-HTTP(S) URL scheme rejection > fetch("about:blank") rejects with TypeError [0.27ms]
(pass) non-HTTP(S) URL scheme rejection > fetch("javascript:alert(1)") rejects with TypeError [0.03ms]
(pass) non-HTTP(S) URL scheme rejection > fetch("chrome:flags") rejects with TypeError [0.01ms]
(pass) non-HTTP(S) URL scheme rejection > fetch("foo:bar") rejects with TypeError [0.01ms]
(pass) non-HTTP(S) URL scheme rejection > fetch('localhost:<port>/path') does not reach the network [0.22ms]
(pass) non-HTTP(S) URL scheme rejection > fetch(new Request('about:blank')) rejects with TypeError [0.15ms]
(pass) non-HTTP(S) URL scheme rejection > http:// and data: still work [1.02ms]
(pass) fetch(request subclass with headers) [0.58ms]
(pass) fetch(RequestInit, headers) [0.19ms]
(pass) fetch(url, RequestSubclass) [0.21ms]
(pass) fetch({toString throwing}, {headers} isn't accessed) [0.48ms]
(pass) fetch() rejects instead of throwing synchronously when option conversion throws > url toString() throws [0.13ms]
(pass) fetch() rejects instead of throwing synchronously when option conversion throws > init.he
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/fetch/fetch-args.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (a3619b52d)

test/js/web/fetch/fetch-args.test.ts:
(pass) non-HTTP(S) URL scheme rejection > fetch("about:blank") rejects with TypeError [7.93ms]
(pass) non-HTTP(S) URL scheme rejection > fetch("javascript:alert(1)") rejects with TypeError [1.34ms]
(pass) non-HTTP(S) URL scheme rejection > fetch("chrome:flags") rejects with TypeError [1.02ms]
(pass) non-HTTP(S) URL scheme rejection > fetch("foo:bar") rejects with TypeError [0.95ms]
(pass) non-HTTP(S) URL scheme rejection > fetch('localhost:<port>/path') does not reach the network [8.46ms]
(pass) non-HTTP(S) URL scheme rejection > fetch(new Request('about:blank')) rejects with TypeError [5.85ms]
(pass) non-HTTP(S) URL scheme rejection > http:// and data: still work [12.35ms]
(pass)
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
[configured] bun-profile → bun (stripped) in 715ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/74] gen ErrorCode+*.h
[2/23] gen generated_host_exports.rs
generated_host_exports.rs: 91 exports (host=3, lazy=10, generic=78, rust=0); 243 extern-C blocks audited
[3/23] gen cpp.rs (cppbind)
[4/23] gen JS modules (bundle-modules)
Preprocess modules (6775ms)
Bundle modules (43ms)
Postprocesss modules (26ms)
Bundle Functions (675ms)
Generate Code (79ms)

[7.61s] Bundled "src/js" for production
  1913 kb
  162 internal modules
  12 native modules
  90 internal functions across 19 files
[4/9] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu)
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to d
... (truncated)

diff hotspot

src/runtime/webcore/fetch.rs         | 24 +++++++++----------
 test/js/web/fetch/fetch-args.test.ts | 45 ++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 13 deletions(-)

gate history · 2 passed · 0 rejected · iteration 1

evidence per changed file

file                                  reads  edits  tests
src/runtime/webcore/fetch.rs              4      1      0
test/js/web/fetch/fetch-args.test.ts      2      1      0

fetch() with a URL whose scheme is not http/https/s3 must reject with TypeError before any network step. The existing scheme gate in fetch_impl was wrapped in `if !url.protocol.is_empty()`, but the simple URL parser only populates `protocol` when it sees `://`. WHATWG-valid URLs without an authority (`about:blank`, `javascript:...`, `localhost:3000/x`) therefore reached the HTTP connect path with the scheme name treated as a hostname, issuing real DNS lookups for `about`, `javascript`, etc. Drop the is_empty guard. By this point the input has already been WHATWG-normalized, and http/https are special schemes that always normalize to `scheme://host`, so `is_http()`/`is_https()` are reliable. data:, file:, and blob: URLs are handled earlier and are unaffected.

robobun · 2026-07-11T02:36:19Z

^{Updated 11:01 PM PT - Jul 10th, 2026}

❌ @robobun, your commit a3619b5 has 2 failures in Build #71795 (All Failures):

test/js/bun/http/proxy-stress-concurrent.test.ts - code 1 on 🍎 14 x64
test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js - SIGABRT on 🐧 13 x64-asan

🧪 To try this PR locally:

bunx bun-pr 33960

That installs a local version of the PR into your bun-33960 executable, so you can run:

bun-33960 --bun

coderabbitai · 2026-07-11T02:38:35Z

Walkthrough

Fetch protocol validation is now unconditional, rejecting unsupported schemes and scheme-parsed host URLs. Tests cover rejection errors, prevented network requests, Request inputs, and continued HTTP(S) and data: support.

Changes

Fetch protocol validation

Layer / File(s)	Summary
Protocol check and regression tests `src/runtime/webcore/fetch.rs`, `test/js/web/fetch/fetch-args.test.ts`	`fetch_impl` rejects protocols other than `http`, `https`, and `s3`; tests cover unsupported schemes, `localhost:<port>` parsing, `Request("about:blank")`, request counts, and supported URL types.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and accurately summarizes the main change: rejecting non-HTTP(S) schemes before network access.
Description check	✅ Passed	It covers what the PR does and how it was verified, though the headings don't match the repository template exactly.

_{Comment @coderabbitai help to get the list of available commands.}

claude

LGTM — dropping the empty-protocol guard is the right fix and matches fetch.preconnect() a few hundred lines up.

What was reviewed:

Confirmed ZigURL::from_string runs WHATWG normalization first, so http/https always arrive as scheme://host and is_http()/is_https() remain reliable without the guard.
Verified data:, file:, and blob: are handled before this check and are unaffected; the new positive test covers http + data.
Checked that s3 URLs (written as s3://…) still parse a non-empty protocol; the unix socket path still requires an http(s) URL and is unaffected.

Extended reasoning...

Overview

Single-line semantic change in src/runtime/webcore/fetch.rs: removes the if !url.protocol.is_empty() outer guard around the http/https/s3 scheme check in fetch_impl. The rest of the Rust diff is de-indentation. New tests in test/js/web/fetch/fetch-args.test.ts cover about:, javascript:, chrome:, foo:, the localhost:<port>/path footgun, new Request('about:blank'), and a positive test for http + data.

Security risks

This is a security-positive change: it closes a hole where non-HTTP schemes leaked DNS queries (and potentially connections) for the scheme name. No new attack surface; the check is strictly tighter than before.

Level of scrutiny

Low-medium. The change is mechanical (drop one guard) and the PR description traces the root cause precisely: URL::parse_protocol (src/url/lib.rs:727) only assigns protocol when :// follows, so non-special WHATWG schemes fell through the old guard. I verified is_http/is_https/is_s3 compare against self.protocol (src/url/lib.rs:399-409), and that ZigURL::from_string (src/url/lib.rs:359) WHATWG-normalizes the href first — meaning http/https (special schemes) always serialize as scheme://host and will never have an empty protocol after re-parse. The identical unconditional check already exists in bun_fetch_preconnect in the same file.

Other factors

data: is handled by an early has_prefix_comptime return; file: and blob: set url_type != Remote and return before this check. All unaffected.
node_http_client shares fetch_impl but node:http always constructs explicit http:///https:// URLs, so no compat regression there.
Tests assert both the TypeError and that requestCount is unchanged (no network activity), plus a positive control.

claude

LGTM — removing the empty-protocol guard makes the scheme check effective for opaque URLs, matching fetch.preconnect() and Node/spec behavior.

What was reviewed:

Confirmed is_http()/is_https()/is_s3() compare against protocol set by parse_protocol, which WHATWG-normalized http/https/s3 URLs always populate — no valid inputs are newly rejected.
data:, file:, and blob: are handled before this check, so they're unaffected (and covered by the positive test).
The node_http_client entry shares fetch_impl but only receives http/https URLs, so it's not impacted.

Extended reasoning...

Overview

The PR removes the outer !url.protocol.is_empty() guard around the http/https/s3 scheme check in fetch_impl (src/runtime/webcore/fetch.rs). Previously, opaque-scheme URLs like about:blank or localhost:3000/x would keep an empty protocol field after bun_url::URL::parse (which only sets protocol when the colon is followed by //), causing the scheme check to be skipped and the scheme name to be treated as a hostname — leaking DNS queries and potentially connecting. Now the check runs unconditionally, rejecting any non-http/https/s3 scheme with a TypeError before any network activity. New tests in test/js/web/fetch/fetch-args.test.ts cover several opaque schemes, the localhost:<port> case with a request-count assertion, new Request('about:blank'), and a positive check that http:// and data: still work.

Security risks

This is a strict tightening: it closes an unintended network egress path (DNS lookup / connection to a host derived from a URL scheme name). No new attack surface is introduced; the change only rejects more inputs earlier.

Level of scrutiny

Low-to-moderate. The native change is a two-line guard removal; the identical unconditional pattern already exists in bun_fetch_preconnect a few lines up in the same file. The key soundness question — whether any valid http/https/s3 URL could have an empty protocol and thus be newly rejected — is answered by the WHATWG special-scheme normalization guarantee (http/https always serialize as scheme://…), which parse_protocol recognizes. data: is handled before URL parsing, and file:/blob: set url_type != Remote and return before this check.

Other factors

The PR includes fail-on-main / pass-on-PR evidence for both debug+ASAN and release builds. Tests are placed in the existing fetch-args.test.ts alongside related "does not send a request when" coverage, use the shared requestCount server to assert no network activity, and include a positive regression guard. The node_http_client entry point (shared fetch_impl<true>) only receives http/https URLs from node:http, so it's unaffected. No outstanding reviewer comments.

robobun · 2026-07-11T03:17:00Z

CI status: the diff is green on its own tests. The only hard failures across builds #71785 and #71795 are

test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js SIGABRT on linux-x64-asan (ASSERTION FAILED: !scope.exception() || !result in JSObject::getOwnPropertyDescriptor): the pre-existing worker termination race being addressed in worker_threads: don't abort when terminate() interrupts a lazy property builder #33418.
test/js/bun/http/proxy-stress-concurrent.test.ts on darwin-x64: 1 of 1200 concurrent https-through-https-proxy requests failed. Same test flaked as [warning] on unrelated build #71792 on the same lane; the test only exercises https:// URLs, which this change does not touch.

Both are unrelated to this change. The remaining annotations are yellow (passed on retry) on Windows install/napi/bake, darwin proxy-stress siblings, and the aarch64 solc timeout.

test/js/web/fetch/fetch-args.test.ts passed on every lane, and the robobun/evidence gate confirmed the new tests fail on main and pass with this change on both ASAN and release.

Ready for review.

github-actions Bot added the claude label Jul 11, 2026

claude Bot reviewed Jul 11, 2026

View reviewed changes

ci: retrigger

a3619b5

claude Bot reviewed Jul 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fetch: reject non-HTTP(S) schemes before touching the network#33960

fetch: reject non-HTTP(S) schemes before touching the network#33960
robobun wants to merge 2 commits into
mainfrom
farm/26549f29/fetch-reject-non-http-schemes

robobun commented Jul 11, 2026 •

edited

Loading

Uh oh!

robobun commented Jul 11, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 11, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Jul 11, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

robobun commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reproduction

Cause

Fix

Verification

Uh oh!

robobun commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Jul 11, 2026 •

edited

Loading

robobun commented Jul 11, 2026 •

edited

Loading

coderabbitai Bot commented Jul 11, 2026 •

edited

Loading

robobun commented Jul 11, 2026 •

edited

Loading