From f610edd85f2de1d25c59f66c4a725021ff83915a Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Wed, 7 Oct 2026 14:45:01 -0700 Subject: [PATCH] Forward String::length in RuntimeDecorator and WithRuntimeDecorator (#58936) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: `IRuntime::length(const String&)` came in with "Add `String::length` API" (590396bd4 on static_h, D96030262), and that commit didn't touch `decorator.h`. Neither `RuntimeDecorator` nor `WithRuntimeDecorator` overrides it, so on a decorated runtime `length()` falls back to the default `utf16(str).size()` and never reaches the wrapped runtime. It looks like an oversight. The same stack added `push`, `tryGetMutableBuffer`, `detached` and the TypedArray methods, and each of those commits added decorator overrides, including one for this method's overload `length(const TypedArray&)`. The `createError*` commit (https://github.com/facebook/hermes/issues/2203) and this one didn't. For Hermes-backed decorators the value is already right, because Hermes' `utf16()` is exact, but every call copies the whole string instead of reading the length. Runtimes with a native `length()` and the default `utf16()` get wrong answers. React Native's `JSCRuntime` is one: built against the system JavaScriptCore and wrapped in a `RuntimeDecorator`, `'a\uD800b'` has length 1 instead of 3 and `'x\uDC00yz'` has 1 instead of 4. With this change both match the undecorated runtime. That setup lives outside this repo, so it isn't a test here. `WithRuntimeDecorator` needs the override too. Otherwise it would inherit the plain forward and `length()` would skip `Around`, which it currently goes through via `utf16()`. `TracingRuntime` no longer records an incidental `Utf16Record` when `length()` is called. That record only came from the `utf16()` fallback. I didn't add a record for `length()`: strings are immutable, and `TracingRuntime` already leaves pure queries like `size(Array)` untraced. X-link: https://github.com/facebook/hermes/pull/2205 Test Plan: Added `JSITest.DecoratorForwardsStringLength`. An inner decorator returns a sentinel from `length()` and counts `length()` and `utf16()` calls. The test checks that a `RuntimeDecorator` and a `WithRuntimeDecorator` over it both reach that `length()` without calling `utf16()`, and that `Around` runs once. It fails without the change (it gets 5 from the `utf16()` fallback) and passes with it. The full `APITests` binary passes (357 tests) in a Debug + ASan build, including the SynthTrace and TraceInterpreter tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: avp Differential Revision: D123943189 Pulled By: lavenzg --- .../ReactCommon/jsi/jsi/decorator.h | 9 +++ .../ReactCommon/jsi/jsi/test/testlib.cpp | 55 +++++++++++++++++++ .../api-snapshots/ReactAndroidDebugCxx.api | 2 + .../api-snapshots/ReactAndroidNewarchCxx.api | 2 + .../api-snapshots/ReactAndroidReleaseCxx.api | 2 + .../api-snapshots/ReactAppleDebugCxx.api | 2 + .../api-snapshots/ReactAppleNewarchCxx.api | 2 + .../api-snapshots/ReactAppleReleaseCxx.api | 2 + .../api-snapshots/ReactCommonDebugCxx.api | 2 + .../api-snapshots/ReactCommonNewarchCxx.api | 2 + .../api-snapshots/ReactCommonReleaseCxx.api | 2 + 11 files changed, 82 insertions(+) diff --git a/packages/react-native/ReactCommon/jsi/jsi/decorator.h b/packages/react-native/ReactCommon/jsi/jsi/decorator.h index f101c1832e61..b78aa14214dd 100644 --- a/packages/react-native/ReactCommon/jsi/jsi/decorator.h +++ b/packages/react-native/ReactCommon/jsi/jsi/decorator.h @@ -243,6 +243,10 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation { return plain_.utf16(sym); } + size_t length(const String& str) override { + return plain_.length(str); + } + void getStringData( const jsi::String& str, void* ctx, @@ -822,6 +826,11 @@ class WithRuntimeDecorator : public RuntimeDecorator { return RD::utf16(sym); } + size_t length(const String& str) override { + Around around{with_}; + return RD::length(str); + } + void getStringData( const jsi::String& str, void* ctx, diff --git a/packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp b/packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp index 9c056ef03790..38e2fc328b1b 100644 --- a/packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp +++ b/packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp @@ -138,6 +138,61 @@ TEST_P(JSITest, StringLengthTest) { EXPECT_EQ(invalid.length(rt), 2); } +TEST_P(JSITest, DecoratorForwardsStringLength) { + // Decorators should forward length(const String&) to the decorated runtime + // instead of falling back to the default Runtime::length, which copies the + // string via utf16() and bypasses the runtime's own implementation. + class Inner : public RuntimeDecorator { + public: + explicit Inner(Runtime& rt) : RuntimeDecorator(rt) {} + + size_t length(const String&) override { + ++lengthCalls; + return 42; + } + + std::u16string utf16(const String& str) override { + ++utf16Calls; + return RuntimeDecorator::utf16(str); + } + + int lengthCalls = 0; + int utf16Calls = 0; + }; + + class Outer : public RuntimeDecorator { + public: + explicit Outer(Runtime& rt) : RuntimeDecorator(rt) {} + }; + + struct Count { + void before() { + ++beforeCalls; + } + void after() { + ++afterCalls; + } + int beforeCalls = 0; + int afterCalls = 0; + }; + + Inner inner(rt); + String str = String::createFromAscii(inner, "hello"); + + Outer outer(inner); + EXPECT_EQ(str.length(outer), 42); + EXPECT_EQ(inner.lengthCalls, 1); + EXPECT_EQ(inner.utf16Calls, 0); + + Count count; + WithRuntimeDecorator with(inner, count); + EXPECT_EQ(str.length(with), 42); + EXPECT_EQ(inner.lengthCalls, 2); + EXPECT_EQ(inner.utf16Calls, 0); + EXPECT_EQ(count.beforeCalls, 1); + EXPECT_EQ(count.afterCalls, 1); +} + TEST_P(JSITest, ObjectTest) { eval("x = {1:2, '3':4, 5:'six', 'seven':['eight', 'nine']}"); Object x = rt.global().getPropertyAsObject(rt, "x"); diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api index 5ee27dc3ab0a..0d655df28e51 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api @@ -12775,6 +12775,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; @@ -12893,6 +12894,7 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api index 8126c975bda3..5f1932553e10 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api @@ -12391,6 +12391,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; @@ -12509,6 +12510,7 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api index 2130ccfe23fa..d209696f6f7e 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api @@ -12622,6 +12622,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; @@ -12740,6 +12741,7 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api index 0168eac13d1b..59bb2ff0982a 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api @@ -14544,6 +14544,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; @@ -14662,6 +14663,7 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api index bdc9f5eaffe0..5b75d59f8cd2 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api @@ -14226,6 +14226,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; @@ -14344,6 +14345,7 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api index 70e0dc1d72ac..f8c5c2af6d36 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api @@ -14401,6 +14401,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; @@ -14519,6 +14520,7 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api index a5f5489a6076..a15e1c68fe48 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api @@ -9730,6 +9730,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; @@ -9848,6 +9849,7 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api index 3637899b15e7..0867f18fcd01 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api @@ -9554,6 +9554,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; @@ -9672,6 +9673,7 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api index caa417e05392..025efa0c8459 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api @@ -9721,6 +9721,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime { protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override; @@ -9839,6 +9840,7 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat protected virtual facebook::jsi::WeakObject createWeakObject(const facebook::jsi::Object& o) override; protected virtual size_t byteLength(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t byteOffset(const facebook::jsi::TypedArray& typedArray) override; + protected virtual size_t length(const facebook::jsi::String& str) override; protected virtual size_t length(const facebook::jsi::TypedArray& typedArray) override; protected virtual size_t push(const facebook::jsi::Array& a, const facebook::jsi::Value* elements, size_t count) override; protected virtual size_t size(const facebook::jsi::Array& a) override;