diff --git a/src/browser/HttpClient.zig b/src/browser/HttpClient.zig index 6112e1cbe..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,29 +1085,33 @@ fn processOneMessage(self: *Client, msg: http.Handles.MultiMessage, transfer: *T } // Handle redirects: reuse the same connection to preserve TCP state. + // 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) { - 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; + } } } @@ -1862,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; @@ -1885,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. @@ -2034,7 +2041,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 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 c1018ec40..30fc699b2 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -1311,6 +1311,63 @@ 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 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 3085f28c1..57cf93ce9 100644 --- a/src/testing.zig +++ b/src/testing.zig @@ -637,6 +637,30 @@ 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, "/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,