fix(android): broaden malformed-JSON catch in NativeProxy handleEvent#9989
Open
raid5 wants to merge 1 commit into
Open
fix(android): broaden malformed-JSON catch in NativeProxy handleEvent#9989raid5 wants to merge 1 commit into
raid5 wants to merge 1 commit into
Conversation
The try/catch around jsi::Value::createFromJsonUtf8() in NativeProxy::handleEvent only caught std::exception&. On Hermes, the JSI value thrown for a malformed JSON payload is not guaranteed to derive from std::exception, so a bad event payload escaped the handler and crashed the app instead of being ignored. Broaden the catch to catch(...) so malformed-JSON event payloads are swallowed exactly as the existing "Ignore events with malformed JSON payload." comment intends. This matches the toString() try/catch a few lines above, which already uses catch(...).
Member
|
Hello @raid5, thanks for submitting the PR! I'm happy with merging it in its current shape. Do you have a repro that we can use to make sure that there are no regressions in the future?
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NativeProxy::handleEvent(packages/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.cpp) wraps the JSON deserialization of an incoming event payload in atry/catchwhose intent — per its own comment,// Ignore events with malformed JSON payload.— is to silently drop events whose payload cannot be parsed:The problem is that the catch only matches
std::exception &. On Hermes, the JSI value thrown when JSON parsing fails is not guaranteed to derive fromstd::exception, so a malformed payload slips past this handler and propagates up, crashing the app instead of being ignored.The fix broadens the catch to
catch (...), so any failure fromcreateFromJsonUtf8is swallowed as the comment already intends. This is consistent with the existingevent->toString()try/catcha few lines above in the same function, which already usescatch (...)for the analogous "payload cannot be represented" case.This is a one-line, behavior-preserving broadening of an existing error path — no new behavior is introduced; it just makes the intended "ignore malformed payload" path actually fire on Hermes.
This fix was prepared with AI assistance and reviewed by a human before submission.
Test plan
This change has been running in a shipping React Native app on a Hermes release build with the New Architecture enabled, where it eliminated a crash caused by malformed event payloads reaching
handleEvent.Root-cause repro on a Hermes release build:
useAnimatedScrollHandleror a gesture-driven handler) sohandleEventruns.createFromJsonUtf8.std::exceptionand escapes thecatch (std::exception &). After this change the event is silently ignored, exactly as the existing comment intends.Well-formed payloads are unaffected — only the exception type matched by the catch was widened. This is a native C++ change; the JS test suite does not cover it.