From f3d2f7266a1000655dd6667be79f2becfb3b9ebc Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 28 Oct 2026 11:44:42 -0700 Subject: [PATCH 1/6] fix(database): handle malformed server messages without crashing Realtime Database read several fields of server protocol messages without null checks: the handshake timestamp, the body and status of request responses, the data of listen acknowledgements, the reason of kill messages, and the body and path of server pushes. Connection only caught ClassCastException, so a missing field threw a NullPointerException that the run loop rethrew on the main thread. A backend change that drops one of these fields (for example the response to the stats request sent on every first connect) crashed apps on every connect. Read these fields null safely: fall back to the local clock when the handshake has no usable timestamp, fail requests whose response has no body or status with a non-null status, treat missing listen warnings as none, keep the kill behavior when the reason is missing, and ignore malformed pushes with a warning. As a safety net, catch RuntimeException while Connection processes a server message, log a warning, and close the connection, so that the persistent connection reconnects with backoff instead of crashing the app. Errors still propagate. --- firebase-database/CHANGELOG.md | 4 + .../database/connection/Connection.java | 35 +- .../connection/PersistentConnectionImpl.java | 115 ++++-- .../database/connection/ConnectionTest.java | 215 ++++++++++ .../PersistentConnectionImplTest.java | 380 ++++++++++++++++++ 5 files changed, 712 insertions(+), 37 deletions(-) create mode 100644 firebase-database/src/test/java/com/google/firebase/database/connection/ConnectionTest.java create mode 100644 firebase-database/src/test/java/com/google/firebase/database/connection/PersistentConnectionImplTest.java diff --git a/firebase-database/CHANGELOG.md b/firebase-database/CHANGELOG.md index 7918e6fa2a1..d25d9e04a78 100644 --- a/firebase-database/CHANGELOG.md +++ b/firebase-database/CHANGELOG.md @@ -1,5 +1,9 @@ # Unreleased +- [fixed] Fixed a crash when the Realtime Database server sent a message with + a missing or malformed field. The SDK now fails the affected request, ignores + the message, or reconnects instead. (#PR_NUMBER) + # 23.0.0 - [changed] **Breaking Change**: Updated minSdkVersion to API level 24 or higher. (#8638) diff --git a/firebase-database/src/main/java/com/google/firebase/database/connection/Connection.java b/firebase-database/src/main/java/com/google/firebase/database/connection/Connection.java index 1ec17c5c4b8..6c4afcda245 100644 --- a/firebase-database/src/main/java/com/google/firebase/database/connection/Connection.java +++ b/firebase-database/src/main/java/com/google/firebase/database/connection/Connection.java @@ -15,6 +15,7 @@ package com.google.firebase.database.connection; import com.google.firebase.database.logging.LogWrapper; +import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -143,8 +144,12 @@ public void onMessage(Map message) { "Failed to parse server message: missing message type:" + message.toString()); close(); } - } catch (ClassCastException e) { - if (logger.logsDebug()) logger.debug("Failed to parse server message: " + e.toString()); + } catch (RuntimeException e) { + // All handling of a server message, including the persistent connection's, runs + // synchronously from here. An unexpected message must not throw into the run loop, which + // would crash the app. Close the connection instead, so that the persistent connection + // reconnects with its usual backoff. + logger.warn("Failed to process server message, closing the connection", e); close(); } } @@ -190,8 +195,8 @@ private void onControlMessage(Map data) { if (logger.logsDebug()) logger.debug("Got invalid control message: " + data.toString()); close(); } - } catch (ClassCastException e) { - if (logger.logsDebug()) logger.debug("Failed to parse control message: " + e.toString()); + } catch (RuntimeException e) { + logger.warn("Failed to process control message, closing the connection", e); close(); } } @@ -203,7 +208,12 @@ private void onConnectionShutdown(String reason) { } private void onHandshake(Map handshake) { - long timestamp = (Long) handshake.get(SERVER_HELLO_TIMESTAMP); + if (handshake == null) { + // Every field of the handshake is optional for the client, so treat a missing payload like + // an empty one. + handshake = Collections.emptyMap(); + } + long timestamp = getServerTimestamp(handshake); String host = (String) handshake.get(SERVER_HELLO_HOST); delegate.onCacheHost(host); String sessionId = (String) handshake.get(SERVER_HELLO_SESSION_ID); @@ -214,6 +224,21 @@ private void onHandshake(Map handshake) { } } + /** + * Returns the server time from the handshake, which the client uses to compute the offset between + * the server clock and the local clock. If the server did not send a usable timestamp, this + * returns the local time instead, so that the client assumes no offset and the connection still + * works. + */ + private long getServerTimestamp(Map handshake) { + Object timestamp = handshake.get(SERVER_HELLO_TIMESTAMP); + if (timestamp instanceof Number) { + return ((Number) timestamp).longValue(); + } + logger.warn("Server handshake did not include a valid timestamp, using the local time"); + return System.currentTimeMillis(); + } + private void onConnectionReady(long timestamp, String sessionId) { if (logger.logsDebug()) logger.debug("realtime connection established"); state = State.REALTIME_CONNECTED; diff --git a/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java b/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java index 7c4e288a160..88a4219b96d 100644 --- a/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java +++ b/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java @@ -271,6 +271,12 @@ private enum ConnectionState { private static final String SERVER_RESPONSE_DATA = "d"; private static final String INVALID_APP_CHECK_TOKEN = "Invalid appcheck token"; + /** + * Status used for a server response that does not include a valid status. It is not a status the + * server sends, so it is reported to callers as an unknown error. + */ + private static final String INVALID_RESPONSE_STATUS = "invalid_response"; + /** Delay after which a established connection is considered successful */ private static final long SUCCESSFUL_CONNECTION_ESTABLISHED_DELAY = 30 * 1000; @@ -417,12 +423,12 @@ public Task get(List path, Map queryParams) { REQUEST_ACTION_GET, request, response -> { - String status = (String) response.get(REQUEST_STATUS); - if (status.equals("ok")) { + String status = getResponseStatus(response); + if ("ok".equals(status)) { Object body = response.get(SERVER_DATA_UPDATE_BODY); source.setResult(body); } else { - source.setException(new Exception((String) response.get(SERVER_DATA_UPDATE_BODY))); + source.setException(new Exception(getString(response, SERVER_DATA_UPDATE_BODY))); } }); outstandingGets.put(readId, outstandingGet); @@ -491,8 +497,12 @@ public void onDataMessage(Map message) { ConnectionRequestCallback responseListener = requestCBHash.remove(rn); if (responseListener != null) { // jackson gives up Map for json objects + Object body = message.get(RESPONSE_FOR_REQUEST); + // A response without a valid body is handled like an empty one, which the request + // callbacks treat as a failed request. @SuppressWarnings("unchecked") - Map response = (Map) message.get(RESPONSE_FOR_REQUEST); + Map response = + body instanceof Map ? (Map) body : Collections.emptyMap(); responseListener.onResponse(response); } } else if (message.containsKey(REQUEST_ERROR)) { @@ -537,7 +547,7 @@ public void onDisconnect(Connection.DisconnectReason reason) { @Override public void onKill(String reason) { - if (reason.equals(INVALID_APP_CHECK_TOKEN) + if (INVALID_APP_CHECK_TOKEN.equals(reason) && invalidAppCheckTokenCount < INVALID_TOKEN_THRESHOLD) { invalidAppCheckTokenCount++; logger.warn( @@ -847,12 +857,12 @@ private void sendOnDisconnect( new ConnectionRequestCallback() { @Override public void onResponse(Map response) { - String status = (String) response.get(REQUEST_STATUS); + String status = getResponseStatus(response); String errorMessage = null; String errorCode = null; - if (!status.equals("ok")) { + if (!"ok".equals(status)) { errorCode = status; - errorMessage = (String) response.get(SERVER_DATA_UPDATE_BODY); + errorMessage = getString(response, SERVER_DATA_UPDATE_BODY); } if (onComplete != null) { onComplete.onRequestResult(errorCode, errorMessage); @@ -931,6 +941,10 @@ private Collection removeListens(List path) { private void onDataPush(String action, Map body) { if (logger.logsDebug()) logger.debug("handleServerMessage: " + action + " " + body); + if (!isWellFormedPush(action, body)) { + logger.warn("Ignoring malformed server push for action: " + action); + return; + } if (action.equals(SERVER_ASYNC_DATA_UPDATE) || action.equals(SERVER_ASYNC_DATA_MERGE)) { boolean isMerge = action.equals(SERVER_ASYNC_DATA_MERGE); @@ -984,6 +998,22 @@ private void onDataPush(String action, Map body) { } } + /** + * Returns whether a push from the server has the fields that {@link #onDataPush} reads without + * further checks: an action, a body, and a path for pushes about a location. + */ + private static boolean isWellFormedPush(String action, Map body) { + if (action == null || body == null) { + return false; + } + boolean requiresPath = + action.equals(SERVER_ASYNC_DATA_UPDATE) + || action.equals(SERVER_ASYNC_DATA_MERGE) + || action.equals(SERVER_ASYNC_DATA_RANGE_MERGE) + || action.equals(SERVER_ASYNC_LISTEN_CANCELLED); + return !requiresPath || body.get(SERVER_DATA_UPDATE_PATH) instanceof String; + } + private void onListenRevoked(List path) { // Remove the listen and manufacture a "permission denied" error for the failed listen @@ -1039,8 +1069,8 @@ private void sendAuthHelper(final boolean restoreStateAfterComplete) { new ConnectionRequestCallback() { @Override public void onResponse(Map response) { - String status = (String) response.get(REQUEST_STATUS); - if (status.equals("ok")) { + String status = getResponseStatus(response); + if ("ok".equals(status)) { connectionState = ConnectionState.Connected; invalidAuthTokenCount = 0; sendAppCheckTokenHelper(restoreStateAfterComplete); @@ -1048,11 +1078,11 @@ public void onResponse(Map response) { authToken = null; forceAuthTokenRefresh = true; delegate.onConnectionStatus(false); - String reason = (String) response.get(SERVER_RESPONSE_DATA); + String reason = getString(response, SERVER_RESPONSE_DATA); logger.debug("Authentication failed: " + status + " (" + reason + ")"); realtime.close(); - if (status.equals("invalid_token")) { + if ("invalid_token".equals(status)) { // We'll wait a couple times before logging the warning / increasing the // retry period since oauth tokens will report as "invalid" if they're // just expired. Plus there may be transient issues that resolve themselves. @@ -1098,13 +1128,13 @@ private void sendAppCheckTokenHelper(final boolean restoreStateAfterComplete) { ConnectionRequestCallback onComplete = response -> { - String status = (String) response.get(REQUEST_STATUS); - if (status.equals("ok")) { + String status = getResponseStatus(response); + if ("ok".equals(status)) { invalidAppCheckTokenCount = 0; } else { appCheckToken = null; forceAppCheckTokenRefresh = true; - String reason = (String) response.get(SERVER_RESPONSE_DATA); + String reason = getString(response, SERVER_RESPONSE_DATA); logger.debug("App check failed: " + status + " (" + reason + ")"); // Note: We don't close the connection as the developer may not have // enforcement enabled. The backend closes connections with enforcements. @@ -1253,11 +1283,11 @@ public void onResponse(Map response) { outstandingPuts.remove(putId); if (onComplete != null) { - String status = (String) response.get(REQUEST_STATUS); - if (status.equals("ok")) { + String status = getResponseStatus(response); + if ("ok".equals(status)) { onComplete.onRequestResult(null, null); } else { - String errorMessage = (String) response.get(SERVER_DATA_UPDATE_BODY); + String errorMessage = getString(response, SERVER_DATA_UPDATE_BODY); onComplete.onRequestResult(status, errorMessage); } } @@ -1331,25 +1361,27 @@ private void sendListen(final OutstandingListen listen) { @Override public void onResponse(Map response) { - String status = (String) response.get(REQUEST_STATUS); + String status = getResponseStatus(response); // log warnings in any case, even if listener was already removed - if (status.equals("ok")) { - @SuppressWarnings("unchecked") - Map serverBody = - (Map) response.get(SERVER_DATA_UPDATE_BODY); - if (serverBody.containsKey(SERVER_DATA_WARNINGS)) { - @SuppressWarnings("unchecked") - List warnings = (List) serverBody.get(SERVER_DATA_WARNINGS); - warnOnListenerWarnings(warnings, listen.query); + if ("ok".equals(status)) { + // The body and the warnings in it are optional. + Object serverBody = response.get(SERVER_DATA_UPDATE_BODY); + if (serverBody instanceof Map) { + Object warnings = ((Map) serverBody).get(SERVER_DATA_WARNINGS); + if (warnings instanceof List) { + @SuppressWarnings("unchecked") + List warningList = (List) warnings; + warnOnListenerWarnings(warningList, listen.query); + } } } OutstandingListen currentListen = listens.get(listen.getQuery()); // only trigger actions if the listen hasn't been removed (and maybe readded) if (currentListen == listen) { - if (!status.equals("ok")) { + if (!"ok".equals(status)) { removeListen(listen.getQuery()); - String errorMessage = (String) response.get(SERVER_DATA_UPDATE_BODY); + String errorMessage = getString(response, SERVER_DATA_UPDATE_BODY); listen.resultCallback.onRequestResult(status, errorMessage); } else { listen.resultCallback.onRequestResult(null, null); @@ -1369,9 +1401,9 @@ private void sendStats(final Map stats) { new ConnectionRequestCallback() { @Override public void onResponse(Map response) { - String status = (String) response.get(REQUEST_STATUS); - if (!status.equals("ok")) { - String errorMessage = (String) response.get(SERVER_DATA_UPDATE_BODY); + String status = getResponseStatus(response); + if (!"ok".equals(status)) { + String errorMessage = getString(response, SERVER_DATA_UPDATE_BODY); if (logger.logsDebug()) { logger.debug( "Failed to send stats: " + status + " (message: " + errorMessage + ")"); @@ -1428,6 +1460,25 @@ private void sendSensitive( requestCBHash.put(rn, onResponse); } + /** + * Returns the status of a response from the server. A response without a valid status is treated + * as a failed request, so this never returns null. + */ + private String getResponseStatus(Map response) { + String status = getString(response, REQUEST_STATUS); + if (status == null) { + logger.warn("Server response did not include a valid status, treating the request as failed"); + return INVALID_RESPONSE_STATUS; + } + return status; + } + + /** Returns the value for {@code key} if it is a string, or null otherwise. */ + private static String getString(Map message, String key) { + Object value = message.get(key); + return value instanceof String ? (String) value : null; + } + private long nextRequestNumber() { return requestCounter++; } diff --git a/firebase-database/src/test/java/com/google/firebase/database/connection/ConnectionTest.java b/firebase-database/src/test/java/com/google/firebase/database/connection/ConnectionTest.java new file mode 100644 index 00000000000..83f4ecd1d66 --- /dev/null +++ b/firebase-database/src/test/java/com/google/firebase/database/connection/ConnectionTest.java @@ -0,0 +1,215 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.database.connection; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.firebase.database.MapBuilder; +import com.google.firebase.database.logging.Logger; +import java.util.Map; +import java.util.concurrent.ScheduledExecutorService; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +@RunWith(RobolectricTestRunner.class) +@Config(manifest = Config.NONE) +public class ConnectionTest { + + private static final long SERVER_TIMESTAMP = 1700000000000L; + + private final Logger logger = mock(Logger.class); + private final Connection.Delegate delegate = mock(Connection.Delegate.class); + private Connection connection; + + @Before + public void setUp() { + // Log everything, so that the debug logging code paths run too. + when(logger.getLogLevel()).thenReturn(Logger.Level.DEBUG); + ConnectionContext context = + new ConnectionContext( + logger, + mock(ConnectionTokenProvider.class), + mock(ConnectionTokenProvider.class), + mock(ScheduledExecutorService.class), + /* persistenceEnabled= */ false, + "test-sdk-version", + "test-user-agent", + "test-app-id", + /* sslCacheDirectory= */ null); + // The connection is never opened, so it does not connect to the network. Server messages are + // delivered by calling onMessage directly. + connection = + new Connection( + context, + new HostInfo("test.firebaseio.com", "test", /* secure= */ true), + /* cachedHost= */ null, + delegate, + /* optLastSessionId= */ null, + /* appCheckToken= */ ""); + } + + @Test + public void handshakeWithTimestamp_reportsServerTime() { + connection.onMessage( + handshake( + new MapBuilder() + .put("ts", SERVER_TIMESTAMP) + .put("h", "host") + .put("s", "session") + .build())); + + verify(delegate).onCacheHost("host"); + verify(delegate).onReady(SERVER_TIMESTAMP, "session"); + verify(delegate, never()).onDisconnect(any()); + } + + @Test + public void handshakeWithIntegerTimestamp_reportsServerTime() { + connection.onMessage(handshake(new MapBuilder().put("ts", 42).put("s", "session").build())); + + verify(delegate).onReady(42L, "session"); + verify(delegate, never()).onDisconnect(any()); + } + + @Test + public void handshakeWithoutTimestamp_usesLocalTime() { + assertHandshakeUsesLocalTime(new MapBuilder().put("h", "host").put("s", "session").build()); + } + + @Test + public void handshakeWithNullTimestamp_usesLocalTime() { + assertHandshakeUsesLocalTime( + new MapBuilder().put("ts", null).put("h", "host").put("s", "session").build()); + } + + @Test + public void handshakeWithNonNumericTimestamp_usesLocalTime() { + assertHandshakeUsesLocalTime( + new MapBuilder().put("ts", "not a number").put("h", "host").put("s", "session").build()); + } + + @Test + public void handshakeWithoutData_isAccepted() { + connection.onMessage(controlMessage(new MapBuilder().put("t", "h").build())); + + verify(delegate).onCacheHost(null); + verify(delegate).onReady(anyLong(), isNull()); + verify(delegate, never()).onDisconnect(any()); + } + + @Test + public void shutdownWithoutReason_killsConnection() { + connection.onMessage(controlMessage(new MapBuilder().put("t", "s").build())); + + verify(delegate).onKill(null); + verify(delegate).onDisconnect(Connection.DisconnectReason.OTHER); + } + + @Test + public void dataMessage_isPassedToDelegate() { + Map data = new MapBuilder().put("r", 1).build(); + + connection.onMessage(dataMessage(data)); + + verify(delegate).onDataMessage(data); + verify(delegate, never()).onDisconnect(any()); + } + + @Test + public void dataMessageWithoutPayload_closesConnection() { + connection.onMessage(new MapBuilder().put("t", "d").build()); + + verify(delegate).onDisconnect(Connection.DisconnectReason.OTHER); + } + + @Test + public void exceptionWhileHandlingDataMessage_closesConnection() { + doThrow(new NullPointerException()).when(delegate).onDataMessage(any()); + + connection.onMessage(dataMessage(new MapBuilder().put("r", 1).build())); + + verify(delegate).onDisconnect(Connection.DisconnectReason.OTHER); + verifyWarningLogged("closing the connection"); + } + + @Test + public void exceptionWhileHandlingControlMessage_closesConnection() { + doThrow(new IllegalStateException()).when(delegate).onReady(anyLong(), any()); + + connection.onMessage( + handshake(new MapBuilder().put("ts", SERVER_TIMESTAMP).put("s", "session").build())); + + verify(delegate).onDisconnect(Connection.DisconnectReason.OTHER); + verifyWarningLogged("closing the connection"); + } + + @Test + public void errorWhileHandlingMessage_isNotCaught() { + // Errors, such as failed internal assertions, must still surface. + doThrow(new AssertionError("internal error")).when(delegate).onDataMessage(any()); + Map message = dataMessage(new MapBuilder().put("r", 1).build()); + + assertThrows(AssertionError.class, () -> connection.onMessage(message)); + verify(delegate, never()).onDisconnect(any()); + } + + private void assertHandshakeUsesLocalTime(Map handshakeData) { + long before = System.currentTimeMillis(); + connection.onMessage(handshake(handshakeData)); + long after = System.currentTimeMillis(); + + ArgumentCaptor timestamp = ArgumentCaptor.forClass(Long.class); + verify(delegate).onCacheHost("host"); + verify(delegate).onReady(timestamp.capture(), eq("session")); + assertThat(timestamp.getValue()).isAtLeast(before); + assertThat(timestamp.getValue()).isAtMost(after); + verify(delegate, never()).onDisconnect(any()); + verifyWarningLogged("timestamp"); + } + + private void verifyWarningLogged(String text) { + verify(logger, atLeastOnce()) + .onLogMessage(eq(Logger.Level.WARN), anyString(), contains(text), anyLong()); + } + + private static Map dataMessage(Map data) { + return new MapBuilder().put("t", "d").put("d", data).build(); + } + + private static Map controlMessage(Map data) { + return new MapBuilder().put("t", "c").put("d", data).build(); + } + + private static Map handshake(Map handshakeData) { + return controlMessage(new MapBuilder().put("t", "h").put("d", handshakeData).build()); + } +} diff --git a/firebase-database/src/test/java/com/google/firebase/database/connection/PersistentConnectionImplTest.java b/firebase-database/src/test/java/com/google/firebase/database/connection/PersistentConnectionImplTest.java new file mode 100644 index 00000000000..850edba5c20 --- /dev/null +++ b/firebase-database/src/test/java/com/google/firebase/database/connection/PersistentConnectionImplTest.java @@ -0,0 +1,380 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.database.connection; + +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.ArgumentMatchers.notNull; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockConstruction; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.android.gms.tasks.Task; +import com.google.firebase.database.DatabaseError; +import com.google.firebase.database.MapBuilder; +import com.google.firebase.database.logging.Logger; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedConstruction; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +@RunWith(RobolectricTestRunner.class) +@Config(manifest = Config.NONE) +public class PersistentConnectionImplTest { + + private static final long SERVER_TIMESTAMP = 1700000000000L; + private static final List PATH = Arrays.asList("a", "b"); + private static final Map DEFAULT_QUERY = Collections.emptyMap(); + + private final Logger logger = mock(Logger.class); + private final ScheduledExecutorService executor = mock(ScheduledExecutorService.class); + private final PersistentConnection.Delegate delegate = mock(PersistentConnection.Delegate.class); + private final RequestResultCallback onComplete = mock(RequestResultCallback.class); + private MockedConstruction connections; + private PersistentConnectionImpl persistentConnection; + private Connection realtime; + + @Before + public void setUp() { + // Log everything, so that the debug logging code paths run too. + when(logger.getLogLevel()).thenReturn(Logger.Level.DEBUG); + // Replaces the realtime connection, which would open a WebSocket, with a mock. + connections = mockConstruction(Connection.class); + ConnectionContext context = + new ConnectionContext( + logger, + mock(ConnectionTokenProvider.class), + mock(ConnectionTokenProvider.class), + executor, + /* persistenceEnabled= */ false, + "1.2.3", + "test-user-agent", + "test-app-id", + /* sslCacheDirectory= */ null); + persistentConnection = + new PersistentConnectionImpl( + context, new HostInfo("test.firebaseio.com", "test", /* secure= */ true), delegate); + } + + @After + public void tearDown() { + connections.close(); + } + + @Test + public void statsResponseWithoutBody_keepsConnection() { + connect(); + // The client sends connection stats right after it connects for the first time. + int statsRequest = lastRequestNumber("s"); + + respond(statsRequest, /* body= */ null); + + verify(realtime, never()).close(); + verifyWarningLogged("status"); + } + + @Test + public void putResponseWithoutBody_failsWrite() { + connect(); + persistentConnection.put(PATH, "value", onComplete); + + respond(lastRequestNumber("p"), /* body= */ null); + + // A null status would report the write as successful, so the status must be set, and the + // client must be able to turn it into a DatabaseError. + ArgumentCaptor status = ArgumentCaptor.forClass(String.class); + verify(onComplete).onRequestResult(status.capture(), isNull()); + assertThat(status.getValue()).isNotNull(); + assertThat(DatabaseError.fromStatus(status.getValue()).getCode()) + .isEqualTo(DatabaseError.UNKNOWN_ERROR); + } + + @Test + public void putResponseWithoutStatus_failsWriteWithServerMessage() { + connect(); + persistentConnection.put(PATH, "value", onComplete); + + respond(lastRequestNumber("p"), new MapBuilder().put("d", "Something went wrong").build()); + + verify(onComplete).onRequestResult(notNull(), eq("Something went wrong")); + } + + @Test + public void putResponseOk_completesWrite() { + connect(); + persistentConnection.put(PATH, "value", onComplete); + + respond(lastRequestNumber("p"), new MapBuilder().put("s", "ok").put("d", "").build()); + + verify(onComplete).onRequestResult(null, null); + } + + @Test + public void putResponseWithError_failsWrite() { + connect(); + persistentConnection.put(PATH, "value", onComplete); + + respond( + lastRequestNumber("p"), + new MapBuilder().put("s", "permission_denied").put("d", "Permission denied").build()); + + verify(onComplete).onRequestResult("permission_denied", "Permission denied"); + } + + @Test + public void listenResponseWithoutData_completesListen() { + connect(); + listen(); + + respond(lastRequestNumber("q"), new MapBuilder().put("s", "ok").build()); + + verify(onComplete).onRequestResult(null, null); + } + + @Test + public void listenResponseWithNullWarnings_completesListen() { + connect(); + listen(); + + respond( + lastRequestNumber("q"), + new MapBuilder().put("s", "ok").put("d", new MapBuilder().put("w", null).build()).build()); + + verify(onComplete).onRequestResult(null, null); + } + + @Test + public void listenResponseWithWarnings_completesListenAndLogsWarning() { + connect(); + listen(); + + respond( + lastRequestNumber("q"), + new MapBuilder() + .put("s", "ok") + .put("d", new MapBuilder().put("w", Collections.singletonList("no_index")).build()) + .build()); + + verify(onComplete).onRequestResult(null, null); + verifyWarningLogged("indexOn"); + } + + @Test + public void listenResponseWithoutStatus_failsListen() { + connect(); + listen(); + + respond(lastRequestNumber("q"), new MapBuilder().put("d", new MapBuilder().build()).build()); + + verify(onComplete).onRequestResult(notNull(), isNull()); + } + + @Test + public void getResponseWithoutBody_failsGet() { + connect(); + Task task = persistentConnection.get(PATH, DEFAULT_QUERY); + + respond(lastRequestNumber("g"), /* body= */ null); + + assertThat(task.isComplete()).isTrue(); + assertThat(task.isSuccessful()).isFalse(); + } + + @Test + public void getResponseWithoutStatus_failsGet() { + connect(); + Task task = persistentConnection.get(PATH, DEFAULT_QUERY); + + respond( + lastRequestNumber("g"), + new MapBuilder().put("d", new MapBuilder().put("key", "value").build()).build()); + + assertThat(task.isComplete()).isTrue(); + assertThat(task.isSuccessful()).isFalse(); + } + + @Test + public void getResponseOk_returnsData() { + connect(); + Task task = persistentConnection.get(PATH, DEFAULT_QUERY); + + respond(lastRequestNumber("g"), new MapBuilder().put("s", "ok").put("d", "value").build()); + + assertThat(task.getResult()).isEqualTo("value"); + } + + @Test + public void onDisconnectResponseWithoutStatus_failsRequest() { + connect(); + persistentConnection.onDisconnectPut(PATH, "value", onComplete); + + respond(lastRequestNumber("o"), new MapBuilder().build()); + + verify(onComplete).onRequestResult(notNull(), isNull()); + } + + @Test + public void authResponseWithoutStatus_closesConnection() { + connect(/* authToken= */ "auth-token", /* appCheckToken= */ null); + + respond(lastRequestNumber("auth"), new MapBuilder().build()); + + verify(delegate).onConnectionStatus(false); + verify(realtime).close(); + } + + @Test + public void appCheckResponseWithoutStatus_keepsConnection() { + connect(/* authToken= */ null, /* appCheckToken= */ "app-check-token"); + + respond(lastRequestNumber("appcheck"), new MapBuilder().build()); + + verify(realtime, never()).close(); + } + + @Test + public void killWithoutReason_stopsReconnecting() { + connect(); + + persistentConnection.onKill(null); + + assertThat(persistentConnection.isInterrupted("server_kill")).isTrue(); + verify(realtime).close(); + } + + @Test + public void killForInvalidAppCheckToken_keepsReconnecting() { + connect(); + + persistentConnection.onKill("Invalid appcheck token"); + + assertThat(persistentConnection.isInterrupted("server_kill")).isFalse(); + } + + @Test + public void pushWithoutBody_isIgnored() { + connect(); + + for (String action : Arrays.asList("d", "m", "rm", "c", "ac", "apc", "sd")) { + persistentConnection.onDataMessage(new MapBuilder().put("a", action).build()); + } + + verifyNoDataUpdates(); + verify(realtime, never()).close(); + } + + @Test + public void pushWithoutPath_isIgnored() { + connect(); + + for (String action : Arrays.asList("d", "m", "rm", "c")) { + persistentConnection.onDataMessage( + new MapBuilder() + .put("a", action) + .put("b", new MapBuilder().put("d", "private data").build()) + .build()); + } + + verifyNoDataUpdates(); + verifyWarningLogged("push"); + // Warnings must not include the data from the push. + verify(logger, never()) + .onLogMessage(eq(Logger.Level.WARN), anyString(), contains("private data"), anyLong()); + } + + @Test + public void pushWithPath_updatesData() { + connect(); + + persistentConnection.onDataMessage( + new MapBuilder() + .put("a", "d") + .put("b", new MapBuilder().put("p", "a/b").put("d", "value").build()) + .build()); + + verify(delegate).onDataUpdate(PATH, "value", /* isMerge= */ false, /* optTag= */ null); + } + + private void connect() { + connect(/* authToken= */ null, /* appCheckToken= */ null); + } + + /** Connects to the (mocked) server, the same way a successful handshake does. */ + private void connect(String authToken, String appCheckToken) { + persistentConnection.initialize(); + // initialize() schedules a connection attempt, after the idle timer that the constructor + // schedules. The attempt starts to fetch tokens, which the mocked token providers never + // return, so open the connection with the given tokens directly. + ArgumentCaptor scheduledTask = ArgumentCaptor.forClass(Runnable.class); + verify(executor, atLeastOnce()) + .schedule(scheduledTask.capture(), anyLong(), any(TimeUnit.class)); + scheduledTask.getValue().run(); + persistentConnection.openNetworkConnection(authToken, appCheckToken); + realtime = connections.constructed().get(0); + persistentConnection.onReady(SERVER_TIMESTAMP, "session"); + } + + private void listen() { + persistentConnection.listen( + PATH, DEFAULT_QUERY, mock(ListenHashProvider.class), /* tag= */ null, onComplete); + } + + /** Returns the number of the last request sent to the server, after checking its action. */ + private int lastRequestNumber(String expectedAction) { + @SuppressWarnings("unchecked") + ArgumentCaptor> request = ArgumentCaptor.forClass(Map.class); + verify(realtime, atLeastOnce()).sendRequest(request.capture(), anyBoolean()); + assertThat(request.getValue()).containsEntry("a", expectedAction); + return ((Long) request.getValue().get("r")).intValue(); + } + + /** Delivers a response from the server. The response has no body if {@code body} is null. */ + private void respond(int requestNumber, Map body) { + MapBuilder message = new MapBuilder().put("r", requestNumber); + if (body != null) { + message.put("b", body); + } + persistentConnection.onDataMessage(message.build()); + } + + private void verifyNoDataUpdates() { + verify(delegate, never()).onDataUpdate(any(), any(), anyBoolean(), any()); + verify(delegate, never()).onRangeMergeUpdate(any(), any(), any()); + } + + private void verifyWarningLogged(String text) { + verify(logger, atLeastOnce()) + .onLogMessage(eq(Logger.Level.WARN), anyString(), contains(text), anyLong()); + } +} From 3b4861cf37afab0947f675aeb4877655931eb19f Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 28 Oct 2026 11:46:20 -0700 Subject: [PATCH 2/6] docs(database): add PR number to changelog entry --- firebase-database/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firebase-database/CHANGELOG.md b/firebase-database/CHANGELOG.md index d25d9e04a78..feaf3dc480e 100644 --- a/firebase-database/CHANGELOG.md +++ b/firebase-database/CHANGELOG.md @@ -2,7 +2,7 @@ - [fixed] Fixed a crash when the Realtime Database server sent a message with a missing or malformed field. The SDK now fails the affected request, ignores - the message, or reconnects instead. (#PR_NUMBER) + the message, or reconnects instead. (#8764) # 23.0.0 From 058eec328afb2dfee7ce415aaea2087f27fdab94 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 28 Oct 2026 13:29:20 -0700 Subject: [PATCH 3/6] fix(database): validate merge push data and describe failed gets --- .../connection/PersistentConnectionImpl.java | 21 ++++++-- .../PersistentConnectionImplTest.java | 53 +++++++++++++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java b/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java index 88a4219b96d..71affdc5339 100644 --- a/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java +++ b/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java @@ -428,7 +428,12 @@ public Task get(List path, Map queryParams) { Object body = response.get(SERVER_DATA_UPDATE_BODY); source.setResult(body); } else { - source.setException(new Exception(getString(response, SERVER_DATA_UPDATE_BODY))); + String errorMessage = getString(response, SERVER_DATA_UPDATE_BODY); + source.setException( + new Exception( + errorMessage != null + ? errorMessage + : "Get request failed with status: " + status)); } }); outstandingGets.put(readId, outstandingGet); @@ -1000,7 +1005,8 @@ private void onDataPush(String action, Map body) { /** * Returns whether a push from the server has the fields that {@link #onDataPush} reads without - * further checks: an action, a body, and a path for pushes about a location. + * further checks: an action, a body, a path for pushes about a location, and data of the expected + * type for merges and range merges. */ private static boolean isWellFormedPush(String action, Map body) { if (action == null || body == null) { @@ -1011,7 +1017,16 @@ private static boolean isWellFormedPush(String action, Map body) || action.equals(SERVER_ASYNC_DATA_MERGE) || action.equals(SERVER_ASYNC_DATA_RANGE_MERGE) || action.equals(SERVER_ASYNC_LISTEN_CANCELLED); - return !requiresPath || body.get(SERVER_DATA_UPDATE_PATH) instanceof String; + if (requiresPath && !(body.get(SERVER_DATA_UPDATE_PATH) instanceof String)) { + return false; + } + if (action.equals(SERVER_ASYNC_DATA_MERGE)) { + return body.get(SERVER_DATA_UPDATE_BODY) instanceof Map; + } + if (action.equals(SERVER_ASYNC_DATA_RANGE_MERGE)) { + return body.get(SERVER_DATA_UPDATE_BODY) instanceof List; + } + return true; } private void onListenRevoked(List path) { diff --git a/firebase-database/src/test/java/com/google/firebase/database/connection/PersistentConnectionImplTest.java b/firebase-database/src/test/java/com/google/firebase/database/connection/PersistentConnectionImplTest.java index 850edba5c20..1ea3eccb2f2 100644 --- a/firebase-database/src/test/java/com/google/firebase/database/connection/PersistentConnectionImplTest.java +++ b/firebase-database/src/test/java/com/google/firebase/database/connection/PersistentConnectionImplTest.java @@ -326,6 +326,59 @@ public void pushWithPath_updatesData() { verify(delegate).onDataUpdate(PATH, "value", /* isMerge= */ false, /* optTag= */ null); } + @Test + public void getResponseWithErrorStatusAndNoMessage_failsGetWithStatus() { + connect(); + Task task = persistentConnection.get(PATH, DEFAULT_QUERY); + + respond(lastRequestNumber("g"), new MapBuilder().put("s", "permission_denied").build()); + + assertThat(task.isComplete()).isTrue(); + assertThat(task.getException()).hasMessageThat().contains("permission_denied"); + } + + @Test + public void mergePushWithDataThatIsNotAMap_isIgnored() { + connect(); + + persistentConnection.onDataMessage( + new MapBuilder() + .put("a", "m") + .put("b", new MapBuilder().put("p", "a/b").put("d", "not a map").build()) + .build()); + + verifyNoDataUpdates(); + verify(realtime, never()).close(); + } + + @Test + public void mergePushWithMap_updatesData() { + connect(); + Map data = new MapBuilder().put("key", "value").build(); + + persistentConnection.onDataMessage( + new MapBuilder() + .put("a", "m") + .put("b", new MapBuilder().put("p", "a/b").put("d", data).build()) + .build()); + + verify(delegate).onDataUpdate(PATH, data, /* isMerge= */ true, /* optTag= */ null); + } + + @Test + public void rangeMergePushWithDataThatIsNotAList_isIgnored() { + connect(); + + persistentConnection.onDataMessage( + new MapBuilder() + .put("a", "rm") + .put("b", new MapBuilder().put("p", "a/b").put("d", "not a list").build()) + .build()); + + verifyNoDataUpdates(); + verify(realtime, never()).close(); + } + private void connect() { connect(/* authToken= */ null, /* appCheckToken= */ null); } From 5660d13a53637dfc65b5a955a875b04e51aa9311 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 28 Oct 2026 16:16:06 -0700 Subject: [PATCH 4/6] fix(database): validate range merge items and control push fields --- .../connection/PersistentConnectionImpl.java | 35 +++++++- .../PersistentConnectionImplTest.java | 79 +++++++++++++++++++ 2 files changed, 110 insertions(+), 4 deletions(-) diff --git a/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java b/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java index 71affdc5339..532693a0c41 100644 --- a/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java +++ b/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java @@ -1004,9 +1004,8 @@ private void onDataPush(String action, Map body) { } /** - * Returns whether a push from the server has the fields that {@link #onDataPush} reads without - * further checks: an action, a body, a path for pushes about a location, and data of the expected - * type for merges and range merges. + * Returns whether a push from the server has the fields and field types that {@link #onDataPush} + * casts without further checks. */ private static boolean isWellFormedPush(String action, Map body) { if (action == null || body == null) { @@ -1024,7 +1023,35 @@ private static boolean isWellFormedPush(String action, Map body) return body.get(SERVER_DATA_UPDATE_BODY) instanceof Map; } if (action.equals(SERVER_ASYNC_DATA_RANGE_MERGE)) { - return body.get(SERVER_DATA_UPDATE_BODY) instanceof List; + Object payload = body.get(SERVER_DATA_UPDATE_BODY); + if (!(payload instanceof List)) { + return false; + } + for (Object item : (List) payload) { + if (!(item instanceof Map)) { + return false; + } + Map range = (Map) item; + Object start = range.get(SERVER_DATA_START_PATH); + if (start != null && !(start instanceof String)) { + return false; + } + Object end = range.get(SERVER_DATA_END_PATH); + if (end != null && !(end instanceof String)) { + return false; + } + } + return true; + } + if (action.equals(SERVER_ASYNC_AUTH_REVOKED) || action.equals(SERVER_ASYNC_APP_CHECK_REVOKED)) { + Object status = body.get(REQUEST_STATUS); + Object reason = body.get(SERVER_DATA_UPDATE_BODY); + return (status == null || status instanceof String) + && (reason == null || reason instanceof String); + } + if (action.equals(SERVER_ASYNC_SECURITY_DEBUG)) { + Object msg = body.get("msg"); + return msg == null || msg instanceof String; } return true; } diff --git a/firebase-database/src/test/java/com/google/firebase/database/connection/PersistentConnectionImplTest.java b/firebase-database/src/test/java/com/google/firebase/database/connection/PersistentConnectionImplTest.java index 1ea3eccb2f2..450a530f19a 100644 --- a/firebase-database/src/test/java/com/google/firebase/database/connection/PersistentConnectionImplTest.java +++ b/firebase-database/src/test/java/com/google/firebase/database/connection/PersistentConnectionImplTest.java @@ -24,6 +24,7 @@ import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.ArgumentMatchers.notNull; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockConstruction; import static org.mockito.Mockito.never; @@ -379,6 +380,84 @@ public void rangeMergePushWithDataThatIsNotAList_isIgnored() { verify(realtime, never()).close(); } + @Test + public void rangeMergePushWithMalformedRangeEntry_isIgnored() { + connect(); + List badEntries = + Arrays.asList( + "not a map", + new MapBuilder().put("s", 123).put("e", "z").build(), + new MapBuilder().put("s", "a").put("e", 123).build()); + + for (Object entry : badEntries) { + persistentConnection.onDataMessage( + new MapBuilder() + .put("a", "rm") + .put( + "b", + new MapBuilder() + .put("p", "a/b") + .put("d", Collections.singletonList(entry)) + .build()) + .build()); + } + + verifyNoDataUpdates(); + verify(realtime, never()).close(); + } + + @Test + public void rangeMergePushWithValidRanges_updatesData() { + connect(); + Map range = + new MapBuilder() + .put("s", null) + .put("e", "k2") + .put("m", new MapBuilder().put("k1", "v1").build()) + .build(); + + persistentConnection.onDataMessage( + new MapBuilder() + .put("a", "rm") + .put( + "b", + new MapBuilder().put("p", "a/b").put("d", Collections.singletonList(range)).build()) + .build()); + + @SuppressWarnings("unchecked") + ArgumentCaptor> merges = ArgumentCaptor.forClass(List.class); + verify(delegate).onRangeMergeUpdate(eq(PATH), merges.capture(), isNull()); + assertThat(merges.getValue()).hasSize(1); + assertThat(merges.getValue().get(0).getOptExclusiveStart()).isNull(); + assertThat(merges.getValue().get(0).getOptInclusiveEnd()).containsExactly("k2"); + } + + @Test + public void controlPushesWithNonStringFields_areIgnored() { + connect(); + clearInvocations(delegate); + + for (String action : Arrays.asList("ac", "apc")) { + persistentConnection.onDataMessage( + new MapBuilder() + .put("a", action) + .put("b", new MapBuilder().put("s", 123).put("d", "reason").build()) + .build()); + persistentConnection.onDataMessage( + new MapBuilder() + .put("a", action) + .put("b", new MapBuilder().put("s", "expired_token").put("d", 123).build()) + .build()); + } + persistentConnection.onDataMessage( + new MapBuilder().put("a", "sd").put("b", new MapBuilder().put("msg", 123).build()).build()); + + verify(realtime, never()).close(); + verify(delegate, never()).onConnectionStatus(false); + verify(logger, never()) + .onLogMessage(eq(Logger.Level.INFO), anyString(), anyString(), anyLong()); + } + private void connect() { connect(/* authToken= */ null, /* appCheckToken= */ null); } From 89a522eac2d9164fd25e2e821658a617624b6682 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 28 Oct 2026 17:16:22 -0700 Subject: [PATCH 5/6] fix(database): guard against null map in getString helper Return null immediately from PersistentConnectionImpl.getString when the input map is null instead of throwing a NullPointerException. --- .../firebase/database/connection/PersistentConnectionImpl.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java b/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java index 532693a0c41..6086dc62e6f 100644 --- a/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java +++ b/firebase-database/src/main/java/com/google/firebase/database/connection/PersistentConnectionImpl.java @@ -1517,6 +1517,9 @@ private String getResponseStatus(Map response) { /** Returns the value for {@code key} if it is a string, or null otherwise. */ private static String getString(Map message, String key) { + if (message == null) { + return null; + } Object value = message.get(key); return value instanceof String ? (String) value : null; } From b6e7523f4a0e601f7c11a0e0275efafddcfa5dad Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 28 Oct 2026 17:44:13 -0700 Subject: [PATCH 6/6] fix(database): extract handshake host and session ID safely Check instanceof String before casting handshake host and session ID fields so non-string values default to null instead of closing the connection and reconnecting. --- .../firebase/database/connection/Connection.java | 6 ++++-- .../firebase/database/connection/ConnectionTest.java | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/firebase-database/src/main/java/com/google/firebase/database/connection/Connection.java b/firebase-database/src/main/java/com/google/firebase/database/connection/Connection.java index 6c4afcda245..6a74a794133 100644 --- a/firebase-database/src/main/java/com/google/firebase/database/connection/Connection.java +++ b/firebase-database/src/main/java/com/google/firebase/database/connection/Connection.java @@ -214,9 +214,11 @@ private void onHandshake(Map handshake) { handshake = Collections.emptyMap(); } long timestamp = getServerTimestamp(handshake); - String host = (String) handshake.get(SERVER_HELLO_HOST); + Object hostObj = handshake.get(SERVER_HELLO_HOST); + String host = hostObj instanceof String ? (String) hostObj : null; delegate.onCacheHost(host); - String sessionId = (String) handshake.get(SERVER_HELLO_SESSION_ID); + Object sessionIdObj = handshake.get(SERVER_HELLO_SESSION_ID); + String sessionId = sessionIdObj instanceof String ? (String) sessionIdObj : null; if (state == State.REALTIME_CONNECTING) { conn.start(); diff --git a/firebase-database/src/test/java/com/google/firebase/database/connection/ConnectionTest.java b/firebase-database/src/test/java/com/google/firebase/database/connection/ConnectionTest.java index 83f4ecd1d66..9844c6d7e7c 100644 --- a/firebase-database/src/test/java/com/google/firebase/database/connection/ConnectionTest.java +++ b/firebase-database/src/test/java/com/google/firebase/database/connection/ConnectionTest.java @@ -126,6 +126,17 @@ public void handshakeWithoutData_isAccepted() { verify(delegate, never()).onDisconnect(any()); } + @Test + public void handshakeWithNonStringHostAndSessionId_isAccepted() { + connection.onMessage( + handshake( + new MapBuilder().put("ts", SERVER_TIMESTAMP).put("h", 123).put("s", 456).build())); + + verify(delegate).onCacheHost(null); + verify(delegate).onReady(SERVER_TIMESTAMP, null); + verify(delegate, never()).onDisconnect(any()); + } + @Test public void shutdownWithoutReason_killsConnection() { connection.onMessage(controlMessage(new MapBuilder().put("t", "s").build()));