From ece81a96418ddffd929000292f7719ce1ece41f3 Mon Sep 17 00:00:00 2001 From: jcalabro Date: Sat, 4 Apr 2026 07:52:28 -0400 Subject: [PATCH] add errdefer for decode allocations, verify with checkAllAllocationFailures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add errdefer allocator.free() for array items and map entries slices in decodeAt so partial allocations are cleaned up on error. Add 3 tests using std.testing.checkAllAllocationFailures to exhaustively verify that every allocation failure in decode (flat map, nested record, array) is handled without leaking — tests use ArenaAllocator over the failing allocator to match the intended usage pattern. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/internal/repo/cbor.zig | 2 ++ src/internal/repo/cbor_test.zig | 62 +++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/src/internal/repo/cbor.zig b/src/internal/repo/cbor.zig index a8d3f8c..f6cfd1b 100644 --- a/src/internal/repo/cbor.zig +++ b/src/internal/repo/cbor.zig @@ -315,6 +315,7 @@ fn decodeAt(allocator: Allocator, data: []const u8, pos: *usize, depth: usize) D // sanity check: each element is at least 1 byte if (arg.val > data.len - pos.*) return error.UnexpectedEof; const items = try allocator.alloc(Value, @intCast(arg.val)); + errdefer allocator.free(items); for (items) |*item| { item.* = try decodeAt(allocator, data, pos, depth + 1); } @@ -325,6 +326,7 @@ fn decodeAt(allocator: Allocator, data: []const u8, pos: *usize, depth: usize) D // sanity check: each entry is at least 2 bytes (key + value) if (arg.val > (data.len - pos.*) / 2) return error.UnexpectedEof; const entries = try allocator.alloc(Value.MapEntry, @intCast(arg.val)); + errdefer allocator.free(entries); for (entries, 0..) |*entry, i| { // DAG-CBOR: map keys must be text strings — inline read to avoid // a full decodeAt + Value union construction per key diff --git a/src/internal/repo/cbor_test.zig b/src/internal/repo/cbor_test.zig index 6b048ee..a311ab7 100644 --- a/src/internal/repo/cbor_test.zig +++ b/src/internal/repo/cbor_test.zig @@ -1209,6 +1209,68 @@ test "reject tag 42 with only prefix + 1 byte (too short for CID)" { })); } +// === allocation failure safety (checkAllAllocationFailures) === + +fn decodeSimpleImpl(backing: std.mem.Allocator, data: []const u8) !void { + // use an arena over the backing allocator — checkAllAllocationFailures + // tracks the backing allocator's alloc/free calls. the arena batches + // frees on deinit, so OOM from any arena allocation correctly frees + // everything allocated so far. + var arena = std.heap.ArenaAllocator.init(backing); + defer arena.deinit(); + _ = try cbor.decodeAll(arena.allocator(), data); +} + +test "checkAllAllocationFailures: decode flat map" { + var setup_arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer setup_arena.deinit(); + const encoded = try cbor.encodeAlloc(setup_arena.allocator(), .{ .map = &.{ + .{ .key = "a", .value = .{ .unsigned = 1 } }, + .{ .key = "b", .value = .{ .text = "hello" } }, + } }); + + try std.testing.checkAllAllocationFailures(std.testing.allocator, decodeSimpleImpl, .{encoded}); +} + +fn decodeNestedImpl(backing: std.mem.Allocator, data: []const u8) !void { + var arena = std.heap.ArenaAllocator.init(backing); + defer arena.deinit(); + _ = try cbor.decodeAll(arena.allocator(), data); +} + +test "checkAllAllocationFailures: decode nested record" { + var setup_arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer setup_arena.deinit(); + const sa = setup_arena.allocator(); + + const record: Value = .{ .map = &.{ + .{ .key = "$type", .value = .{ .text = "app.bsky.feed.post" } }, + .{ .key = "langs", .value = .{ .array = &.{.{ .text = "en" }} } }, + .{ .key = "text", .value = .{ .text = "hello" } }, + } }; + const encoded = try cbor.encodeAlloc(sa, record); + + try std.testing.checkAllAllocationFailures(std.testing.allocator, decodeNestedImpl, .{encoded}); +} + +fn decodeArrayImpl(backing: std.mem.Allocator, data: []const u8) !void { + var arena = std.heap.ArenaAllocator.init(backing); + defer arena.deinit(); + _ = try cbor.decodeAll(arena.allocator(), data); +} + +test "checkAllAllocationFailures: decode array" { + var setup_arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer setup_arena.deinit(); + const encoded = try cbor.encodeAlloc(setup_arena.allocator(), .{ .array = &.{ + .{ .unsigned = 1 }, + .{ .unsigned = 2 }, + .{ .text = "three" }, + } }); + + try std.testing.checkAllAllocationFailures(std.testing.allocator, decodeArrayImpl, .{encoded}); +} + // === negative integer encode round-trip at min i64 === test "round-trip encode min i64" { -- 2.51.2