zzstoatzz.io / logfire-zig
logfire client for zig
fork atom

fix: correct copy semantics for string attributes

zig's struct copy semantics caused dangling pointers when Span.Data
was copied (in span.end()). the Value.string slice pointed to the
original struct's _string_storage, not the copy's.

fix: don't store a slice in Value.string at all. store string data in
_string_storage and reconstruct the slice via getString() on access.
this ensures the slice always points to the current struct's storage.

- Value.string is now just a tag (no payload)
- added getString() method to reconstruct slice from internal storage
- copyString() no longer returns a slice
- all consumers now use getString() instead of .string
- for loops capture |*attr| to get pointers

added test verifying copy safety: the getString() pointer comes from
the copy's _string_storage, not the original's.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

zzstoatzz.io 817d230f b792c86b

+61 -28
unified split
+53 -20
src/attribute.zig
··· 2 2 //! 3 3 //! converts zig values to OTLP-compatible attribute format. 4 4 //! string values are copied into internal storage for memory safety. 5 + //! 6 + //! NOTE: string values are stored in _string_storage, NOT as a slice in Value. 7 + //! this is intentional - zig's struct copy semantics would leave dangling pointers 8 + //! if we stored a slice pointing to _string_storage. instead, we store the length 9 + //! and reconstruct the slice via getString() when needed. 5 10 6 11 const std = @import("std"); 7 12 ··· 14 19 15 20 pub const max_string_len = 512; 16 21 22 + /// value types - NOTE: string payload is in _string_storage, not here 23 + /// this ensures correct behavior when Attribute is copied 17 24 pub const Value = union(enum) { 18 - string: []const u8, 25 + string, // string data is in _string_storage[0.._string_len] 19 26 int: i64, 20 27 float: f64, 21 28 bool_val: bool, 22 29 }; 23 30 24 - /// copy a string into internal storage and return slice pointing to it 25 - fn copyString(self: *Attribute, str: []const u8) []const u8 { 31 + /// get string value - reconstructs slice from internal storage 32 + /// returns null if value is not a string 33 + pub fn getString(self: *const Attribute) ?[]const u8 { 34 + return switch (self.value) { 35 + .string => self._string_storage[0..self._string_len], 36 + else => null, 37 + }; 38 + } 39 + 40 + /// copy a string into internal storage 41 + fn copyString(self: *Attribute, str: []const u8) void { 26 42 const len = @min(str.len, max_string_len); 27 43 @memcpy(self._string_storage[0..len], str[0..len]); 28 44 self._string_len = len; 29 - return self._string_storage[0..len]; 30 45 } 31 46 32 47 /// convert a comptime struct to attributes array ··· 73 88 }, 74 89 .pointer => |ptr| { 75 90 if (ptr.size == .slice and ptr.child == u8) { 76 - // copy the string into internal storage 77 - attr.* = .{ .key = key, .value = undefined }; 78 - const copied = attr.copyString(value); 79 - attr.value = .{ .string = copied }; 91 + // copy string into internal storage, tag value as string 92 + attr.* = .{ .key = key, .value = .string }; 93 + attr.copyString(value); 80 94 return true; 81 95 } 82 96 if (ptr.size == .one) { 83 97 const child_info = @typeInfo(ptr.child); 84 98 if (child_info == .array and child_info.array.child == u8) { 85 - attr.* = .{ .key = key, .value = undefined }; 86 - const copied = attr.copyString(value); 87 - attr.value = .{ .string = copied }; 99 + attr.* = .{ .key = key, .value = .string }; 100 + attr.copyString(value); 88 101 return true; 89 102 } 90 103 } ··· 92 105 }, 93 106 .array => |arr| { 94 107 if (arr.child == u8) { 95 - attr.* = .{ .key = key, .value = undefined }; 96 - const copied = attr.copyString(&value); 97 - attr.value = .{ .string = copied }; 108 + attr.* = .{ .key = key, .value = .string }; 109 + attr.copyString(&value); 98 110 return true; 99 111 } 100 112 return false; ··· 110 122 } 111 123 112 124 /// write attribute as OTLP JSON 113 - pub fn writeJson(self: Attribute, w: anytype) !void { 125 + pub fn writeJson(self: *const Attribute, w: anytype) !void { 114 126 try w.writeAll("{\"key\":"); 115 127 try writeJsonString(w, self.key); 116 128 try w.writeAll(",\"value\":{"); 117 129 118 130 switch (self.value) { 119 - .string => |s| { 131 + .string => { 120 132 try w.writeAll("\"stringValue\":"); 121 - try writeJsonString(w, s); 133 + try writeJsonString(w, self.getString().?); 122 134 }, 123 135 .int => |i| { 124 136 try w.print("\"intValue\":\"{d}\"", .{i}); ··· 163 175 164 176 try std.testing.expectEqual(@as(usize, 3), count); 165 177 try std.testing.expectEqualStrings("name", attrs[0].key); 166 - try std.testing.expectEqualStrings("test", attrs[0].value.string); 178 + try std.testing.expectEqualStrings("test", attrs[0].getString().?); 167 179 try std.testing.expectEqual(@as(i64, 42), attrs[1].value.int); 168 180 try std.testing.expectEqual(true, attrs[2].value.bool_val); 169 181 } ··· 171 183 test "writeJson string" { 172 184 var buf: [256]u8 = undefined; 173 185 var fbs = std.io.fixedBufferStream(&buf); 174 - const attr = Attribute{ .key = "foo", .value = .{ .string = "bar" } }; 175 - try attr.writeJson(fbs.writer()); 186 + // create attr with string value via fromStruct 187 + var attrs: [1]Attribute = undefined; 188 + _ = Attribute.fromStruct(.{ .foo = "bar" }, &attrs); 189 + try attrs[0].writeJson(fbs.writer()); 176 190 try std.testing.expectEqualStrings("{\"key\":\"foo\",\"value\":{\"stringValue\":\"bar\"}}", fbs.getWritten()); 177 191 } 192 + 193 + test "copy safety - string survives struct copy" { 194 + // this test verifies the fix for dangling pointer after struct copy 195 + var original: [1]Attribute = undefined; 196 + _ = Attribute.fromStruct(.{ .query = "hello world" }, &original); 197 + 198 + // copy the struct (this is what span.end() does internally) 199 + const copy = original; 200 + 201 + // the copy should have valid string data 202 + try std.testing.expectEqualStrings("query", copy[0].key); 203 + try std.testing.expectEqualStrings("hello world", copy[0].getString().?); 204 + 205 + // verify the copy's getString returns data from the copy's _string_storage 206 + // not from original (which would be a dangling pointer bug) 207 + const copy_str_ptr = copy[0].getString().?.ptr; 208 + const copy_storage_ptr = &copy[0]._string_storage; 209 + try std.testing.expect(copy_str_ptr == copy_storage_ptr); 210 + }
+8 -8
src/exporter.zig
··· 295 295 296 296 try jw.objectField("attributes"); 297 297 try jw.beginArray(); 298 - for (s.attributes[0..s.attribute_count]) |attr| { 298 + for (s.attributes[0..s.attribute_count]) |*attr| { 299 299 try writeAttributeFromAttr(jw, attr); 300 300 } 301 301 try jw.endArray(); ··· 335 335 336 336 try jw.objectField("attributes"); 337 337 try jw.beginArray(); 338 - for (log.attributes[0..log.attribute_count]) |attr| { 338 + for (log.attributes[0..log.attribute_count]) |*attr| { 339 339 try writeAttributeFromAttr(jw, attr); 340 340 } 341 341 try jw.endArray(); ··· 445 445 // attributes - always include even if empty 446 446 try jw.objectField("attributes"); 447 447 try jw.beginArray(); 448 - for (dp.attributes) |attr| { 448 + for (dp.attributes) |*attr| { 449 449 try writeAttributeFromAttr(jw, attr); 450 450 } 451 451 try jw.endArray(); ··· 508 508 if (dp.attributes.len > 0) { 509 509 try jw.objectField("attributes"); 510 510 try jw.beginArray(); 511 - for (dp.attributes) |attr| { 511 + for (dp.attributes) |*attr| { 512 512 try writeAttributeFromAttr(jw, attr); 513 513 } 514 514 try jw.endArray(); ··· 559 559 if (dp.attributes.len > 0) { 560 560 try jw.objectField("attributes"); 561 561 try jw.beginArray(); 562 - for (dp.attributes) |attr| { 562 + for (dp.attributes) |*attr| { 563 563 try writeAttributeFromAttr(jw, attr); 564 564 } 565 565 try jw.endArray(); ··· 603 603 try jw.endObject(); 604 604 } 605 605 606 - fn writeAttributeFromAttr(jw: *json.Stringify, attr: Attribute) !void { 606 + fn writeAttributeFromAttr(jw: *json.Stringify, attr: *const Attribute) !void { 607 607 try jw.beginObject(); 608 608 try jw.objectField("key"); 609 609 try jw.write(attr.key); 610 610 try jw.objectField("value"); 611 611 try jw.beginObject(); 612 612 switch (attr.value) { 613 - .string => |s| { 613 + .string => { 614 614 try jw.objectField("stringValue"); 615 - try writeStringValue(jw, s); 615 + try writeStringValue(jw, attr.getString().?); 616 616 }, 617 617 .int => |i| { 618 618 try jw.objectField("intValue");