Field is_ok should be private for DiplomatResult

[imbrem]

Since is_ok is public, it is trivial to get UB in Safe Rust using Diplomat (without any FFI) by writing

let rust_result: Result<u64, Box<u64>> = Ok(32);
let mut result: DiplomatResult<u64, Box<u64>> = rust_result.into();
result.is_ok = false;
std::mem::drop(result);

This will attempt to call Box::drop on the value 32, leading to a segmentation fault.

One can retain about equivalent API power (since is_ok is the only public field) by having a safe getter returning the value and an unsafe getter returning a mutable reference to the value.

[Manishearth]

Yeah, makes sense. The diplomat_runtime crate is basically supposed to be used in certain ways so we haven't tried to guard it against things like this, but this seems reasonable enough to do.

[imbrem]

Want me to submit a PR? I'm also willing to have a read through for some more UB, as I want to use this to do the FFI for a project I'll be doing as part of my PhD thesis which needs a good C++ interface. Not doing it right away as it probably needs a version bump due to semver...

[Manishearth]

Yeah go for it. I'm fine with breaking semver over soundness; people should not have been using the is_ok field anyway.

As far as fixing everything, an audit would be great, but we probably won't fix everything.

Since it lives on the FFI boundary there probably a lot of soundness issues inherent to the design that we won't fix (since they will be hard to trigger unintentionally)

In particular, currently we let you violate single-mutable-reference rules in a way that's relatively benign as long as you design your diplomat APIs carefully. We do have a plan for fixing that (#225), if you follow the rules set out there it should be fine.

Alspo happy to answer questions as you're using it. We haven't documented this that well (and have some major improvements in the pipeline) so you might have more questions.