diff --git a/src/msgpack.zig b/src/msgpack.zig index b3a7106..7327751 100644 --- a/src/msgpack.zig +++ b/src/msgpack.zig @@ -865,16 +865,24 @@ fn clonePayload(payload: Payload, allocator: Allocator) !Payload { .arr => |arr| { const new_arr = try allocator.alloc(Payload, arr.len); - errdefer allocator.free(new_arr); + var cloned_count: usize = 0; + errdefer { + // cleanup partial clones on error + for (new_arr[0..cloned_count]) |item| { + item.free(allocator); + } + allocator.free(new_arr); + } for (arr, 0..) |item, i| { new_arr[i] = try clonePayload(item, allocator); + cloned_count += 1; } return Payload{ .arr = new_arr }; }, .map => |m| { var new_map = Map.init(allocator); - errdefer new_map.deinit(); + errdefer (Payload{ .map = new_map }).free(allocator); // Clone all entries var it = m.map.iterator(); diff --git a/src/test.zig b/src/test.zig index 69f9f51..19e5637 100644 --- a/src/test.zig +++ b/src/test.zig @@ -1187,7 +1187,7 @@ test "payload utility methods" { const cloned_str_payload = try str_payload.deepClone(allocator); defer cloned_str_payload.free(allocator); try expect(u8eql("test", cloned_str_payload.str.value())); - try expect(@ptrToInt(str_payload.str.value().ptr) != @ptrToInt(cloned_str_payload.str.value().ptr)); + try expect(@intFromPtr(str_payload.str.value().ptr) != @intFromPtr(cloned_str_payload.str.value().ptr)); const arr_payload = try Payload.arrPayload(3, allocator); defer arr_payload.free(allocator); @@ -3613,6 +3613,54 @@ test "iterative parser: deep nested maps" { try expect(decoded == .map); } +test "clonePayload arr partial fail path frees partially cloned elements" { + var src = try Payload.arrPayload(2, std.heap.page_allocator); + defer src.free(std.heap.page_allocator); + + try src.setArrElement(0, try Payload.strToPayload("a", std.heap.page_allocator)); + try src.setArrElement(1, try Payload.strToPayload("this-is-a-very-large-string-that-will-always-fail-because-it-is-much-longer-than-the-fixed-buffer-can-handle-and-should-cause-out-of-memory-error-during-cloning-process", std.heap.page_allocator)); + + var buffer: [@sizeOf(Payload) * 2 + 8]u8 = undefined; + var pool = std.heap.FixedBufferAllocator.init(&buffer); + const clone_alloc = pool.allocator(); + + const result = src.deepClone(clone_alloc); + if (result) |cloned| { + cloned.free(clone_alloc); + try std.testing.expect(false); + } else |err| { + try std.testing.expect(err == error.OutOfMemory); + } + + // Verify post-failure allocator state is valid: next alloc must succeed. + const next_alloc = try clone_alloc.alloc(u8, 16); + clone_alloc.free(next_alloc); +} + +test "clonePayload map partial fail path frees partially cloned entries" { + var src = Payload.mapPayload(std.heap.page_allocator); + defer src.free(std.heap.page_allocator); + + try src.mapPut("k1", try Payload.strToPayload("v1", std.heap.page_allocator)); + try src.mapPut("k2", try Payload.strToPayload("very-long-string-to-force-out-of-memory-on-clone-because-the-fixed-buffer-is-too-small-to-allocate-this-large-string-during-the-cloning-operation", std.heap.page_allocator)); + + var buffer: [@sizeOf(Payload) * 2 + 16]u8 = undefined; + var pool = std.heap.FixedBufferAllocator.init(&buffer); + const clone_alloc = pool.allocator(); + + const result = src.deepClone(clone_alloc); + if (result) |cloned| { + cloned.free(clone_alloc); + try std.testing.expect(false); + } else |err| { + try std.testing.expect(err == error.OutOfMemory); + } + + // Verify post-failure allocator state is valid: next alloc must succeed. + const next_alloc = try clone_alloc.alloc(u8, 16); + clone_alloc.free(next_alloc); +} + test "iterative free: deeply nested payload" { // Create a deeply nested structure in memory var root = try Payload.arrPayload(1, allocator);