From 73e4a1d782c54ae9f1496d78eec3f831439a5ba0 Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Wed, 24 Jun 2026 16:48:16 +0000 Subject: [PATCH 1/3] fix(android): guard WebSocket methods against missing client NPE Replace non-null assertion operators (!!) with safe access patterns across all WebSocket client methods to prevent NullPointerException when a client doesn't exist for a given URI. - invalidateClientFor: use ?.let for safe access, resolve even if client doesn't exist (desired state is already achieved) - connectFor, disconnectFor, sendDataFor: explicit null check with promise rejection instead of relying on NPE caught by try/catch - ensureClientFor: use ?.let instead of containsKey + !! Fixes Sentry #7153287556 (1,568 events / 1,074 affected users) Co-authored-by: Claude --- .../WebSocketClientModuleImpl.kt | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt b/android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt index 07ce7c730..a46ce1265 100644 --- a/android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt +++ b/android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt @@ -44,8 +44,8 @@ class WebSocketClientModuleImpl(reactApplicationContext: ReactApplicationContext return promise.reject(error) } - if (clients.containsKey(wsUri)) { - clients[wsUri]!!.webSocket?.close(1000, null) + clients[wsUri]?.let { client -> + client.webSocket?.close(1000, null) clients.remove(wsUri) } @@ -82,9 +82,10 @@ class WebSocketClientModuleImpl(reactApplicationContext: ReactApplicationContext return promise.reject(error) } - - clients[wsUri]!!.webSocket?.close(1000, null) - clients.remove(wsUri) + clients[wsUri]?.let { client -> + client.webSocket?.close(1000, null) + clients.remove(wsUri) + } promise.resolve(null) } @@ -97,8 +98,13 @@ class WebSocketClientModuleImpl(reactApplicationContext: ReactApplicationContext return promise.reject(error) } + val client = clients[wsUri] + if (client == null) { + return promise.reject("WebSocket error", "no client for this websocket url") + } + try { - clients[wsUri]!!.createWebSocket() + client.createWebSocket() promise.resolve(null) } catch (error: Exception) { promise.reject(error) @@ -113,8 +119,13 @@ class WebSocketClientModuleImpl(reactApplicationContext: ReactApplicationContext return promise.reject(error) } + val client = clients[wsUri] + if (client == null) { + return promise.reject("WebSocket error", "no client for this websocket url") + } + try { - clients[wsUri]!!.webSocket!!.cancel() + client.webSocket?.cancel() promise.resolve(null) } catch (error: Exception) { promise.reject(error) @@ -130,8 +141,13 @@ class WebSocketClientModuleImpl(reactApplicationContext: ReactApplicationContext } + val client = clients[wsUri] + if (client == null) { + return promise.reject("WebSocket error", "no client for this websocket url") + } + try { - clients[wsUri]!!.webSocket!!.send(data) + client.webSocket?.send(data) promise.resolve(null) } catch (error: Exception) { promise.reject(error) From 5c15e25e2aa8113148d2dc80855cd64530b367d2 Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Wed, 24 Jun 2026 16:57:46 +0000 Subject: [PATCH 2/3] fix(android): reject promise when WebSocket send fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check the return value of webSocket.send() — returns false when OkHttp declines the frame (e.g. socket closing) and null when the socket is missing. Only resolve the promise on a confirmed send. Addresses CodeRabbit review feedback on PR #170. Co-authored-by: Claude --- .../mattermost/networkclient/WebSocketClientModuleImpl.kt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt b/android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt index a46ce1265..4ce8d0fdf 100644 --- a/android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt +++ b/android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt @@ -147,8 +147,12 @@ class WebSocketClientModuleImpl(reactApplicationContext: ReactApplicationContext } try { - client.webSocket?.send(data) - promise.resolve(null) + val sent = client.webSocket?.send(data) + if (sent == true) { + promise.resolve(null) + } else { + promise.reject("WebSocket error", "websocket is not connected or failed to send data") + } } catch (error: Exception) { promise.reject(error) } From f86a8b4df2be224ad56f3754faa5c8a377d7bf89 Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Wed, 24 Jun 2026 16:59:56 +0000 Subject: [PATCH 3/3] fix(android): guard all ApiClient methods against missing client NPE Replaced unsafe `clients[url]!!` pattern with null-safe checks across: - getClientHeadersFor - addClientHeadersFor - importClientP12For - invalidateClientFor - download - upload All methods now reject promise with clear error when client doesn't exist for the given URL, matching the pattern already established in WebSocket methods. Sentry issue: MATTERMOST-ANDROID-TEST-2E / MM-68210 Co-authored-by: Claude --- .../networkclient/ApiClientModuleImpl.kt | 40 +++++++++++-- .../WebSocketClientNullSafetyTest.kt | 56 +++++++++++++++++++ 2 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 android/src/test/java/com/mattermost/networkclient/WebSocketClientNullSafetyTest.kt diff --git a/android/src/main/java/com/mattermost/networkclient/ApiClientModuleImpl.kt b/android/src/main/java/com/mattermost/networkclient/ApiClientModuleImpl.kt index 3e2244754..eb6c5e350 100644 --- a/android/src/main/java/com/mattermost/networkclient/ApiClientModuleImpl.kt +++ b/android/src/main/java/com/mattermost/networkclient/ApiClientModuleImpl.kt @@ -160,8 +160,13 @@ class ApiClientModuleImpl(appContext: Context) { return promise.reject(error) } + val client = clients[url] + if (client == null) { + return promise.reject(Error("Client not found for baseUrl: $url")) + } + try { - promise.resolve(clients[url]!!.clientHeaders) + promise.resolve(client.clientHeaders) } catch (error: Exception) { promise.reject(error) } @@ -175,8 +180,13 @@ class ApiClientModuleImpl(appContext: Context) { return promise.reject(error) } + val client = clients[url] + if (client == null) { + return promise.reject(Error("Client not found for baseUrl: $url")) + } + try { - clients[url]!!.addClientHeaders(headers) + client.addClientHeaders(headers) promise.resolve(null) } catch (error: Exception) { promise.reject(error) @@ -191,8 +201,13 @@ class ApiClientModuleImpl(appContext: Context) { return promise.reject(error) } + val client = clients[url] + if (client == null) { + return promise.reject(Error("Client not found for baseUrl: $url")) + } + try { - clients[url]!!.importClientP12AndRebuildClient(path, password) + client.importClientP12AndRebuildClient(path, password) promise.resolve(null) } catch (error: Exception) { promise.reject(error) @@ -207,8 +222,13 @@ class ApiClientModuleImpl(appContext: Context) { return promise.reject(error) } + val client = clients[url] + if (client == null) { + return promise.reject(Error("Client not found for baseUrl: $url")) + } + try { - clients[url]!!.invalidate() + client.invalidate() clients.remove(url) promise.resolve(null) } catch (error: Exception) { @@ -254,7 +274,11 @@ class ApiClientModuleImpl(appContext: Context) { return promise.reject(Error("Couldn't create dir: " + parent.path)) } - val client = clients[url]!! + val client = clients[url] + if (client == null) { + return promise.reject(Error("Client not found for baseUrl: $url")) + } + val downloadCall = client.buildDownloadCall(endpoint, taskId, options) calls[taskId] = downloadCall @@ -314,7 +338,11 @@ class ApiClientModuleImpl(appContext: Context) { return promise.reject(error) } - val client = clients[url]!! + val client = clients[url] + if (client == null) { + return promise.reject(Error("Client not found for baseUrl: $url")) + } + val uploadCall = client.buildUploadCall(endpoint, filePath, taskId, options) calls[taskId] = uploadCall diff --git a/android/src/test/java/com/mattermost/networkclient/WebSocketClientNullSafetyTest.kt b/android/src/test/java/com/mattermost/networkclient/WebSocketClientNullSafetyTest.kt new file mode 100644 index 000000000..8ff6c0fcc --- /dev/null +++ b/android/src/test/java/com/mattermost/networkclient/WebSocketClientNullSafetyTest.kt @@ -0,0 +1,56 @@ +package com.mattermost.networkclient + +import org.junit.Assert.assertNull +import org.junit.Assert.fail +import org.junit.Test + +/** + * Reproduces Sentry #7153287556: NullPointerException when accessing a + * map entry that doesn't exist using Kotlin's force-unwrap operator (!!). + * + * WebSocketClientModuleImpl stores clients in a MutableMap. + * The old code used clients[wsUri]!! which crashes when the URI isn't in the map. + * The fix uses clients[wsUri]?.let { ... } for safe access. + */ +class WebSocketClientNullSafetyTest { + + /** + * Demonstrates the crash: force-unwrap (!!) on a missing map key throws + * KotlinNullPointerException. This is exactly what happened in + * invalidateClientFor() before the fix. + */ + @Test(expected = NullPointerException::class) + fun forceUnwrap_onMissingMapKey_throwsNPE() { + val clients = mutableMapOf() + + // Old pattern: clients[key]!! — crashes when key doesn't exist + clients["ws://nonexistent"]!! + } + + /** + * The fix: safe access with ?.let returns null instead of crashing. + * invalidateClientFor should be idempotent — invalidating a + * non-existent client is not an error. + */ + @Test + fun safeAccess_onMissingMapKey_returnsNull() { + val clients = mutableMapOf() + + // New pattern: clients[key]?.let { ... } — no crash + val result = clients["ws://nonexistent"]?.let { it } + assertNull(result) + } + + /** + * Verifies that safe access still works correctly when the key exists. + */ + @Test + fun safeAccess_onExistingMapKey_returnsValue() { + val clients = mutableMapOf() + clients["ws://example.com"] = "client" + + var accessed = false + clients["ws://example.com"]?.let { accessed = true } + assert(accessed) { "Expected ?.let block to execute for existing key" } + } +}