From 6665684abe7a88d7751017f36656699ff0f6d602 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Fri, 12 Jun 2026 02:23:10 +0200 Subject: [PATCH 1/2] http: deliver 3xx responses without a Location header as final responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every 300-399 status was routed into the redirect path, where the missing Location header fails the transfer with error.LocationNotFound — the old document is destroyed and nothing replaces it. Per the fetch standard's HTTP-redirect fetch ("If locationURL is null, then return response") and RFC 9110 §15.4, a 3xx without Location is a normal final response whose body must be delivered. Guard both the redirect dispatch and the 3xx skip-body branch on the header's presence so such responses fall through to the regular completion path. Closes #2713 --- src/browser/HttpClient.zig | 9 +++++++-- src/cdp/domains/page.zig | 28 ++++++++++++++++++++++++++++ src/testing.zig | 11 +++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/browser/HttpClient.zig b/src/browser/HttpClient.zig index 6112e1cbe..66d03e353 100644 --- a/src/browser/HttpClient.zig +++ b/src/browser/HttpClient.zig @@ -1078,9 +1078,11 @@ fn processOneMessage(self: *Client, msg: http.Handles.MultiMessage, transfer: *T } // Handle redirects: reuse the same connection to preserve TCP state. + // A 3xx without a Location header is not a redirect: per RFC 9110 §15.4 + // it's a final response and falls through so its body is delivered. if (msg.err == null) { const status = try msg.conn.getResponseCode(); - if (status >= 300 and status <= 399) { + if (status >= 300 and status <= 399 and msg.conn.getResponseHeader("location", 0) != null) { try transfer.handleRedirect(); const conn = transfer._conn.?; @@ -2034,7 +2036,10 @@ pub const Transfer = struct { log.err(.http, "getResponseCode", .{ .err = err, .source = "body callback" }); return http.writefunc_error; }; - if (status >= 300 and status <= 399) { + // Only skip the body when the 3xx will actually be retried as a + // redirect (it has a Location header). A 3xx without one is a + // final response whose body must be kept. + if (status >= 300 and status <= 399 and conn.getResponseHeader("location", 0) != null) { res.skip_body = true; return @intCast(chunk_len); } diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index ec8ad40a8..72974cf77 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -1310,6 +1310,34 @@ test "cdp.frame: navigate inherits original fragment across redirect" { } } +test "cdp.frame: navigate renders body of 3xx response without Location" { + // RFC 9110 §15.4: a 3xx response without a Location header is not a + // redirect — it's a final response whose body must be delivered, like + // any other status. It must not abort the navigation. + var ctx = try testing.context(); + defer ctx.deinit(); + + var bc = try ctx.loadBrowserContext(.{ .id = "BID-3XX", .url = "hi.html", .target_id = "FID-0000003XX0".* }); + + try ctx.processMessage(.{ + .id = 50, + .method = "Page.navigate", + .params = .{ .url = "http://127.0.0.1:9582/303-no-location" }, + }); + + var runner = try bc.session.runner(.{}); + try runner.wait(.{ .ms = 2000 }); + + const frame = bc.session.currentFrame() orelse unreachable; + try testing.expectEqualSlices(u8, "http://127.0.0.1:9582/303-no-location", frame.url); + + var ls: js.Local.Scope = undefined; + frame.js.localScope(&ls); + defer ls.deinit(); + const v = try ls.local.exec("document.title === 'landed' && document.body.innerText.includes('see other body')", null); + try testing.expect(v.toBool()); +} + test "cdp.frame: navigate to about:blank replaces a non-blank document" { // Regression test for #2363. Page.navigate("about:blank") issued against a // tab that already holds a real document must replace the active document diff --git a/src/testing.zig b/src/testing.zig index 3085f28c1..4ed8e0028 100644 --- a/src/testing.zig +++ b/src/testing.zig @@ -637,6 +637,17 @@ fn testHTTPHandler(req: *std.http.Server.Request) !void { }); } + if (std.mem.eql(u8, path, "/303-no-location")) { + // 3xx WITHOUT a Location header: not a redirect, a final response + // whose body must be delivered (RFC 9110 §15.4). + return req.respond("landed

