facebook
RCTWebSocketModule - fix error crash when connect webSocket with header value null
Summary:
If you make a websocket connection with a Header key whose value is null, the application will crash on iOS.
2024-07-30 15:36:19.684613+0700 mobile[12777:918335] [native] NSInvalidArgumentException: -[NSNull length]: unrecognized selector sent to instance 0x11923b590
2024-07-30 15:36:19.732300+0700 mobile[12777:918335] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[NSNull length]: unrecognized selector sent to instance 0x11923b590'
*** First throw call stack:
(
0 CoreFoundation 0x0000000118fd578b __exceptionPreprocess + 242
1 libobjc.A.dylib 0x000000010d942b73 objc_exception_throw + 48
2 CoreFoundation 0x0000000118fe48c4 +[NSObject(NSObject) instanceMethodSignatureForSelector:] + 0
3 CoreFoundation 0x0000000118fd9c66 ___forwarding___ + 1443
4 CoreFoundation 0x0000000118fdbe08 _CF_forwarding_prep_0 + 120
5 CFNetwork 0x000000010e865606 _CFHTTPServerCreateSelfSignedIdentity + 44538
6 CFNetwork 0x000000010e74e6bb _CFStreamErrorFromCFError + 7517
7 CFNetwork 0x000000010e6e056a CFURLRequestAppendHTTPHeaderFieldValue + 229
8 mobile 0x00000001024e24b6 __57-[RCTWebSocketModule connect:protocols:options:socketID:]_block_invoke + 88
9 CoreFoundation 0x0000000118f1e8db __NSDICTIONARY_IS_CALLING_OUT_TO_A_BLOCK__ + 7
10 CoreFoundation 0x000000011904abc8 -[__NSDictionaryM enumerateKeysAndObjectsWithOptions:usingBlock:] + 225
11 mobile 0x00000001024e2244 -[RCTWebSocketModule connect:protocols:options:socketID:] + 628
12 CoreFoundation 0x0000000118fdc09c __invoking___ + 140
13 CoreFoundation 0x0000000118fd9406 -[NSInvocation invoke] + 305
14 CoreFoundation 0x0000000118fd96a5 -[NSInvocation invokeWithTarget:] + 70
15 mobile 0x0000000102cdefab -[RCTModuleMethod invokeWithBridge:module:arguments:] + 583
16 mobile 0x0000000102ce1792 _ZN8facebook5reactL11invokeInnerEP9RCTBridgeP13RCTModuleDatajRKN5folly7dynamicEiN12_GLOBAL__N_117SchedulingContextE + 562
17 mobile 0x0000000102ce13ae ___ZN8facebook5react15RCTNativeModule6invokeEjON5folly7dynamicEi_block_invoke + 110
18 libdispatch.dylib 0x000000011a1a454f _dispatch_call_block_and_release + 12
19 libdispatch.dylib 0x000000011a1a57ec _dispatch_client_callout + 8
20 libdispatch.dylib 0x000000011a1b66e2 _dispatch_main_queue_drain + 1462
21 libdispatch.dylib 0x000000011a1b611e _dispatch_main_queue_callback_4CF + 31
22 CoreFoundation 0x0000000118f336cc __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
23 CoreFoundation 0x0000000118f2dfbe __CFRunLoopRun + 2429
24 CoreFoundation 0x0000000118f2d264 CFRunLoopRunSpecific + 560
25 Foundation 0x0000000112707c8d -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 213
26 Foundation 0x0000000112707f04 -[NSRunLoop(NSRunLoop) runUntilDate:] + 72
27 mobile 0x0000000102f608a4 +[RNSplashScreen show] + 224
28 mobile 0x0000000102478710 -[AppDelegate application:didFinishLaunchingWithOptions:] + 352
29 UIKitCore 0x000000012b9b82c3 -[UIApplication _handleDelegateCallbacksWithOptions:isSuspended:restoreState:] + 271
30 UIKitCore 0x000000012b9ba186 -[UIApplication _callInitializationDelegatesWithActions:forCanvas:payload:fromOriginatingProcess:] + 4288
31 UIKitCore 0x000000012b9bfed2 -[UIApplication _runWithMainScene:transitionContext:completion:] + 1236
32 UIKitCore 0x000000012add923e -[_UISceneLifecycleMultiplexer completeApplicationLaunchWithFBSScene:transitionContext:] + 122
33 UIKitCore
Changelog:
[GENERAL] [FIXED] - fix error crash when connect webSocket with header value null
Pick one each for the category and type tags: IOS
Test Plan:
Check value not null
Hi @lethanh9398!
Thank you for your pull request and welcome to our community.
Action Required
In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.
Process
In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.
Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.
If you have received this in error or have any questions, please contact us at [email protected]. Thanks!
| Fails | |
|---|---|
| :no_entry_sign: |
:clipboard: Verify Changelog Format - See Changelog format |
Generated by :no_entry_sign: dangerJS against 23c5b99dbd0535b696635298e3ea881b71c48214
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!
It seems like the real issue here is that null headers are being passed in. Both iOS and Android will hit an exception in that case. Skipping a null header value could result in unexpected behavior.
@tdn120 I think null value not being sent is a correct thing. Because null and empty are different
I still think the correct thing here would be to avoid passing in nulls. Do you have more details on the scenario that's causing this crash? You're most likely better off fixing wherever the null(s) came from.
RN is not alone in its null handling here: https://github.com/square/okhttp/blob/okhttp_3.14.x/okhttp/src/main/java/okhttp3/Headers.java#L280
@tdn120 Avoiding passing null values is the right thing to do. However, to prevent application crashes, I think null checking is necessary. This problem occurs in IOS, it does not occur on AOS devices
@sammy-SC has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.