Skip to content

Commit e5a43bd

Browse files
Pass a top-level JS null TurboModule arg to ObjC as nil, not NSNull
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 existing `testInvokeTurboModuleWithNull` case. 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
1 parent c6b137c commit e5a43bd

2 files changed

Lines changed: 53 additions & 0 deletions

File tree

packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#import <hermes/hermes.h>
1313
#import <jsi/decorator.h>
1414
#import <react/featureflags/ReactNativeFeatureFlags.h>
15+
#import <react/featureflags/ReactNativeFeatureFlagsDefaults.h>
1516

1617
#import <memory>
1718
#import <vector>
@@ -22,6 +23,11 @@
2223

2324
@interface RCTTestTurboModule : NSObject <RCTBridgeModule>
2425

26+
// Deliberately not exported with RCT_EXPORT_METHOD: TurboModules dispatch through the codegen'd
27+
// `...SpecJSI`, so many of them carry no `__rct_export__` metadata and `getArgumentTypeName` returns
28+
// nil for their arguments.
29+
- (void)testMethodWhichTakesStringWithoutExportMacro:(NSString *)string;
30+
2531
@end
2632

2733
@implementation RCTTestTurboModule
@@ -30,8 +36,20 @@ @implementation RCTTestTurboModule
3036

3137
RCT_EXPORT_METHOD(testMethodWhichTakesObject : (id)object) {}
3238

39+
- (void)testMethodWhichTakesStringWithoutExportMacro:(NSString *)string
40+
{
41+
}
42+
3343
@end
3444

45+
class ReactNativeFeatureFlagsNSNullConversionEnabled : public ReactNativeFeatureFlagsDefaults {
46+
public:
47+
bool enableModuleArgumentNSNullConversionIOS() override
48+
{
49+
return true;
50+
}
51+
};
52+
3553
// Minimal concrete MutableBuffer that owns its bytes, used to observe lifetime.
3654
class TestMutableBuffer : public facebook::jsi::MutableBuffer {
3755
public:
@@ -122,6 +140,8 @@ - (void)tearDown
122140
module_ = nullptr;
123141
instance_ = nil;
124142

143+
ReactNativeFeatureFlags::dangerouslyReset();
144+
125145
[super tearDown];
126146
}
127147

@@ -159,6 +179,29 @@ - (void)testInvokeTurboModuleWithNull
159179
OCMVerify(OCMTimes(1), [instance_ testMethodWhichTakesObject:nil]);
160180
}
161181

182+
// A JS `null` argument must arrive as `nil` even when the method carries no `__rct_export__`
183+
// metadata, so that nullability checks in the receiver behave. `NSNull` is truthy and does not
184+
// respond to most NSString/NSDictionary selectors, so leaking it crashes the callee.
185+
- (void)testInvokeUnexportedTurboModuleMethodWithNullPassesNil
186+
{
187+
ReactNativeFeatureFlags::dangerouslyForceOverride(std::make_unique<ReactNativeFeatureFlagsNSNullConversionEnabled>());
188+
189+
auto hermesRuntime = facebook::hermes::makeHermesRuntime();
190+
facebook::jsi::Runtime *rt = hermesRuntime.get();
191+
192+
facebook::jsi::Value args[1] = {facebook::jsi::Value::null()};
193+
module_->invokeObjCMethod(
194+
*rt,
195+
VoidKind,
196+
"testMethodWhichTakesStringWithoutExportMacro",
197+
@selector(testMethodWhichTakesStringWithoutExportMacro:),
198+
args,
199+
1);
200+
201+
OCMVerify(OCMTimes(1), [instance_ testMethodWhichTakesStringWithoutExportMacro:nil]);
202+
OCMVerify(OCMNever(), [instance_ testMethodWhichTakesStringWithoutExportMacro:(id)kCFNull]);
203+
}
204+
162205
// A native-backed ArrayBuffer is aliased rather than copied, and the RCTArrayBuffer retains
163206
// the backing MutableBuffer, so the alias outlives the JS object.
164207
- (void)testNativeBackedArrayBufferIsAliasedAndKeepsBackingStoreAlive

packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,16 @@ TraceSection s(
727727
BOOL enableModuleArgumentNSNullConversionIOS = ReactNativeFeatureFlags::enableModuleArgumentNSNullConversionIOS();
728728
id objCArg =
729729
convertJSIValueToObjCObject(runtime, arg, jsInvoker_, enableModuleArgumentNSNullConversionIOS, mustCopyBytes);
730+
731+
/**
732+
* A JS `null` in argument position must reach ObjC as `nil`; only nulls *nested* inside arrays and
733+
* dictionaries are preserved as `kCFNull`. Returning without calling `setArgument:` leaves the
734+
* NSInvocation slot zeroed, i.e. `nil`.
735+
*/
736+
if (enableModuleArgumentNSNullConversionIOS && objCArg == (id)kCFNull) {
737+
return;
738+
}
739+
730740
if (objCArg != nullptr) {
731741
NSString *methodNameNSString = @(methodName);
732742

0 commit comments

Comments
 (0)