Key struct is fragile, possibly unsound

The Key struct is currently initialized in two phases. First it is created, like this:

https://github.com/maciejhirsz/json-rust/blob/0775592d339002ab148185264970c2a6e30b5d37/src/object.rs#L71-L78

And only later it is attached to its actual contents.

Before the key is attached, calling as_bytes results in undefined behavior:

https://github.com/maciejhirsz/json-rust/blob/0775592d339002ab148185264970c2a6e30b5d37/src/object.rs#L81-L85

because a null pointer is passed to slice::from_raw_parts, violating its safety invariant. Replacing a null pointer with NonNull::dangling() will not solve the problem because then the slice will non-zero length would to unclaimed memory, which is also UB.

If the two-phase initialization is truly necessary, a less fragile way to approach it is to explicitly encode the initialization state in the type system: have UnattachedKey struct without any accessor methods for the partly initialized state, and provide a method to turn it into a proper Key by calling attach().

Key also seems to contain a hand-rolled implementation of the small string optimization that, in addition to the usual dangers of custom unsafe code, leaves performance on the table. The current implementation always zero-initializes the underlying buffer. Using smallstr crate or something along those lines will (even though I'm not really a fan of the underlying smallvec crate).

Using slotmap to store the strings in a single allocation instead of using the small string optimization would probably work even better, since it would avoid inflating the struct, and also remove the need for pointer fixups on cloning and reallocation altogether. It would also remove the need for two-phase initialization, and Key would no longer have to be self-referential.

Miri doesn't like it either

error: Undefined Behavior: trying to retag from <3928> for SharedReadOnly permission at alloc1799[0x18], but that tag does not exist in the borrow stack for this location
   --> /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/raw.rs:97:9
    |
97  |         &*ptr::slice_from_raw_parts(data, len)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |         |
    |         trying to retag from <3928> for SharedReadOnly permission at alloc1799[0x18], but that tag does not exist in the borrow stack for this location
    |         this error occurs as part of retag at alloc1799[0x18..0x1c]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information

maciejhirsz / json-rust

Key struct is fragile, possibly unsound #196