getty-zig / getty

A (de)serialization framework for Zig
https://getty.so
MIT License
183 stars 14 forks source link

Fix one-pointer string deserialization #107

Closed ibokuri closed 1 year ago

ibokuri commented 1 year ago

Description

Deserializing into any of the following types (which all return true from std.meta.traits.isZigString) from a string seems to fail:

How to Reproduce the Bug

const std = @import("std");
const json = @import("json");

const allocator = std.heap.page_allocator;

pub fn main() !void {
    const slice = try json.fromSlice(allocator, *const [2:0]u8, "\"HI\"");
    std.debug.print("{s}\n", .{slice});
}
$ zig build run
error: InvalidType

Additional Context

No response

ibokuri commented 1 year ago

Okay, this isn't a Getty issue, but a Getty JSON issue.

The pointer tests, which deserialize into the aforementioned types, all pass even though my repro code doesn't because the test deserializer they use implements both deserializeSeq and deserializeString with deserializeAny. That means that deserialization is driven by the deserializer's input data:

As a result, strings can be deserialized from either sequences or strings, which takes care of all string cases.

However, Getty JSON doesn't use deserializeAny for sequences or strings. So, Getty is finding the child DB for the pointer type, seeing that it's an array, and calling deserializeSeq, which in Getty JSON expects a JSON Array in the input, not a JSON String. Easiest fix I think is to just use deserializeAny for strings and sequences in Getty JSON. Or, handle sequence and string tokens in either functions.

ibokuri commented 1 year ago

Hmm, it might be better to check in the pointer DB if the type being deserialized into is a string. If so, then call deserializeString in the visitor as that's probably what the user is expecting. I think that's a more reasonable default to have. It'd make the lives of deserializers easier as well.