From 5b12c42df2a089b9105c8aa9100f9901c80a63b0 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 22 Jul 2026 14:59:33 +0800 Subject: [PATCH] cdp: Sanitize non-UTF 8 values Builds on https://github.com/lightpanda-io/browser/pull/2972 in order to fix https://github.com/lightpanda-io/browser/issues/2992 I couldn't think of a generic way to fix this. Created a SafeString which wraps a `[]const u8` with a custom jsonStringify function. This is now applied to both header names and values. The existing latin1 to utf8 conversion is now extracted into string.js and used in Frame for the filename. This was moved to Frame because the same issue could cause issues trying to save the file. --- src/browser/Frame.zig | 12 +++- src/cdp/SafeString.zig | 126 ++++++++++++++++++++++++++++++++++++ src/cdp/domains/network.zig | 76 +++------------------- src/string.zig | 50 ++++++++++++++ 4 files changed, 194 insertions(+), 70 deletions(-) create mode 100644 src/cdp/SafeString.zig diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index c15abd7e6..dc311b53e 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -1286,6 +1286,9 @@ fn frameHeaderDoneCallback(transfer: *HttpClient.Transfer) !HttpClient.Transfer. // (allow/allowAndName) and the response carries Content-Disposition: attachment. // See issue #2701. fn maybeStartDownload(self: *Frame, transfer: *HttpClient.Transfer) !bool { + const uuidv4 = @import("../id.zig").uuidv4; + const latin1ToUtf8 = @import("../string.zig").latin1ToUtf8; + const session = self._session; switch (session.download_behavior) { .allow, .allow_and_name => {}, @@ -1313,11 +1316,16 @@ fn maybeStartDownload(self: *Frame, transfer: *HttpClient.Transfer) !bool { // `guid` is the CDP "Global Unique Identifier" that ties the // downloadWillBegin / downloadProgress events to one download. var guid_buf: [36]u8 = undefined; - @import("../id.zig").uuidv4(&guid_buf); + uuidv4(&guid_buf); const guid = try self.arena.dupe(u8, &guid_buf); + // Content-Disposition filenames can be in a legacy encoding (e.g. + // Shift_JIS) but must yield valid UTF-8 const suggested = dispositionFilename(disposition) orelse (try urlBasename(self.arena, self.url)) orelse guid; - const suggested_filename = try self.arena.dupe(u8, suggested); + const suggested_filename = if (std.unicode.utf8ValidateSlice(suggested)) + try self.arena.dupe(u8, suggested) + else + try latin1ToUtf8(self.arena, suggested); // allowAndName stores the file under its guid; allow uses the suggested name. const on_disk_name = switch (session.download_behavior) { diff --git a/src/cdp/SafeString.zig b/src/cdp/SafeString.zig new file mode 100644 index 000000000..ceef3c720 --- /dev/null +++ b/src/cdp/SafeString.zig @@ -0,0 +1,126 @@ +// Copyright (C) 2023-2026 Lightpanda (Selecy SAS) +// +// Francis Bouvier +// Pierre Tachoire +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +/// Wraps bytes that may not be valid UTF-8 — raw network octets like header +/// values. std.json serializes invalid UTF-8 as a JSON array of numbers, +/// which crashes CDP clients that expect a string (#2972, #2992). Non-UTF-8 +/// bytes are interpreted as Latin-1 (ISO-8859-1) and transcoded, matching +/// Chrome's behavior for DevTools. +const std = @import("std"); + +const SafeString = @This(); + +bytes: []const u8, + +pub fn wrap(bytes: []const u8) SafeString { + return .{ .bytes = bytes }; +} + +pub fn jsonStringify(self: *const SafeString, jws: anytype) !void { + if (std.unicode.utf8ValidateSlice(self.bytes)) { + return jws.write(self.bytes); + } + try jws.beginWriteRaw(); + try writeQuoted(jws, self.bytes); + jws.endWriteRaw(); +} + +// Object keys can't take std.json's byte-array fallback: objectField writes +// invalid UTF-8 straight into the frame, and RFC 6455 lets strict clients +// kill the connection over an invalid UTF-8 text frame. +pub fn writeObjectField(jws: anytype, name: []const u8) !void { + if (std.unicode.utf8ValidateSlice(name)) { + return jws.objectField(name); + } + try jws.beginObjectFieldRaw(); + try writeQuoted(jws, name); + jws.endObjectFieldRaw(); +} + +// Latin-1 -> UTF-8: each byte is a codepoint U+0000..U+00FF (max 2 bytes) +fn writeQuoted(jws: anytype, value: []const u8) !void { + try jws.writer.writeByte('"'); + var start: usize = 0; + for (value, 0..) |b, i| { + if (b < 0x80) { + continue; + } + try std.json.Stringify.encodeJsonStringChars(value[start..i], jws.options, jws.writer); + var buf: [2]u8 = undefined; + const n = std.unicode.utf8Encode(b, &buf) catch unreachable; + try jws.writer.writeAll(buf[0..n]); + start = i + 1; + } + try std.json.Stringify.encodeJsonStringChars(value[start..], jws.options, jws.writer); + try jws.writer.writeByte('"'); +} + +test "cdp.SafeString: jsonStringify" { + const expectJson = struct { + fn expect(expected: []const u8, value: []const u8) !void { + var buf: [256]u8 = undefined; + var writer = std.Io.Writer.fixed(&buf); + var jws: std.json.Stringify = .{ .writer = &writer }; + try jws.write(wrap(value)); + try std.testing.expectEqualStrings(expected, writer.buffered()); + } + }.expect; + + // valid UTF-8 is written as-is + try expectJson( + "\"mié, 15 jul 2026 13:19:10 GMT\"", + "mié, 15 jul 2026 13:19:10 GMT", + ); + + // Latin-1 bytes are transcoded to UTF-8 instead of a byte array + try expectJson( + "\"mié, 15 jul 2026 13:19:10 GMT\"", + "mi\xE9, 15 jul 2026 13:19:10 GMT", + ); + + // JSON escaping still applies around transcoded bytes + try expectJson( + "\"a\\\"é\\nb\"", + "a\"\xE9\nb", + ); + + // pure ASCII untouched + try expectJson( + "\"max-age=180, s-maxage=180, public\"", + "max-age=180, s-maxage=180, public", + ); +} + +test "cdp.SafeString: writeObjectField" { + const expectJson = struct { + fn expect(expected: []const u8, name: []const u8) !void { + var buf: [256]u8 = undefined; + var writer = std.Io.Writer.fixed(&buf); + var jws: std.json.Stringify = .{ .writer = &writer }; + try jws.beginObject(); + try writeObjectField(&jws, name); + try jws.write(true); + try jws.endObject(); + try std.testing.expectEqualStrings(expected, writer.buffered()); + } + }.expect; + + try expectJson("{\"etag\":true}", "etag"); + try expectJson("{\"naïve\":true}", "na\xEFve"); + try expectJson("{\"a\\\"é\":true}", "a\"\xE9"); +} diff --git a/src/cdp/domains/network.zig b/src/cdp/domains/network.zig index 89db15679..23152cda8 100644 --- a/src/cdp/domains/network.zig +++ b/src/cdp/domains/network.zig @@ -21,6 +21,7 @@ const lp = @import("lightpanda"); const id = @import("../id.zig"); const CDP = @import("../CDP.zig"); +const SafeString = @import("../SafeString.zig"); const Config = @import("../../Config.zig"); const URL = @import("../../browser/URL.zig"); @@ -282,7 +283,9 @@ fn getResponseBody(cmd: *CDP.Command) !void { const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; const resp = bc.captured_responses.getPtr(key) orelse return error.RequestNotFound; - if (!resp.must_encode) { + // must_encode trusts the declared charset; a server can declare UTF-8 and + // still send invalid bytes. + if (!resp.must_encode and std.unicode.utf8ValidateSlice(resp.data.items)) { return cmd.sendResult(.{ .body = resp.data.items, .base64Encoded = false, @@ -435,8 +438,8 @@ pub const RequestWriter = struct { try jws.beginObject(); var it = request.headers.iterator(); while (it.next()) |hdr| { - try jws.objectField(hdr.name); - try writeHeaderValue(jws, hdr.value); + try SafeString.writeObjectField(jws, hdr.name); + try jws.write(SafeString.wrap(hdr.value)); } if (try request.getCookieString(transfer.arena)) |cookies| { try jws.objectField("Cookie"); @@ -540,8 +543,8 @@ const ResponseWriter = struct { try jws.beginObject(); var map_it = map.iterator(); while (map_it.next()) |entry| { - try jws.objectField(entry.key_ptr.*); - try writeHeaderValue(jws, entry.value_ptr.*); + try SafeString.writeObjectField(jws, entry.key_ptr.*); + try jws.write(SafeString.wrap(entry.value_ptr.*)); } try jws.endObject(); } @@ -549,33 +552,6 @@ const ResponseWriter = struct { } }; -// HTTP header values are octets; per historical practice non-UTF-8 bytes are -// interpreted as Latin-1 (ISO-8859-1), which is what Chrome does for DevTools. -// Transcode so we emit a JSON string — std.json would otherwise serialize -// invalid UTF-8 as a JSON array of numbers. -fn writeHeaderValue(jws: anytype, value: []const u8) !void { - if (std.unicode.utf8ValidateSlice(value)) { - return jws.write(value); - } - // Latin-1 -> UTF-8: each byte is a codepoint U+0000..U+00FF (max 2 bytes) - try jws.beginWriteRaw(); - try jws.writer.writeByte('"'); - var start: usize = 0; - for (value, 0..) |b, i| { - if (b < 0x80) { - continue; - } - try std.json.Stringify.encodeJsonStringChars(value[start..i], jws.options, jws.writer); - var buf: [2]u8 = undefined; - const n = std.unicode.utf8Encode(b, &buf) catch unreachable; - try jws.writer.writeAll(buf[0..n]); - start = i + 1; - } - try std.json.Stringify.encodeJsonStringChars(value[start..], jws.options, jws.writer); - try jws.writer.writeByte('"'); - jws.endWriteRaw(); -} - fn keyFromRequestId(request_id: []const u8) !CDP.BrowserContext.CapturedResponseKey { const key = std.fmt.parseInt(u32, request_id[4..], 10) catch return error.InvalidParams; @@ -712,42 +688,6 @@ test "cdp.network setExtraHTTPHeaders rejects a header that smuggles CRLF" { try testing.expectEqual("x-keep: ok", std.mem.span(bc.extra_headers.items[0])); } -test "cdp.network writeHeaderValue" { - const expectHeaderJson = struct { - fn expect(expected: []const u8, value: []const u8) !void { - var buf: [256]u8 = undefined; - var writer = std.Io.Writer.fixed(&buf); - var jws: std.json.Stringify = .{ .writer = &writer }; - try writeHeaderValue(&jws, value); - try std.testing.expectEqualStrings(expected, writer.buffered()); - } - }.expect; - - // valid UTF-8 is written as-is - try expectHeaderJson( - "\"mié, 15 jul 2026 13:19:10 GMT\"", - "mié, 15 jul 2026 13:19:10 GMT", - ); - - // Latin-1 bytes are transcoded to UTF-8 instead of a byte array - try expectHeaderJson( - "\"mié, 15 jul 2026 13:19:10 GMT\"", - "mi\xE9, 15 jul 2026 13:19:10 GMT", - ); - - // JSON escaping still applies around transcoded bytes - try expectHeaderJson( - "\"a\\\"é\\nb\"", - "a\"\xE9\nb", - ); - - // pure ASCII untouched - try expectHeaderJson( - "\"max-age=180, s-maxage=180, public\"", - "max-age=180, s-maxage=180, public", - ); -} - test "cdp.Network: cookies" { const ResCookie = CdpStorage.ResCookie; const CdpCookie = CdpStorage.CdpCookie; diff --git a/src/string.zig b/src/string.zig index 181371114..b62458094 100644 --- a/src/string.zig +++ b/src/string.zig @@ -17,7 +17,9 @@ // along with this program. If not, see . const std = @import("std"); + const Allocator = std.mem.Allocator; +const IS_DEBUG = @import("builtin").mode == .Debug; const M = @This(); @@ -409,6 +411,40 @@ pub fn truncateUtf8(bytes: []const u8, max_bytes: usize) []const u8 { return bytes[0..i]; } +/// Reinterprets `bytes` as Latin-1 (each byte one codepoint) and encodes it +/// as UTF-8. For bytes that aren't valid UTF-8 but must become a valid UTF-8 +/// string (JSON, filenames). +pub fn latin1ToUtf8(allocator: Allocator, bytes: []const u8) ![]u8 { + var extra: usize = 0; + for (bytes) |b| { + if (b >= 0x80) { + extra += 1; + } + } + if (comptime IS_DEBUG) { + // The way this is currently used: + // 1 - the caller always wants the value duped, + // 2 - the caller only got here because utf8ValidateSlice failed. + // If both of those ever change, maybe it's worth reconsidering whether + // this API unconditionally dupes. + std.debug.assert(extra != 0); + } + + const out = try allocator.alloc(u8, bytes.len + extra); + var i: usize = 0; + for (bytes) |b| { + if (b < 0x80) { + out[i] = b; + i += 1; + } else { + out[i] = 0xC0 | (b >> 6); + out[i + 1] = 0x80 | (b & 0x3F); + i += 2; + } + } + return out; +} + // Discriminatory type that signals the bridge to use arena instead of call_arena // Use this for strings that need to persist beyond the current call // The caller can unwrap and store just the underlying .str field @@ -455,6 +491,20 @@ test "truncateUtf8" { try testing.expectEqual("\xFFx", truncateUtf8("\xFFx", 2)); } +test "latin1ToUtf8" { + const cases = [_]struct { in: []const u8, out: []const u8 }{ + .{ .in = "caf\xE9.txt", .out = "café.txt" }, + .{ .in = "report\xFF.csv", .out = "report\xC3\xBF.csv" }, + .{ .in = "\x82\xd3\x82\xe9.xlsx", .out = "\xC2\x82\xC3\x93\xC2\x82\xC3\xA9.xlsx" }, + }; + for (cases) |case| { + const out = try latin1ToUtf8(testing.allocator, case.in); + defer testing.allocator.free(out); + try testing.expectEqual(case.out, out); + try testing.expectEqual(true, std.unicode.utf8ValidateSlice(out)); + } +} + test "String" { const other_short = try String.init(undefined, "other_short", .{}); const other_long = try String.init(testing.allocator, "other_long" ** 100, .{});