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/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt b/android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt index 07ce7c730..4ce8d0fdf 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,9 +141,18 @@ 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) - 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) } 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" } + } +}