authorgravatar for ian@ianjohnson.devIan Johnson <ian@ianjohnson.dev> 2023-08-17 07:52:46-04:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2023-08-17 07:52:46-04:00
logf3f554b9b89cc39cf00b4df68bd3455e8ef34984
tree819fed8d1bd38c4d6ce10dbdf1cc3def7aec5636
parent5395c2786a2ea9fd70e8c91aab099976216089aa
signaturebadge-question-mark Signed by PGP key 4AEE18F83AFDEB23

std.json: avoid stale pointers when parsing Value (#16864)

Closes #16861 Using `alloc_if_needed` when parsing a `Value` allows receiving a token which points to the buffer of the underlying `Reader`. This token will no longer be valid after the `Reader`'s buffer is refilled, which will happen with large values. Using `alloc_always` avoids this issue by ensuring the returned tokens always own their data independently of the underlying buffer.

2 files changed, 47 insertions(+), 7 deletions(-)

lib/std/json/dynamic.zig+7-7
...@@ -93,11 +93,11 @@ pub const Value = union(enum) {...@@ -93,11 +93,11 @@ pub const Value = union(enum) {
93 stack.items[stack.items.len - 1] == .array or93 stack.items[stack.items.len - 1] == .array or
94 (stack.items[stack.items.len - 2] == .object and stack.items[stack.items.len - 1] == .string));94 (stack.items[stack.items.len - 2] == .object and stack.items[stack.items.len - 1] == .string));
9595
96 switch (try source.nextAlloc(allocator, .alloc_if_needed)) {96 switch (try source.nextAlloc(allocator, .alloc_always)) {
97 inline .string, .allocated_string => |s| {97 .allocated_string => |s| {
98 return try handleCompleteValue(&stack, allocator, source, Value{ .string = s }) orelse continue;98 return try handleCompleteValue(&stack, allocator, source, Value{ .string = s }) orelse continue;
99 },99 },
100 inline .number, .allocated_number => |slice| {100 .allocated_number => |slice| {
101 return try handleCompleteValue(&stack, allocator, source, Value.parseFromNumberSlice(slice)) orelse continue;101 return try handleCompleteValue(&stack, allocator, source, Value.parseFromNumberSlice(slice)) orelse continue;
102 },102 },
103103
...@@ -106,9 +106,9 @@ pub const Value = union(enum) {...@@ -106,9 +106,9 @@ pub const Value = union(enum) {
106 .false => return try handleCompleteValue(&stack, allocator, source, Value{ .bool = false }) orelse continue,106 .false => return try handleCompleteValue(&stack, allocator, source, Value{ .bool = false }) orelse continue,
107107
108 .object_begin => {108 .object_begin => {
109 switch (try source.nextAlloc(allocator, .alloc_if_needed)) {109 switch (try source.nextAlloc(allocator, .alloc_always)) {
110 .object_end => return try handleCompleteValue(&stack, allocator, source, Value{ .object = ObjectMap.init(allocator) }) orelse continue,110 .object_end => return try handleCompleteValue(&stack, allocator, source, Value{ .object = ObjectMap.init(allocator) }) orelse continue,
111 inline .string, .allocated_string => |key| {111 .allocated_string => |key| {
112 try stack.appendSlice(&[_]Value{112 try stack.appendSlice(&[_]Value{
113 Value{ .object = ObjectMap.init(allocator) },113 Value{ .object = ObjectMap.init(allocator) },
114 Value{ .string = key },114 Value{ .string = key },
...@@ -152,7 +152,7 @@ fn handleCompleteValue(stack: *Array, allocator: Allocator, source: anytype, val...@@ -152,7 +152,7 @@ fn handleCompleteValue(stack: *Array, allocator: Allocator, source: anytype, val
152152
153 // This is an invalid state to leave the stack in,153 // This is an invalid state to leave the stack in,
154 // so we have to process the next token before we return.154 // so we have to process the next token before we return.
155 switch (try source.nextAlloc(allocator, .alloc_if_needed)) {155 switch (try source.nextAlloc(allocator, .alloc_always)) {
156 .object_end => {156 .object_end => {
157 // This object is complete.157 // This object is complete.
158 value = stack.pop();158 value = stack.pop();
...@@ -160,7 +160,7 @@ fn handleCompleteValue(stack: *Array, allocator: Allocator, source: anytype, val...@@ -160,7 +160,7 @@ fn handleCompleteValue(stack: *Array, allocator: Allocator, source: anytype, val
160 if (stack.items.len == 0) return value;160 if (stack.items.len == 0) return value;
161 continue;161 continue;
162 },162 },
163 inline .string, .allocated_string => |next_key| {163 .allocated_string => |next_key| {
164 // We've got another key.164 // We've got another key.
165 try stack.append(Value{ .string = next_key });165 try stack.append(Value{ .string = next_key });
166 // stack: [..., .object, .string]166 // stack: [..., .object, .string]
lib/std/json/dynamic_test.zig+40
...@@ -15,6 +15,7 @@ const parseFromValueLeaky = @import("static.zig").parseFromValueLeaky;...@@ -15,6 +15,7 @@ const parseFromValueLeaky = @import("static.zig").parseFromValueLeaky;
15const ParseOptions = @import("static.zig").ParseOptions;15const ParseOptions = @import("static.zig").ParseOptions;
1616
17const jsonReader = @import("scanner.zig").reader;17const jsonReader = @import("scanner.zig").reader;
18const JsonReader = @import("scanner.zig").Reader;
1819
19test "json.parser.dynamic" {20test "json.parser.dynamic" {
20 const s =21 const s =
...@@ -288,3 +289,42 @@ test "polymorphic parsing" {...@@ -288,3 +289,42 @@ test "polymorphic parsing" {
288 try testing.expect(tree.div.color == .blue);289 try testing.expect(tree.div.color == .blue);
289 try testing.expectEqualStrings("Cancel", tree.div.children[1].button.caption);290 try testing.expectEqualStrings("Cancel", tree.div.children[1].button.caption);
290}291}
292
293test "long object value" {
294 const value = "01234567890123456789";
295 const doc = "{\"key\":\"" ++ value ++ "\"}";
296 var fbs = std.io.fixedBufferStream(doc);
297 var reader = smallBufferJsonReader(testing.allocator, fbs.reader());
298 defer reader.deinit();
299 var parsed = try parseFromTokenSource(Value, testing.allocator, &reader, .{});
300 defer parsed.deinit();
301
302 try testing.expectEqualStrings(value, parsed.value.object.get("key").?.string);
303}
304
305test "many object keys" {
306 const doc =
307 \\{
308 \\ "k1": "v1",
309 \\ "k2": "v2",
310 \\ "k3": "v3",
311 \\ "k4": "v4",
312 \\ "k5": "v5"
313 \\}
314 ;
315 var fbs = std.io.fixedBufferStream(doc);
316 var reader = smallBufferJsonReader(testing.allocator, fbs.reader());
317 defer reader.deinit();
318 var parsed = try parseFromTokenSource(Value, testing.allocator, &reader, .{});
319 defer parsed.deinit();
320
321 try testing.expectEqualStrings("v1", parsed.value.object.get("k1").?.string);
322 try testing.expectEqualStrings("v2", parsed.value.object.get("k2").?.string);
323 try testing.expectEqualStrings("v3", parsed.value.object.get("k3").?.string);
324 try testing.expectEqualStrings("v4", parsed.value.object.get("k4").?.string);
325 try testing.expectEqualStrings("v5", parsed.value.object.get("k5").?.string);
326}
327
328fn smallBufferJsonReader(allocator: Allocator, io_reader: anytype) JsonReader(16, @TypeOf(io_reader)) {
329 return JsonReader(16, @TypeOf(io_reader)).init(allocator, io_reader);
330}