themis icon indicating copy to clipboard operation
themis copied to clipboard

RNThemis hot fix: parameter checks to generate exceptions in case of empty values

Open radetsky opened this issue 2 years ago • 2 comments

React Native Themis hotfix fixes the issue https://github.com/cossacklabs/themis/issues/925

I have added simple checks for empty values where it is possible to generate JS exceptions to protect React Native developers from native exceptions.

radetsky avatar Aug 03 '22 09:08 radetsky

Maybe this should be a catch-all clause in native code instead?..

I'm not sure what exactly causes the exception in the original issue, but adding random checks for empty values looks like a bandaid to me. Native code all the way down the stack should be handling "empty plaintext" just fine, returning an error. How come, say, failure to decrypt does not cause an unhandled native exception but an empty input does?

Or is this yet another React Native bullshit? Like, it not being able to marshal empty JS string to the native code, and failing somewhere in between,

ilammy avatar Aug 03 '22 16:08 ilammy

Or is this yet another React Native bullshit? Like, it not being able to marshal empty JS string to the native code, and failing somewhere in between,

that's our hypothesis right now. Also, we have similar checks in some special wrappers, i.e. iOS

vixentael avatar Aug 03 '22 19:08 vixentael

Actually, I'd like to make an investigation in the native code to handle it when I have a couple of hours.

radetsky avatar Aug 12 '22 08:08 radetsky

Thanks for doing this. It would still be great to have a catch all wrapper that handles uncaught exceptions and propagates them back to the JavaScript layer. It doesn't matter what the exception is, in fact the empty string listed in the original issue that I created is just an example. We shouldn't ever let native exceptions go uncaught since the calling JavaScript code doesn't know about them and will (incorrectly) treat the call as successful. Could we consider adding a big try {} catch {} around the encrypt/decrypt code and forward the error back to the JavaScript code, where it is thrown as a JS error (which is catchable by the calling code)? Thanks again!

tom-at-pixel avatar Aug 16 '22 02:08 tom-at-pixel