see other body

", .{ + .status = .see_other, + .extra_headers = &.{ + .{ .name = "Content-Type", .value = "text/html; charset=utf-8" }, + }, + }); + } + if (std.mem.eql(u8, path, "/redirect-with-fragment")) { return req.respond("", .{ .status = .found, From 3686fd2600b3b6add0d486c0f2e1c881ca30fea4 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 12 Jun 2026 11:44:58 +0800 Subject: [PATCH 2/2] Stricter redirect status code check Single Location header lookup --- src/browser/HttpClient.zig | 69 ++++++++++++++++++++------------------ src/cdp/domains/page.zig | 29 ++++++++++++++++ src/testing.zig | 13 +++++++ 3 files changed, 79 insertions(+), 32 deletions(-) diff --git a/src/browser/HttpClient.zig b/src/browser/HttpClient.zig index 66d03e353..026974bcb 100644 --- a/src/browser/HttpClient.zig +++ b/src/browser/HttpClient.zig @@ -1045,6 +1045,13 @@ fn isTeardownMethod(method: []const u8) bool { std.mem.eql(u8, method, "Page.close"); } +fn isRedirectStatus(status: u16) bool { + return switch (status) { + 301, 302, 303, 307, 308 => true, + else => false, + }; +} + fn processOneMessage(self: *Client, msg: http.Handles.MultiMessage, transfer: *Transfer) !bool { // State at entry: .inflight = conn (multi just delivered a completion). if (msg.err == null or msg.err.? == error.RecvError) { @@ -1078,31 +1085,33 @@ fn processOneMessage(self: *Client, msg: http.Handles.MultiMessage, transfer: *T } // Handle redirects: reuse the same connection to preserve TCP state. - // A 3xx without a Location header is not a redirect: per RFC 9110 §15.4 - // it's a final response and falls through so its body is delivered. + // A redirect status without a Location header is not a redirect, it's a + // final response and falls through so its body is delivered. if (msg.err == null) { const status = try msg.conn.getResponseCode(); - if (status >= 300 and status <= 399 and msg.conn.getResponseHeader("location", 0) != null) { - try transfer.handleRedirect(); + if (isRedirectStatus(status)) { + if (msg.conn.getResponseHeader("location", 0)) |location| { + try transfer.handleRedirect(location.value); - const conn = transfer._conn.?; + const conn = transfer._conn.?; - try self.handles.remove(conn); - conn.debug_removed = 3; - // Conn temporarily out of multi during reconfigure. - // _detached_conn lets processMessages release it if any of - // the steps below throw. State stays .inflight; _conn stays set - transfer._detached_conn = conn; + try self.handles.remove(conn); + conn.debug_removed = 3; + // Conn temporarily out of multi during reconfigure. + // _detached_conn lets processMessages release it if any of + // the steps below throw. State stays .inflight; _conn stays set + transfer._detached_conn = conn; - transfer.reset(); - try transfer.configureConn(conn); - try self.handles.add(conn); - conn.debug_added = 2; - transfer._detached_conn = null; + transfer.reset(); + try transfer.configureConn(conn); + try self.handles.add(conn); + conn.debug_added = 2; + transfer._detached_conn = null; - _ = try self.perform(0); + _ = try self.perform(0); - return false; + return false; + } } } @@ -1864,7 +1873,7 @@ pub const Transfer = struct { self.req.url = url; } - fn handleRedirect(transfer: *Transfer) !void { + fn handleRedirect(transfer: *Transfer, location: []const u8) !void { const req = &transfer.req; const conn = transfer._conn.?; const arena = transfer.arena; @@ -1887,21 +1896,17 @@ pub const Transfer = struct { } // resolve the redirect target. - const location = conn.getResponseHeader("location", 0) orelse { - return error.LocationNotFound; - }; - const url: [:0]const u8 = blk: { - if (location.value.len == 0) { - // Might seem silly, but URL.resovle will return location.value as-is - // if empty, and location.value is memory owned by libcurl. + if (location.len == 0) { + // Might seem silly, but URL.resovle will return location as-is + // if empty, and location is memory owned by libcurl. break :blk ""; } const base_url = try conn.getEffectiveUrl(); - // base_url and location.value are owned by curl. The returned value + // base_url and location are owned by curl. The returned value // will be stored in transfer.req.url, hence the always_dupe. - const resolved = try URL.resolve(arena, std.mem.span(base_url), location.value, .{ .always_dupe = true }); + const resolved = try URL.resolve(arena, std.mem.span(base_url), location, .{ .always_dupe = true }); // RFC 7231 §7.1.2: if the Location value has no fragment, the redirect // inherits the fragment from the URI used to generate the request. @@ -2036,10 +2041,10 @@ pub const Transfer = struct { log.err(.http, "getResponseCode", .{ .err = err, .source = "body callback" }); return http.writefunc_error; }; - // Only skip the body when the 3xx will actually be retried as a - // redirect (it has a Location header). A 3xx without one is a - // final response whose body must be kept. - if (status >= 300 and status <= 399 and conn.getResponseHeader("location", 0) != null) { + // Only skip the body when the response will actually be retried + // as a redirect (a redirect status with a Location header). Any + // other 3xx is a final response whose body must be kept. + if (isRedirectStatus(status) and conn.getResponseHeader("location", 0) != null) { res.skip_body = true; return @intCast(chunk_len); } diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index 72974cf77..a6e229480 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -1338,6 +1338,35 @@ test "cdp.frame: navigate renders body of 3xx response without Location" { try testing.expect(v.toBool()); } +test "cdp.frame: navigate does not follow Location on a non-redirect 3xx" { + // The fetch standard's redirect statuses are exactly 301, 302, 303, 307 + // and 308. A 300 (Multiple Choices) may carry a Location header as a + // preference hint, but it is not a redirect: the response itself must be + // delivered and the Location must not be followed. + var ctx = try testing.context(); + defer ctx.deinit(); + + var bc = try ctx.loadBrowserContext(.{ .id = "BID-300L", .url = "hi.html", .target_id = "FID-0000003000".* }); + + try ctx.processMessage(.{ + .id = 51, + .method = "Page.navigate", + .params = .{ .url = "http://127.0.0.1:9582/300-with-location" }, + }); + + var runner = try bc.session.runner(.{}); + try runner.wait(.{ .ms = 2000 }); + + const frame = bc.session.currentFrame() orelse unreachable; + try testing.expectEqualSlices(u8, "http://127.0.0.1:9582/300-with-location", frame.url); + + var ls: js.Local.Scope = undefined; + frame.js.localScope(&ls); + defer ls.deinit(); + const v = try ls.local.exec("document.title === 'choices' && document.body.innerText.includes('multiple choices body')", null); + try testing.expect(v.toBool()); +} + test "cdp.frame: navigate to about:blank replaces a non-blank document" { // Regression test for #2363. Page.navigate("about:blank") issued against a // tab that already holds a real document must replace the active document diff --git a/src/testing.zig b/src/testing.zig index 4ed8e0028..57cf93ce9 100644 --- a/src/testing.zig +++ b/src/testing.zig @@ -648,6 +648,19 @@ fn testHTTPHandler(req: *std.http.Server.Request) !void { }); } + if (std.mem.eql(u8, path, "/300-with-location")) { + // A non-redirect 3xx (fetch's redirect statuses are only 301, 302, + // 303, 307 and 308) carrying a Location header: the header is a + // preference hint, not a redirect — the body must be delivered. + return req.respond("choices

multiple choices body

", .{ + .status = .multiple_choice, + .extra_headers = &.{ + .{ .name = "Content-Type", .value = "text/html; charset=utf-8" }, + .{ .name = "Location", .value = "http://127.0.0.1:9582/hi.html" }, + }, + }); + } + if (std.mem.eql(u8, path, "/redirect-with-fragment")) { return req.respond("", .{ .status = .found,