From 1904f58d31603fd31bcf105ada4d0de4a30f398e Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Thu, 8 Oct 2026 14:43:18 -0700 Subject: [PATCH] Forward createValueFromJsonUtf8 in RuntimeDecorator Summary: `RuntimeDecorator` doesn't forward `createValueFromJsonUtf8` to the plain runtime, so decorated runtimes use the `JSON.parse`-based default from jsi.cpp instead of the plain runtime's parser. For Hermes that means `makeThreadSafeHermesRuntime` and anything built on `WithRuntimeDecorator` never reach the native parser from a3f615cec. I think this was missed rather than chosen. `WithRuntimeDecorator` already overrides it (added in c2a0da2ae as `Around` plus `RD::createValueFromJsonUtf8`), which only makes sense if `RuntimeDecorator` forwards it. Today that `RD::` call lands in the default, so it takes one `Around` plus several nested ones for `global`, `getProperty`, `createStringFromUtf8` and `call`. It's also observable: on a thread-safe runtime, after `JSON.parse = () => 'hijacked'`, `Value::createFromJsonUtf8` returns `'hijacked'`, and after `delete globalThis.JSON` it throws. A plain `HermesRuntime` parses normally in both cases. `TracingRuntime` relies on the current behavior. JSON parsing only shows up in traces because the default splits into traced `global`/`getProperty`/`call` calls. Forwarding alone would hand back a value from the plain runtime with no def, and replay would hit the "has no def" assert in `useObjectID`. So `TracingRuntime` now overrides `createValueFromJsonUtf8` and calls `jsi::Runtime::createValueFromJsonUtf8` explicitly. That runs the same code as before, so traces are unchanged and there's no new record type. If you'd prefer a dedicated SynthTrace record I can do that instead. A side effect: decorators that watched JSON parsing through `global`/`getProperty`/`call` now need to override `createValueFromJsonUtf8`, like every other forwarded method. For example, `TimedRuntime` no longer counts JSON parsing under its `call` timer. The other methods decorators don't forward yet are the `createError*` family (https://github.com/facebook/hermes/issues/2203) and `String::length` (https://github.com/facebook/hermes/issues/2205). React Native's `decorator.h` has the same gap. X-link: https://github.com/facebook/hermes/pull/2206 Differential Revision: D124177263 Pulled By: lavenzg --- .../ReactCommon/jsi/jsi/decorator.h | 3 ++ .../ReactCommon/jsi/jsi/test/testlib.cpp | 48 +++++++++++++++++++ .../api-snapshots/ReactAndroidDebugCxx.api | 1 + .../ReactAndroidFrameworksCxx.api | 1 + .../api-snapshots/ReactAndroidNewarchCxx.api | 1 + .../api-snapshots/ReactAndroidReleaseCxx.api | 1 + .../api-snapshots/ReactAppleDebugCxx.api | 1 + .../api-snapshots/ReactAppleFrameworksCxx.api | 1 + .../api-snapshots/ReactAppleNewarchCxx.api | 1 + .../api-snapshots/ReactAppleReleaseCxx.api | 1 + .../api-snapshots/ReactCommonDebugCxx.api | 1 + .../ReactCommonFrameworksCxx.api | 1 + .../api-snapshots/ReactCommonNewarchCxx.api | 1 + .../api-snapshots/ReactCommonReleaseCxx.api | 1 + 14 files changed, 63 insertions(+) diff --git a/packages/react-native/ReactCommon/jsi/jsi/decorator.h b/packages/react-native/ReactCommon/jsi/jsi/decorator.h index 26478a3ad345..26a0daea6c32 100644 --- a/packages/react-native/ReactCommon/jsi/jsi/decorator.h +++ b/packages/react-native/ReactCommon/jsi/jsi/decorator.h @@ -148,6 +148,9 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation { Instrumentation& instrumentation() override { return *this; } + Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override { + return plain_.createValueFromJsonUtf8(json, length); + } protected: // plain is generally going to be a reference to an object managed diff --git a/packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp b/packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp index 38e2fc328b1b..3f7c279d21f7 100644 --- a/packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp +++ b/packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp @@ -1981,6 +1981,54 @@ TEST_P(JSITest, CreateFromUtf16Test) { EXPECT_EQ(cp, 55357); // 0xD83D in decimal } +TEST_P(JSITest, CreateValueFromJsonUtf8Test) { + uint8_t jsonText[] = "{\"a\": 1}"; + + // This Runtime Decorator is used to test the default createValueFromJsonUtf8 + // implementation for VMs that do not provide their own implementation + class DefaultRD : public RuntimeDecorator { + public: + explicit DefaultRD(Runtime& rt) : RuntimeDecorator(rt) {} + + Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override { + return Runtime::createValueFromJsonUtf8(json, length); + } + }; + + DefaultRD defaultRd(rt); + Object obj = + Value::createFromJsonUtf8(defaultRd, jsonText, sizeof(jsonText) - 1) + .getObject(defaultRd); + EXPECT_EQ(obj.getProperty(defaultRd, "a").getNumber(), 1); + + // A RuntimeDecorator must forward createValueFromJsonUtf8 to the runtime it + // decorates, like every other method. + class CountingRD : public RuntimeDecorator { + public: + explicit CountingRD(Runtime& rt) : RuntimeDecorator(rt) {} + + Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override { + ++count; + return plain().createValueFromJsonUtf8(json, length); + } + + int count = 0; + }; + + // Overrides nothing, so it should forward everything to CountingRD. + class OuterRD : public RuntimeDecorator { + public: + explicit OuterRD(Runtime& rt) : RuntimeDecorator(rt) {} + }; + + CountingRD counting(rt); + OuterRD outer(counting); + obj = outer.createValueFromJsonUtf8(jsonText, sizeof(jsonText) - 1) + .getObject(outer); + EXPECT_EQ(counting.count, 1); + EXPECT_EQ(obj.getProperty(outer, "a").getNumber(), 1); +} + TEST_P(JSITest, GetStringDataTest) { // This Runtime Decorator is used to test the default getStringData // implementation for VMs that do not provide their own implementation diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api index 777b8427b4ba..0a5549f5dcbb 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api @@ -6264,6 +6264,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override; public virtual facebook::jsi::Instrumentation& instrumentation() override; public virtual facebook::jsi::Object global() override; + public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override; public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr& buffer, const std::string& sourceURL) override; public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr& js) override; public virtual std::shared_ptr prepareJavaScript(const std::shared_ptr& buffer, std::string sourceURL) override; diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api index 07a514f87218..8377d984cb11 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api @@ -10880,6 +10880,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override; public virtual facebook::jsi::Instrumentation& instrumentation() override; public virtual facebook::jsi::Object global() override; + public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override; public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr& buffer, const std::string& sourceURL) override; public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr& js) override; public virtual std::shared_ptr prepareJavaScript(const std::shared_ptr& buffer, std::string sourceURL) override; diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api index e61666350cc0..d0198266a98d 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api @@ -6113,6 +6113,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override; public virtual facebook::jsi::Instrumentation& instrumentation() override; public virtual facebook::jsi::Object global() override; + public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override; public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr& buffer, const std::string& sourceURL) override; public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr& js) override; public virtual std::shared_ptr prepareJavaScript(const std::shared_ptr& buffer, std::string sourceURL) override; diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api index e61666350cc0..d0198266a98d 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api @@ -6113,6 +6113,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override; public virtual facebook::jsi::Instrumentation& instrumentation() override; public virtual facebook::jsi::Object global() override; + public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override; public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr& buffer, const std::string& sourceURL) override; public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr& js) override; public virtual std::shared_ptr prepareJavaScript(const std::shared_ptr& buffer, std::string sourceURL) override; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api index 4a7b602e202b..8cf3ee48f4f3 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api @@ -8108,6 +8108,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override; public virtual facebook::jsi::Instrumentation& instrumentation() override; public virtual facebook::jsi::Object global() override; + public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override; public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr& buffer, const std::string& sourceURL) override; public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr& js) override; public virtual std::shared_ptr prepareJavaScript(const std::shared_ptr& buffer, std::string sourceURL) override; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api index cb120cc274d0..2f204797272f 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api @@ -12739,6 +12739,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override; public virtual facebook::jsi::Instrumentation& instrumentation() override; public virtual facebook::jsi::Object global() override; + public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override; public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr& buffer, const std::string& sourceURL) override; public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr& js) override; public virtual std::shared_ptr prepareJavaScript(const std::shared_ptr& buffer, std::string sourceURL) override; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api index 8a11f3569d73..447d20d4a461 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api @@ -7959,6 +7959,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override; public virtual facebook::jsi::Instrumentation& instrumentation() override; public virtual facebook::jsi::Object global() override; + public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override; public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr& buffer, const std::string& sourceURL) override; public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr& js) override; public virtual std::shared_ptr prepareJavaScript(const std::shared_ptr& buffer, std::string sourceURL) override; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api index aa1ff6a9eb94..c4eb23ef973e 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api @@ -7967,6 +7967,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override; public virtual facebook::jsi::Instrumentation& instrumentation() override; public virtual facebook::jsi::Object global() override; + public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override; public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr& buffer, const std::string& sourceURL) override; public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr& js) override; public virtual std::shared_ptr prepareJavaScript(const std::shared_ptr& buffer, std::string sourceURL) override; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api index 217e69f46061..f84e54ddaea2 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api @@ -3511,6 +3511,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override; public virtual facebook::jsi::Instrumentation& instrumentation() override; public virtual facebook::jsi::Object global() override; + public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override; public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr& buffer, const std::string& sourceURL) override; public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr& js) override; public virtual std::shared_ptr prepareJavaScript(const std::shared_ptr& buffer, std::string sourceURL) override; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api index cd69da282893..e5ba09473c28 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api @@ -8034,6 +8034,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override; public virtual facebook::jsi::Instrumentation& instrumentation() override; public virtual facebook::jsi::Object global() override; + public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override; public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr& buffer, const std::string& sourceURL) override; public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr& js) override; public virtual std::shared_ptr prepareJavaScript(const std::shared_ptr& buffer, std::string sourceURL) override; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api index 05433e06a97a..91c39a366f5a 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api @@ -3504,6 +3504,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override; public virtual facebook::jsi::Instrumentation& instrumentation() override; public virtual facebook::jsi::Object global() override; + public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override; public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr& buffer, const std::string& sourceURL) override; public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr& js) override; public virtual std::shared_ptr prepareJavaScript(const std::shared_ptr& buffer, std::string sourceURL) override; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api index 05433e06a97a..91c39a366f5a 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api @@ -3504,6 +3504,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override; public virtual facebook::jsi::Instrumentation& instrumentation() override; public virtual facebook::jsi::Object global() override; + public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override; public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr& buffer, const std::string& sourceURL) override; public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr& js) override; public virtual std::shared_ptr prepareJavaScript(const std::shared_ptr& buffer, std::string sourceURL) override;