From d2000e9243cd0e71b7d5c28bc8d487ebd2f1a93e Mon Sep 17 00:00:00 2001 From: Christoph Purrer Date: Fri, 28 Aug 2026 22:53:44 -0700 Subject: [PATCH] Pass a top-level JS null TurboModule arg to ObjC as nil, not NSNull (#58190) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Changelog: [iOS][Fixed] - Pass a top-level JS `null` TurboModule argument to Objective-C as `nil` instead of `NSNull` when `enableModuleArgumentNSNullConversionIOS` is enabled When `enableModuleArgumentNSNullConversionIOS` is on, `convertJSIValueToObjCObject` maps a JS `null` to `(id)kCFNull`. That is the intended behaviour for nulls *nested* inside arrays and dictionaries, but a `null` in **argument position** must still reach Objective-C as `nil` — `NSNull` is truthy and does not respond to the selectors the receiver expects, so leaking it crashes the callee. The guard that enforced this lived three branches deep in `ObjCTurboModule::setInvocationArg`, reachable only when all of the following held: - `objCArgType == encode(id)`, and - `getArgumentTypeName(...)` returned non-nil, and - `RCTConvert` responded to a selector named after that type. `getArgumentTypeName` resolves the argument type by scanning for `__rct_export__`-prefixed selectors, which the compiler only emits for methods declared with `RCT_EXPORT_METHOD`. Any TurboModule method without that macro — or with an `id`-typed argument, since `[RCTConvert respondsToSelector:selector(id:)]` is `NO` — silently skipped the guard and received `NSNull`. This diff hoists the check to immediately after the conversion, so it applies to every argument regardless of the method's `__rct_export__` metadata, its ObjC type encoding, or whether an `RCTConvert` converter exists. Returning without calling `setArgument:` leaves the `NSInvocation` slot zeroed, i.e. `nil` — identical to what the old guard did. Behaviour is unchanged when the flag is off: the check short-circuits on the flag. Nested `NSNull` inside arrays and dictionaries is untouched, as asserted by the new `testInvokeTurboModuleKeepsNestedNullAsNSNullWhenFlagEnabled` case. (The flag-enabled branch of the pre-existing `testInvokeTurboModuleWithNull` case never executes while the flag defaults to `false`, so it did not cover this.) The pre-existing check inside the `RCTConvert` branch is left in place. It is now effectively unreachable — `objCArg == kCFNull` is the only way `convertedObjCArg` can be `kCFNull`, because every `RCTConvert` converter either returns a non-`kCFNull` input unchanged or builds a new object — but it costs nothing and keeps the diff narrow. Differential Revision: D117960577 --- .../core/iostests/RCTTurboModuleTests.mm | 94 +++++++++++++++++++ .../ios/ReactCommon/RCTTurboModule.mm | 7 ++ 2 files changed, 101 insertions(+) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm b/packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm index 0000965280c..a4d41956ea2 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm @@ -12,7 +12,9 @@ #import #import #import +#import +#import #import #import @@ -22,6 +24,12 @@ @interface RCTTestTurboModule : NSObject +// Deliberately not exported with RCT_EXPORT_METHOD: without `__rct_export__` metadata, +// `getArgumentTypeName` returns nil for the arguments. +- (void)testMethodWhichTakesStringWithoutExportMacro:(NSString *)string; + +- (void)logEvent:(NSString *)eventName data:(NSDictionary *)data analyticsModule:(nullable NSString *)analyticsModule; + @end @implementation RCTTestTurboModule @@ -30,8 +38,24 @@ @implementation RCTTestTurboModule RCT_EXPORT_METHOD(testMethodWhichTakesObject : (id)object) {} +- (void)testMethodWhichTakesStringWithoutExportMacro:(NSString *)string +{ +} + +- (void)logEvent:(NSString *)eventName data:(NSDictionary *)data analyticsModule:(nullable NSString *)analyticsModule +{ +} + @end +class ReactNativeFeatureFlagsNSNullConversionEnabled : public ReactNativeFeatureFlagsDefaults { + public: + bool enableModuleArgumentNSNullConversionIOS() override + { + return true; + } +}; + // Minimal concrete MutableBuffer that owns its bytes, used to observe lifetime. class TestMutableBuffer : public facebook::jsi::MutableBuffer { public: @@ -122,6 +146,8 @@ - (void)tearDown module_ = nullptr; instance_ = nil; + ReactNativeFeatureFlags::dangerouslyReset(); + [super tearDown]; } @@ -159,6 +185,74 @@ - (void)testInvokeTurboModuleWithNull OCMVerify(OCMTimes(1), [instance_ testMethodWhichTakesObject:nil]); } +- (void)testInvokeUnexportedTurboModuleMethodWithNullPassesNil +{ + ReactNativeFeatureFlags::dangerouslyForceOverride(std::make_unique()); + + auto hermesRuntime = facebook::hermes::makeHermesRuntime(); + facebook::jsi::Runtime *rt = hermesRuntime.get(); + + std::array args = {facebook::jsi::Value::null()}; + module_->invokeObjCMethod( + *rt, + VoidKind, + "testMethodWhichTakesStringWithoutExportMacro", + @selector(testMethodWhichTakesStringWithoutExportMacro:), + args.data(), + args.size()); + + OCMVerify(OCMTimes(1), [instance_ testMethodWhichTakesStringWithoutExportMacro:nil]); + OCMVerify(OCMNever(), [instance_ testMethodWhichTakesStringWithoutExportMacro:(id)kCFNull]); +} + +- (void)testInvokeUnexportedTurboModuleMethodWithNullTrailingArgumentPassesNil +{ + ReactNativeFeatureFlags::dangerouslyForceOverride(std::make_unique()); + + auto hermesRuntime = facebook::hermes::makeHermesRuntime(); + facebook::jsi::Runtime *rt = hermesRuntime.get(); + + __block id capturedAnalyticsModule = (id)kCFNull; + OCMStub([instance_ logEvent:OCMOCK_ANY + data:OCMOCK_ANY + analyticsModule:[OCMArg checkWithBlock:^BOOL(id value) { + capturedAnalyticsModule = value; + return YES; + }]]); + + std::array args = { + facebook::jsi::String::createFromAscii(*rt, "some_event"), + facebook::jsi::Object(*rt), + facebook::jsi::Value::null()}; + args[1].asObject(*rt).setProperty(*rt, "key", "value"); + + module_->invokeObjCMethod( + *rt, VoidKind, "logEvent", @selector(logEvent:data:analyticsModule:), args.data(), args.size()); + + OCMVerify(OCMTimes(1), [instance_ logEvent:@"some_event" data:@{@"key" : @"value"} analyticsModule:nil]); + XCTAssertNil(capturedAnalyticsModule); + + // `NSNull` is truthy, so this fallback would forward it and throw on -mutableCopy. + NSString *analyticsModule = (capturedAnalyticsModule != nullptr) ? capturedAnalyticsModule : @""; + XCTAssertNoThrow([analyticsModule mutableCopy]); +} + +// Scrubbing a null in argument position must not scrub nulls nested inside a collection argument. +- (void)testInvokeTurboModuleKeepsNestedNullAsNSNullWhenFlagEnabled +{ + ReactNativeFeatureFlags::dangerouslyForceOverride(std::make_unique()); + + auto hermesRuntime = facebook::hermes::makeHermesRuntime(); + facebook::jsi::Runtime *rt = hermesRuntime.get(); + + std::array args = {facebook::jsi::Object(*rt)}; + args[0].asObject(*rt).setProperty(*rt, "foo", facebook::jsi::Value::null()); + module_->invokeObjCMethod( + *rt, VoidKind, "testMethodWhichTakesObject", @selector(testMethodWhichTakesObject:), args.data(), args.size()); + + OCMVerify(OCMTimes(1), [instance_ testMethodWhichTakesObject:@{@"foo" : (id)kCFNull}]); +} + // A native-backed ArrayBuffer is aliased rather than copied, and the RCTArrayBuffer retains // the backing MutableBuffer, so the alias outlives the JS object. - (void)testNativeBackedArrayBufferIsAliasedAndKeepsBackingStoreAlive diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 7270187b414..6efbe9db1f4 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -727,6 +727,13 @@ TraceSection s( BOOL enableModuleArgumentNSNullConversionIOS = ReactNativeFeatureFlags::enableModuleArgumentNSNullConversionIOS(); id objCArg = convertJSIValueToObjCObject(runtime, arg, jsInvoker_, enableModuleArgumentNSNullConversionIOS, mustCopyBytes); + + // A JS `null` in argument position must reach ObjC as `nil`; only nulls nested inside arrays and + // dictionaries are preserved as `kCFNull`. Skipping `setArgument:` leaves the slot zeroed. + if (enableModuleArgumentNSNullConversionIOS && objCArg == (id)kCFNull) { + return; + } + if (objCArg != nullptr) { NSString *methodNameNSString = @(methodName);