Snapchat / djinni

A tool for generating cross-language type declarations and interface bindings. Djinni's new home is in the Snapchat org.
Apache License 2.0
179 stars 50 forks source link

Djinni now throws on invalid UTF8 input when converting from C++ to Java #86

Closed remyjette closed 2 years ago

remyjette commented 2 years ago

Hi there,

Thanks for taking over development of Djinni! My app still usees dropbox/djinni, and I'm looking to switch over.

One thing I noticed is that jni/djinni_support.cpp now uses codecvt_utf8_utf16 which was deprecated in C++17 (and I believe is why dropbox/djinni wasn't using it)

This also had a pretty severe side effect: Previously Djinni had a function utf8_decode_check to validate that the given std::string was actually UTF8 before trying to convert it. Now this check is gone, and so trying to convert a string that contains invalid characters throws std::range_error instead of just returning 0xFFFD: https://godbolt.org/z/6x59nT3T8 (code copied from dropbox/djinni djinni_support.cpp and snapchat/djinni djinni_support.cpp)

li-feng-sc commented 2 years ago

Hi Remy,

This was an intentional change. The idea is if we need to pass non-string data, then we should explicitly use other types, such as binary or DataView, DataRef. If we do specify string, then it needs to be a valid string. C++ allows its strings to contain invalid codepoints but these do not map to other languages without loss of information.

We want this kind of errors to be detected earlier in the development cycle rather than failing silently and lead to unintended behavior.

remyjette commented 2 years ago

Thanks for the explanation!