From 210f7d75259d6491979120eddaf4759482df0bb4 Mon Sep 17 00:00:00 2001 From: Lukas Lalinsky Date: Fri, 16 Oct 2026 22:33:35 +0200 Subject: [PATCH] fix: decode numbers from a reader whose buffer is smaller than them takeInt guarded Reader.takeInt with a check that the reader's buffer can hold the integer, because on a miss takeInt rebases, which asserts that. So a streaming reader with a buffer under 8 bytes failed every u64 or f64 with ReaderBufferTooSmall, even though nothing needs those bytes to be buffered together. Read the integer straight from the buffer when it is all there, and copy it out with readSliceAll otherwise, which works across refills. ReaderBufferTooSmall is now only returned for strings that have to be buffered whole, such as struct field names. --- CHANGELOG.md | 3 +++ src/msgpack.zig | 24 ++++++++++++++++-------- src/utils.zig | 29 ++++++++++++++++++++++------- 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07f93fd..3112f94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Encoding writes straight into the writer's buffer when there is room, and keeps the drain path out of line, which is about 13% cheaper on a typical struct and 37% on an array of integers - Encoding a struct is about 25% cheaper again, by telling the compiler that writing into the buffer cannot change the writer itself +### Fixed +- Decoding numbers from a streaming reader whose buffer is smaller than the number (under 8 bytes) works, instead of failing with `error.ReaderBufferTooSmall`; that error is now only for strings that have to be buffered whole, such as struct field names + ## [0.9.0] - 2026-09-05 ### Changed diff --git a/src/msgpack.zig b/src/msgpack.zig index da693ca..757e286 100644 --- a/src/msgpack.zig +++ b/src/msgpack.zig @@ -278,7 +278,8 @@ pub const SliceDecodeError = error{ pub const DecodeError = SliceDecodeError || error{ /// The reader failed, and its own error says why. ReadFailed, - /// A value had to be buffered whole, and the reader's buffer is smaller. + /// A string had to be buffered whole, such as a struct field name, and + /// the reader's buffer is smaller. ReaderBufferTooSmall, }; @@ -453,21 +454,28 @@ test "decode from a streaming reader whose buffer holds the largest value" { } } -test "decode from a streaming reader with too small a buffer errors, never panics" { - const Numeric = struct { id: u64, seq: i32, ratio: f64, ok: bool }; +test "decode numbers from a streaming reader whose buffer is smaller than them" { + const Numeric = struct { + id: u64, + seq: i32, + ratio: f64, + ok: bool, + + pub fn msgpackFormat() StructFormat { + return .{ .as_array = .{} }; + } + }; const value = Numeric{ .id = 0xdead_beef_cafe_1234, .seq = -4242, .ratio = 3.14159, .ok = true }; var encoded: [64]u8 = undefined; const bytes = try encodeToBuffer(value, &encoded); - // The u64 payload needs 8 buffered bytes; anything smaller cannot rebase. + // The u64 and f64 payloads are 8 bytes, more than any of these buffers. for ([_]usize{ 1, 2, 4, 7 }) |buffer_len| { var buffer: [8]u8 = undefined; var trickle = TrickleReader.init(buffer[0..buffer_len], bytes); - try std.testing.expectError( - error.ReaderBufferTooSmall, - decodeLeaky(Numeric, std.testing.allocator, &trickle.reader), - ); + const decoded = try decodeLeaky(Numeric, std.testing.allocator, &trickle.reader); + try std.testing.expectEqualDeep(value, decoded); } } diff --git a/src/utils.zig b/src/utils.zig index 8dc7b01..1bc4c7f 100644 --- a/src/utils.zig +++ b/src/utils.zig @@ -62,15 +62,30 @@ pub noinline fn writeAllSlow(writer: *std.Io.Writer, bytes: []const u8) std.Io.W return writer.writeAll(bytes); } -/// Reads a big-endian `T` from the reader. `Reader.takeInt` rebases, which -/// asserts the reader's buffer can hold `@sizeOf(T)`; report that as an error -/// rather than letting the assert fire. +/// Reads a big-endian `T` from the reader. When the bytes are already +/// buffered, which is always the case when decoding from a slice, this is one +/// compare and a load. `Reader.takeInt` would check the same thing, but it +/// rebases on a miss, which asserts the reader's buffer can hold `@sizeOf(T)`, +/// so it needed a check of the buffer's capacity in front of it as well. pub inline fn takeInt(reader: *std.Io.Reader, comptime T: type) !T { - if (reader.buffer.len < @divExact(@bitSizeOf(T), 8)) { - @branchHint(.unlikely); - return error.ReaderBufferTooSmall; + const n = @divExact(@bitSizeOf(T), 8); + const buffered = reader.buffered(); + if (buffered.len >= n) { + @branchHint(.likely); + reader.toss(n); + return std.mem.readInt(T, buffered[0..n], .big); } - return reader.takeInt(T, .big); + return takeIntSlow(reader, T); +} + +/// The fallback of `takeInt`, for when the bytes are not all buffered. Copying +/// them out with `readSliceAll` works across refills, so a reader whose buffer +/// is smaller than `T` still works. +noinline fn takeIntSlow(reader: *std.Io.Reader, comptime T: type) !T { + @branchHint(.cold); + var bytes: [@divExact(@bitSizeOf(T), 8)]u8 = undefined; + try reader.readSliceAll(&bytes); + return std.mem.readInt(T, &bytes, .big); } /// Copies `dest.len` bytes out of the reader. When they are already buffered,