diff --git a/src/msgpack.zig b/src/msgpack.zig index 33b22ae..db78b5c 100644 --- a/src/msgpack.zig +++ b/src/msgpack.zig @@ -2847,6 +2847,7 @@ pub fn PackWithLimits( else std.ArrayList(ParseState){}; defer if (current_zig.minor == 14) parse_stack.deinit() else parse_stack.deinit(allocator); + errdefer cleanupParseStack(&parse_stack, allocator); // Root payload to return var root: ?Payload = null; @@ -2861,7 +2862,6 @@ pub fn PackWithLimits( while (true) { // Check depth limit if (parse_stack.items.len >= parse_limits.max_depth) { - cleanupParseStack(&parse_stack, allocator); return MsgPackError.MaxDepthExceeded; } @@ -2903,7 +2903,6 @@ pub fn PackWithLimits( // Validate string length if (val.len > parse_limits.max_string_length) { allocator.free(val); - cleanupParseStack(&parse_stack, allocator); return MsgPackError.StringTooLong; } @@ -2915,7 +2914,6 @@ pub fn PackWithLimits( // Validate binary length if (val.len > parse_limits.max_bin_length) { allocator.free(val); - cleanupParseStack(&parse_stack, allocator); return MsgPackError.BinDataLengthTooLong; } @@ -2928,7 +2926,6 @@ pub fn PackWithLimits( // Validate array length if (len > parse_limits.max_array_length) { - cleanupParseStack(&parse_stack, allocator); return MsgPackError.ArrayTooLarge; } @@ -2961,7 +2958,6 @@ pub fn PackWithLimits( // Validate map size if (len > parse_limits.max_map_size) { - cleanupParseStack(&parse_stack, allocator); return MsgPackError.MapTooLarge; } @@ -2975,7 +2971,6 @@ pub fn PackWithLimits( errdefer if (!map_owned) map.deinit(); const capacity = std.math.cast(u32, len) orelse { - cleanupParseStack(&parse_stack, allocator); return MsgPackError.MapTooLarge; }; try map.ensureTotalCapacity(capacity); diff --git a/src/test.zig b/src/test.zig index 6a3d308..b4e45ed 100644 --- a/src/test.zig +++ b/src/test.zig @@ -125,6 +125,50 @@ test "PackerIO: corrupted length field" { } } +test "PackerIO: truncated array cleanup" { + if (!has_new_io) return error.SkipZigTest; + + // Test that truncated array data is properly cleaned up on error + // This demonstrates the benefit of errdefer cleanupParseStack + var buffer: [50]u8 = undefined; + var input = if (builtin.zig_version.minor == 14) + std.ArrayList(u8).init(allocator) + else + std.ArrayList(u8){}; + defer if (builtin.zig_version.minor == 14) input.deinit() else input.deinit(allocator); + + // Create array header for 3 elements (0x93) + if (builtin.zig_version.minor == 14) { + try input.append(0x93); // fixarray with 3 elements + try input.append(0x01); // first element: uint 1 + try input.append(0x02); // second element: uint 2 + // Missing third element - truncated! + } else { + try input.append(allocator, 0x93); + try input.append(allocator, 0x01); + try input.append(allocator, 0x02); + } + + @memcpy(buffer[0..input.items.len], input.items); + + var writer = std.Io.Writer.fixed(&buffer); + var reader = std.Io.Reader.fixed(buffer[0..input.items.len]); + + var packer = msgpack.PackerIO.init(&reader, &writer); + + // Should fail due to truncated data (missing third element) + const result = packer.read(allocator); + if (result) |payload| { + payload.free(allocator); + try expect(false); // Should not succeed with truncated array + } else |err| { + // Expected error - truncated array cannot be fully read + try expect(err == msgpack.MsgPackError.TypeMarkerReading or + err == msgpack.MsgPackError.DataReading or + err == error.EndOfStream); + } +} + test "PackerIO: multiple payloads with error recovery" { if (!has_new_io) return error.SkipZigTest;