[paulb777]

Summary

Realtime Database no longer crashes the app when the server sends a host (in
the handshake or in a reset message) or a session ID that can't be used in a
URL. The SDK ignores the bad value and keeps connecting.

Problem

The server sends a host to use for later connections in the handshake (h) and
in a reset control message (d), and a session ID in the handshake (s).
PersistentConnectionImpl stores them in memory (cachedHost,
lastSessionId) without checking them. On the next connection,
WebsocketConnection.createConnection builds the URL with URI.create, which
throws IllegalArgumentException if a value contains a character that isn't
allowed in a URI (space, |, ^, {, ", backslash, and so on). This runs
on the database run loop as part of the reconnect, and Android rethrows
uncaught run loop exceptions on the main thread, so the app crashes. After a
reset, the crash is immediate, because SERVER_RESET makes the persistent
connection reconnect without delay.

Nothing is persisted, but the crash recurs in every session while the server
keeps sending the value. A related case doesn't crash: a host whose URL is a
valid URI with no host in it (for example ""). The websocket then connects
to the loopback address (new Socket(null, port)), so the client can't
reconnect for the rest of the session.

The trigger needs a backend bug, regression, or protocol change. This comes
from an expanded audit of how the SDK handles unexpected server data, which
followed
firebase/firebase-ios-sdk#16728.

Fix

• HostInfo.isUsableHost(host) is the one shared check: the connection URL
  for the host must be a valid URI that names a host. It only checks syntax
  and isn't an allow-list. Production hosts
  (s-usc1b-nss-2073.firebaseio.com, x.europe-west1.firebasedatabase.app)
  and emulator hosts (localhost:9000, 10.0.2.2:9000, [::1]:9000) are
  accepted.
• WebsocketConnection builds its URL with the new
  HostInfo.getConnectionUrl(cachedHost, lastSessionId, logger). An unusable
  cached host is replaced with the configured host, and an unusable session ID
  is dropped. Each case logs a warning that doesn't include the value.
• Each value is dropped on its own because of a loop. Suppose a bad session ID
  also discarded a good cached host. The client would connect to the
  configured host. If that host reset the connection to the cached host (as
  during a namespace move), SERVER_RESET would reset the backoff, the next
  URL would fail the same way, and the client would go back to the configured
  host. That repeats with no backoff.
• Connection.onReset no longer caches a non-null host that isn't usable. It
  closes with DisconnectReason.OTHER instead of SERVER_RESET, so the
  persistent connection reconnects with its usual backoff. Otherwise a server
  that resets every connection with the same bad host would cause a tight
  reconnect loop. A null host behaves as before.
• This doesn't touch the methods that open PR fix(database): handle malformed server messages without crashing #8764 changes (onMessage,
  onControlMessage, onHandshake). A trial merge with fix(database): handle malformed server messages without crashing #8764 conflicts only
  in CHANGELOG.md, and the connection tests of both PRs pass together.

Alternatives considered:

• Catching the exception around new Connection(...) in
  PersistentConnectionImpl: the bad value would stay cached, so every
  reconnect would fail for the rest of the session.
• Validating values when they are stored: onHandshake is being rewritten in
  fix(database): handle malformed server messages without crashing #8764. Checking when the URL is built covers every path with one check.
• URL-encoding the session ID: this would change the URL for session IDs that
  work today (for example ones that contain + or %).
• A host allow-list (for example *.firebaseio.com): this would break the
  emulator and other setups.
• Falling back to the configured host without a session ID when either value
  is bad: this causes the loop described above.

Behavior changes to review:

• A reset with an unusable host keeps the previous host and reconnects with
  backoff.
• An unusable host from the handshake makes reconnects go to the configured
  host. An unusable session ID makes reconnects omit the ls parameter.
• A host whose URL doesn't name a host (for example "") is ignored instead
  of making the websocket connect to the loopback address.

Testing

New Robolectric tests in
firebase-database/src/test/java/com/google/firebase/database/connection/:

• HostInfoTest (12 tests): accepts the production and emulator hosts above.
  Rejects hosts with characters that aren't allowed in a URI, an empty host,
  and a non-numeric port. Covers URL selection for each combination of valid
  and invalid host and session ID, and the emulator's ws:// scheme. Checks
  that warnings don't include the values.
• WebsocketConnectionTest (5 tests): creates a WebsocketConnection the way
  a reconnect does and records the URL passed to WebSocket with
  Mockito.mockConstruction. With an invalid host or session ID, the
  constructor no longer throws, and valid values are used as before.
• ConnectionResetTest (4 tests): legitimate hosts and a null host still call
  onCacheHost and close with SERVER_RESET. Invalid hosts aren't cached,
  close with OTHER, and log a warning that doesn't include the host.

I wrote the tests first and ran them against the unmodified main source. 5 of
9 failed: 3 of 5 in WebsocketConnectionTest, with the
IllegalArgumentException from URI.create (the crash), and 2 of 4 in
ConnectionResetTest. HostInfoTest uses the new helper, so it can't compile
without the fix.

Commands, all with --max-workers=4:

• Before the fix:
  ./gradlew :firebase-database:testDebugUnitTest --tests "com.google.firebase.database.connection.WebsocketConnectionTest" --tests "com.google.firebase.database.connection.ConnectionResetTest":
  9 tests, 5 failed.
• ./gradlew :firebase-database:spotlessApply
• ./gradlew :firebase-database:testDebugUnitTest --tests "com.google.firebase.database.connection.*":
  21 tests, 0 failures.
• ./gradlew :firebase-database:testDebugUnitTest: 424 tests, 0 failures, 2
  skipped.
• On a trial merge with fix(database): handle malformed server messages without crashing #8764's branch,
  ./gradlew :firebase-database:testDebugUnitTest --tests "com.google.firebase.database.connection.*":
  61 tests, 0 failures.

Not tested on a device or emulator.

[gemini-code-assist]

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

[github-actions]

📝 PRs merging into main branch

Our main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released.

[paulb777]

/gemini review

[gemini-code-assist]

Code Review

This pull request fixes a crash in the Realtime Database SDK when the server sends an invalid host or session ID. The SDK now validates these values before constructing connection URLs. Specifically, HostInfo checks if the host or session ID is usable, falling back to the configured host or omitting the invalid session ID while logging a warning. Connection also ignores invalid hosts during a reset and closes the connection with a reason of OTHER to prevent a tight reconnect loop. Comprehensive unit tests have been added to verify these changes. There are no review comments, and I have no feedback to provide.