Skip to content

Commit 2c468be

Browse files
committed
fix(ios): harden VoIP DDP WebSocket client on receive failure and TLS
PR 3a from VoIP split plan: call disconnect on WebSocket receive errors, reuse existing Challenge TLS handling via URLSession delegate, remove dead default-branch capture, and add a DEBUG-only XCTest source plus bridging for Challenge. Disconnect is synchronous when already on the client queue so cleanup finishes before further queued work. Made-with: Cursor
1 parent 38ca1df commit 2c468be

4 files changed

Lines changed: 139 additions & 34 deletions

File tree

ios/Libraries/DDPClient.swift

Lines changed: 100 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ final class DDPClient {
1515

1616
private var webSocketTask: URLSessionWebSocketTask?
1717
private var urlSession: URLSession?
18+
/// Retains the delegate passed to `URLSession` so TLS challenges reuse `Challenge` from `SSLPinning.mm`.
19+
private var urlSessionChallengeDelegate: DDPClientURLSessionChallengeDelegate?
1820
private var sendCounter = 0
1921
private var isConnected = false
2022

@@ -43,9 +45,11 @@ final class DDPClient {
4345
print("[\(Self.TAG)] Connecting to \(wsUrl)")
4446
#endif
4547

46-
let session = URLSession(configuration: .default)
48+
let challengeDelegate = DDPClientURLSessionChallengeDelegate()
49+
let session = URLSession(configuration: .default, delegate: challengeDelegate, delegateQueue: nil)
4750
let task = session.webSocketTask(with: url)
4851

52+
self.urlSessionChallengeDelegate = challengeDelegate
4953
self.urlSession = session
5054
self.webSocketTask = task
5155
self.isConnected = false
@@ -145,21 +149,30 @@ final class DDPClient {
145149
// MARK: - Disconnect
146150

147151
func disconnect() {
148-
stateQueue.async {
149-
#if DEBUG
150-
print("[\(Self.TAG)] Disconnecting")
151-
#endif
152-
self.isConnected = false
153-
self.pendingCallbacks.removeAll()
154-
self.clearQueuedMethodCalls()
155-
self.connectedCallback = nil
156-
self.onCollectionMessage = nil
157-
self.webSocketTask?.cancel(with: .normalClosure, reason: nil)
158-
self.webSocketTask = nil
159-
self.urlSession?.invalidateAndCancel()
160-
self.urlSession = nil
152+
if DispatchQueue.getSpecific(key: stateQueueKey) != nil {
153+
disconnectOnStateQueue()
154+
} else {
155+
stateQueue.async { [weak self] in
156+
self?.disconnectOnStateQueue()
157+
}
161158
}
162159
}
160+
161+
private func disconnectOnStateQueue() {
162+
#if DEBUG
163+
print("[\(Self.TAG)] Disconnecting")
164+
#endif
165+
isConnected = false
166+
pendingCallbacks.removeAll()
167+
clearQueuedMethodCalls()
168+
connectedCallback = nil
169+
onCollectionMessage = nil
170+
webSocketTask?.cancel(with: .normalClosure, reason: nil)
171+
webSocketTask = nil
172+
urlSession?.invalidateAndCancel()
173+
urlSession = nil
174+
urlSessionChallengeDelegate = nil
175+
}
163176

164177
// MARK: - Private
165178

@@ -292,27 +305,33 @@ final class DDPClient {
292305

293306
private func listenForMessages(task: URLSessionWebSocketTask) {
294307
task.receive { [weak self] result in
295-
self?.stateQueue.async {
296-
guard let self = self, let currentTask = self.webSocketTask, task === currentTask else { return }
297-
298-
switch result {
299-
case .success(let message):
300-
switch message {
301-
case .string(let text):
302-
self.handleMessage(text)
303-
default:
304-
break
305-
}
306-
self.listenForMessages(task: task)
307-
308-
case .failure(let error):
309-
#if DEBUG
310-
print("[\(Self.TAG)] Receive error: \(error.localizedDescription)")
311-
#endif
312-
}
308+
self?.stateQueue.async { [weak self] in
309+
guard let self else { return }
310+
self.handleWebSocketReceiveResult(result, for: task)
313311
}
314312
}
315313
}
314+
315+
private func handleWebSocketReceiveResult(_ result: Result<URLSessionWebSocketTask.Message, Error>, for task: URLSessionWebSocketTask) {
316+
guard task === webSocketTask else { return }
317+
318+
switch result {
319+
case .success(let message):
320+
switch message {
321+
case .string(let text):
322+
handleMessage(text)
323+
default:
324+
break
325+
}
326+
listenForMessages(task: task)
327+
328+
case .failure(let error):
329+
#if DEBUG
330+
print("[\(Self.TAG)] Receive error: \(error.localizedDescription)")
331+
#endif
332+
disconnect()
333+
}
334+
}
316335

317336
private func handleMessage(_ text: String) {
318337
guard let data = text.data(using: .utf8),
@@ -358,9 +377,8 @@ final class DDPClient {
358377
}
359378

360379
default:
361-
if let collection = json["collection"] as? String {
380+
if (json["collection"] as? String) != nil {
362381
onCollectionMessage?(json)
363-
_ = collection
364382
}
365383
}
366384
}
@@ -389,3 +407,51 @@ final class DDPClient {
389407
return "\(scheme)://\(cleaned)/websocket"
390408
}
391409
}
410+
411+
/// Forwards URL authentication challenges to the existing `Challenge` implementation in `SSLPinning.mm`
412+
/// (same path as `RCTHTTPRequestHandler` swizzling) so WebSocket TLS uses the same client-certificate flow.
413+
private final class DDPClientURLSessionChallengeDelegate: NSObject, URLSessionDelegate, URLSessionTaskDelegate {
414+
func urlSession(
415+
_ session: URLSession,
416+
didReceive challenge: URLAuthenticationChallenge,
417+
completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Void
418+
) {
419+
Challenge.runChallenge(session, didReceiveChallenge: challenge, completionHandler: completionHandler)
420+
}
421+
422+
func urlSession(
423+
_ session: URLSession,
424+
task: URLSessionTask,
425+
didReceive challenge: URLAuthenticationChallenge,
426+
completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Void
427+
) {
428+
Challenge.runChallenge(session, didReceiveChallenge: challenge, completionHandler: completionHandler)
429+
}
430+
}
431+
432+
#if DEBUG
433+
extension DDPClient {
434+
/// Installs a WebSocket task so receive-failure handling can be exercised without a live server.
435+
func testing_installWebSocketSync(urlSession: URLSession, task: URLSessionWebSocketTask) {
436+
stateQueue.sync {
437+
self.urlSession?.invalidateAndCancel()
438+
self.urlSession = urlSession
439+
self.webSocketTask = task
440+
self.isConnected = true
441+
}
442+
}
443+
444+
func testing_applyReceiveResult(_ result: Result<URLSessionWebSocketTask.Message, Error>, for task: URLSessionWebSocketTask) {
445+
stateQueue.async { [weak self] in
446+
guard let self else { return }
447+
self.handleWebSocketReceiveResult(result, for: task)
448+
}
449+
}
450+
451+
func testing_readConnectionState(_ completion: @escaping (_ isConnected: Bool, _ webSocketTaskIsNil: Bool, _ urlSessionIsNil: Bool) -> Void) {
452+
stateQueue.async {
453+
completion(self.isConnected, self.webSocketTask == nil, self.urlSession == nil)
454+
}
455+
}
456+
}
457+
#endif

ios/Libraries/SSLPinning.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@
1010

1111
NS_ASSUME_NONNULL_BEGIN
1212

13+
@class NSURLSession;
14+
@class NSURLAuthenticationChallenge;
15+
16+
/// Shared TLS / client-certificate handling used by React Native networking and native URLSessions.
17+
@interface Challenge : NSObject
18+
+ (void)runChallenge:(NSURLSession *)session
19+
didReceiveChallenge:(NSURLAuthenticationChallenge *)challenge
20+
completionHandler:(void (^)(NSURLSessionAuthChallengeDisposition disposition, NSURLCredential * _Nullable credential))completionHandler;
21+
@end
22+
1323
@interface RCTHTTPRequestHandler (Challenge)
1424

1525
@end

ios/RocketChatRN-Bridging-Header.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@
1212
#import <MobileCrypto/AESCrypto.h>
1313
#import <MobileCrypto/RandomUtils.h>
1414
#import <MobileCrypto/CryptoUtils.h>
15+
#import "SSLPinning.h"
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import XCTest
2+
@testable import RocketChatRN
3+
4+
/// Covers WebSocket `receive` failures for `DDPClient` (not run in CI until this file is added to an iOS unit-test target in Xcode).
5+
///
6+
/// Manual SSL pinning: use Charles with a non-pinned cert on the WebSocket host; the handshake must fail (same `Challenge` path as the rest of the app).
7+
final class DDPClientReceiveFailureTests: XCTestCase {
8+
func testReceiveFailureClearsConnectionAndSession() {
9+
let client = DDPClient()
10+
let session = URLSession(configuration: .ephemeral)
11+
let task = session.webSocketTask(with: URL(string: "wss://example.invalid")!)
12+
13+
client.testing_installWebSocketSync(urlSession: session, task: task)
14+
15+
let error = NSError(domain: NSURLErrorDomain, code: NSURLErrorNetworkConnectionLost, userInfo: nil)
16+
client.testing_applyReceiveResult(.failure(error), for: task)
17+
18+
let expectation = expectation(description: "connection state after receive failure")
19+
client.testing_readConnectionState { isConnected, webSocketTaskIsNil, urlSessionIsNil in
20+
XCTAssertFalse(isConnected, "Client should not stay connected after a receive failure")
21+
XCTAssertTrue(webSocketTaskIsNil, "WebSocket task should be cleared after disconnect")
22+
XCTAssertTrue(urlSessionIsNil, "URLSession should be invalidated after disconnect")
23+
expectation.fulfill()
24+
}
25+
26+
waitForExpectations(timeout: 2.0)
27+
}
28+
}

0 commit comments

Comments
 (0)