From 35cf8fb89c1d962030957b4d32050563847bdc69 Mon Sep 17 00:00:00 2001 From: Ademola Fadumo <48495111+demolaf@users.noreply.github.com> Date: Mon, 7 Sep 2026 03:25:12 +0100 Subject: [PATCH 1/4] fix(auth): keep the session id when the continue URL already has a query --- .../ui/auth/util/ContinueUrlBuilder.kt | 39 +++- .../ui/auth/util/ContinueUrlBuilderTest.kt | 189 ++++++++++++++++++ .../ui/auth/util/EmailLinkParserTest.kt | 132 ++++++++++++ 3 files changed, 350 insertions(+), 10 deletions(-) create mode 100644 auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt create mode 100644 auth/src/test/java/com/firebase/ui/auth/util/EmailLinkParserTest.kt diff --git a/auth/src/main/java/com/firebase/ui/auth/util/ContinueUrlBuilder.kt b/auth/src/main/java/com/firebase/ui/auth/util/ContinueUrlBuilder.kt index 80efbd8bd..5e7698830 100644 --- a/auth/src/main/java/com/firebase/ui/auth/util/ContinueUrlBuilder.kt +++ b/auth/src/main/java/com/firebase/ui/auth/util/ContinueUrlBuilder.kt @@ -22,15 +22,28 @@ import com.firebase.ui.auth.util.EmailLinkParser.LinkParameters.SESSION_IDENTIFI /** * Builder for constructing continue URLs with embedded session and authentication parameters. * Used in email link sign-in flows to pass state between devices. + * + * The incoming URL comes from the consumer's [com.google.firebase.auth.ActionCodeSettings], so it + * may already carry a query string and/or a fragment. Appended parameters join an existing query + * with `&` and are always placed before the fragment. */ @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) class ContinueUrlBuilder(url: String) { - private val continueUrl: StringBuilder + /** The incoming URL up to, but not including, the fragment. */ + private val baseUrl: String + + /** The fragment including its leading `#`, or empty when the URL has none. */ + private val fragment: String + + private val params = StringBuilder() init { require(url.isNotBlank()) { "URL cannot be empty" } - continueUrl = StringBuilder(url).append("?") + + val fragmentStart = url.indexOf('#') + baseUrl = if (fragmentStart == -1) url else url.substring(0, fragmentStart) + fragment = if (fragmentStart == -1) "" else url.substring(fragmentStart) } fun appendSessionId(sessionId: String): ContinueUrlBuilder { @@ -57,16 +70,22 @@ class ContinueUrlBuilder(url: String) { private fun addQueryParam(key: String, value: String) { if (value.isBlank()) return - val isFirstParam = continueUrl.last() == '?' - val mark = if (isFirstParam) "" else "&" - continueUrl.append("$mark$key=$value") + if (params.isNotEmpty()) { + params.append("&") + } + params.append("$key=$value") } fun build(): String { - if (continueUrl.last() == '?') { - // No params added so we remove the '?' - continueUrl.setLength(continueUrl.length - 1) + // No params added, so the URL is handed back untouched. + if (params.isEmpty()) return baseUrl + fragment + + val separator = when { + !baseUrl.contains('?') -> "?" + // The query is already open (`...?` or `...&`), so no separator is needed. + baseUrl.endsWith('?') || baseUrl.endsWith('&') -> "" + else -> "&" } - return continueUrl.toString() + return baseUrl + separator + params + fragment } -} \ No newline at end of file +} diff --git a/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt b/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt new file mode 100644 index 000000000..bc2d62844 --- /dev/null +++ b/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt @@ -0,0 +1,189 @@ +/* + * Copyright 2025 Google Inc. All Rights Reserved. + * + * 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.firebase.ui.auth.util + +import androidx.core.net.toUri +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Unit tests for [ContinueUrlBuilder]. Runs under Robolectric so the assertions can read the + * built URLs back through the real [android.net.Uri] parser rather than by string matching. + * + * @suppress Internal test class + */ +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE) +class ContinueUrlBuilderTest { + + @Test + fun `url without a query gets a question mark separator`() { + val url = ContinueUrlBuilder("https://example.com/finish") + .appendSessionId("sid123") + .appendAnonymousUserId("auid456") + .appendProviderId("google.com") + .appendForceSameDeviceBit(true) + .build() + + assertThat(url).isEqualTo( + "https://example.com/finish?ui_sid=sid123&ui_auid=auid456&ui_pid=google.com&ui_sd=1" + ) + + val uri = url.toUri() + assertThat(uri.getQueryParameter("ui_sid")).isEqualTo("sid123") + assertThat(uri.getQueryParameter("ui_auid")).isEqualTo("auid456") + assertThat(uri.getQueryParameter("ui_pid")).isEqualTo("google.com") + assertThat(uri.getQueryParameter("ui_sd")).isEqualTo("1") + } + + @Test + fun `url with an existing query gets an ampersand separator and keeps the consumer's params`() { + val url = ContinueUrlBuilder("https://example.com/finish?demo=fullcustomization") + .appendSessionId("sid123") + .appendAnonymousUserId("auid456") + .appendProviderId("google.com") + .appendForceSameDeviceBit(true) + .build() + + assertThat(url.count { it == '?' }).isEqualTo(1) + + val uri = url.toUri() + assertThat(uri.getQueryParameter("demo")).isEqualTo("fullcustomization") + assertThat(uri.getQueryParameter("ui_sid")).isEqualTo("sid123") + assertThat(uri.getQueryParameter("ui_auid")).isEqualTo("auid456") + assertThat(uri.getQueryParameter("ui_pid")).isEqualTo("google.com") + assertThat(uri.getQueryParameter("ui_sd")).isEqualTo("1") + } + + @Test + fun `url with several existing params keeps every one of them`() { + val url = ContinueUrlBuilder("https://example.com/finish?demo=full&lang=en") + .appendSessionId("sid123") + .build() + + val uri = url.toUri() + assertThat(uri.getQueryParameter("demo")).isEqualTo("full") + assertThat(uri.getQueryParameter("lang")).isEqualTo("en") + assertThat(uri.getQueryParameter("ui_sid")).isEqualTo("sid123") + } + + @Test + fun `url ending in an open query marker is not given a second separator`() { + assertThat( + ContinueUrlBuilder("https://example.com/finish?").appendSessionId("sid123").build() + ).isEqualTo("https://example.com/finish?ui_sid=sid123") + + assertThat( + ContinueUrlBuilder("https://example.com/finish?demo=full&") + .appendSessionId("sid123") + .build() + ).isEqualTo("https://example.com/finish?demo=full&ui_sid=sid123") + } + + @Test + fun `params are appended before the fragment`() { + val url = ContinueUrlBuilder("https://example.com/finish?demo=full#section") + .appendSessionId("sid123") + .appendForceSameDeviceBit(false) + .build() + + assertThat(url) + .isEqualTo("https://example.com/finish?demo=full&ui_sid=sid123&ui_sd=0#section") + + val uri = url.toUri() + assertThat(uri.getQueryParameter("ui_sid")).isEqualTo("sid123") + assertThat(uri.getQueryParameter("ui_sd")).isEqualTo("0") + assertThat(uri.fragment).isEqualTo("section") + } + + @Test + fun `fragment on a url without a query still gets a question mark separator`() { + val url = ContinueUrlBuilder("https://example.com/finish#section") + .appendSessionId("sid123") + .build() + + assertThat(url).isEqualTo("https://example.com/finish?ui_sid=sid123#section") + assertThat(url.toUri().getQueryParameter("ui_sid")).isEqualTo("sid123") + } + + @Test + fun `blank values are skipped`() { + val url = ContinueUrlBuilder("https://example.com/finish") + .appendSessionId("") + .appendAnonymousUserId(" ") + .appendProviderId("google.com") + .build() + + assertThat(url).isEqualTo("https://example.com/finish?ui_pid=google.com") + + val uri = url.toUri() + assertThat(uri.getQueryParameter("ui_sid")).isNull() + assertThat(uri.getQueryParameter("ui_auid")).isNull() + } + + @Test + fun `blank first value does not steal the separator from the next param`() { + val url = ContinueUrlBuilder("https://example.com/finish?demo=full") + .appendSessionId("") + .appendAnonymousUserId("auid456") + .build() + + assertThat(url).isEqualTo("https://example.com/finish?demo=full&ui_auid=auid456") + } + + @Test + fun `no params leaves the url untouched`() { + assertThat(ContinueUrlBuilder("https://example.com/finish").build()) + .isEqualTo("https://example.com/finish") + + assertThat(ContinueUrlBuilder("https://example.com/finish?demo=full").build()) + .isEqualTo("https://example.com/finish?demo=full") + + assertThat(ContinueUrlBuilder("https://example.com/finish?demo=full#section").build()) + .isEqualTo("https://example.com/finish?demo=full#section") + } + + @Test + fun `path encoded url is untouched`() { + val url = ContinueUrlBuilder("https://example.com/finish/fullcustomization") + .appendSessionId("sid123") + .build() + + assertThat(url).isEqualTo("https://example.com/finish/fullcustomization?ui_sid=sid123") + + val uri = url.toUri() + assertThat(uri.lastPathSegment).isEqualTo("fullcustomization") + assertThat(uri.getQueryParameter("ui_sid")).isEqualTo("sid123") + } + + @Test + fun `force same device bit is written as one or zero`() { + assertThat( + ContinueUrlBuilder("https://example.com/finish").appendForceSameDeviceBit(true).build() + ).isEqualTo("https://example.com/finish?ui_sd=1") + + assertThat( + ContinueUrlBuilder("https://example.com/finish").appendForceSameDeviceBit(false).build() + ).isEqualTo("https://example.com/finish?ui_sd=0") + } + + @Test(expected = IllegalArgumentException::class) + fun `blank url is rejected`() { + ContinueUrlBuilder(" ") + } +} diff --git a/auth/src/test/java/com/firebase/ui/auth/util/EmailLinkParserTest.kt b/auth/src/test/java/com/firebase/ui/auth/util/EmailLinkParserTest.kt new file mode 100644 index 000000000..76c3a9d7f --- /dev/null +++ b/auth/src/test/java/com/firebase/ui/auth/util/EmailLinkParserTest.kt @@ -0,0 +1,132 @@ +/* + * Copyright 2025 Google Inc. All Rights Reserved. + * + * 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.firebase.ui.auth.util + +import androidx.core.net.toUri +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Unit tests for [EmailLinkParser], including round-trips of the continue URLs that + * [ContinueUrlBuilder] produces. + * + * @suppress Internal test class + */ +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE) +class EmailLinkParserTest { + + @Test + fun `parses the parameters of a continue url built from a url without a query`() { + val parser = EmailLinkParser( + ContinueUrlBuilder("https://example.com/finish") + .appendSessionId("sid123") + .appendAnonymousUserId("auid456") + .appendProviderId("google.com") + .appendForceSameDeviceBit(true) + .build() + ) + + assertThat(parser.sessionId).isEqualTo("sid123") + assertThat(parser.anonymousUserId).isEqualTo("auid456") + assertThat(parser.providerId).isEqualTo("google.com") + assertThat(parser.forceSameDeviceBit).isTrue() + } + + @Test + fun `parses the parameters of a continue url built from a url that already had a query`() { + val parser = EmailLinkParser( + ContinueUrlBuilder("https://example.com/finish?demo=fullcustomization") + .appendSessionId("sid123") + .appendAnonymousUserId("auid456") + .appendProviderId("google.com") + .appendForceSameDeviceBit(true) + .build() + ) + + assertThat(parser.sessionId).isEqualTo("sid123") + assertThat(parser.anonymousUserId).isEqualTo("auid456") + assertThat(parser.providerId).isEqualTo("google.com") + assertThat(parser.forceSameDeviceBit).isTrue() + } + + @Test + fun `parses the parameters of a continue url that had a query and a fragment`() { + val parser = EmailLinkParser( + ContinueUrlBuilder("https://example.com/finish?demo=fullcustomization#section") + .appendSessionId("sid123") + .appendForceSameDeviceBit(false) + .build() + ) + + assertThat(parser.sessionId).isEqualTo("sid123") + assertThat(parser.forceSameDeviceBit).isFalse() + } + + @Test + fun `parses the parameters out of the continue url nested in an email link`() { + val continueUrl = ContinueUrlBuilder("https://example.com/finish?demo=fullcustomization") + .appendSessionId("sid123") + .appendAnonymousUserId("auid456") + .appendForceSameDeviceBit(true) + .build() + + val emailLink = "https://example.firebaseapp.com/__/auth/action".toUri() + .buildUpon() + .appendQueryParameter("mode", "signIn") + .appendQueryParameter("oobCode", "oob789") + .appendQueryParameter("continueUrl", continueUrl) + .build() + .toString() + + val parser = EmailLinkParser(emailLink) + + assertThat(parser.oobCode).isEqualTo("oob789") + assertThat(parser.sessionId).isEqualTo("sid123") + assertThat(parser.anonymousUserId).isEqualTo("auid456") + assertThat(parser.forceSameDeviceBit).isTrue() + } + + @Test + fun `missing optional parameters read back as null`() { + val parser = EmailLinkParser( + ContinueUrlBuilder("https://example.com/finish").appendSessionId("sid123").build() + ) + + assertThat(parser.sessionId).isEqualTo("sid123") + assertThat(parser.anonymousUserId).isNull() + assertThat(parser.providerId).isNull() + // The bit defaults to false when the link does not carry one. + assertThat(parser.forceSameDeviceBit).isFalse() + } + + @Test(expected = IllegalArgumentException::class) + fun `blank link is rejected`() { + EmailLinkParser(" ") + } + + @Test(expected = IllegalArgumentException::class) + fun `link without parameters is rejected`() { + EmailLinkParser("https://example.com/finish") + } + + @Test(expected = IllegalArgumentException::class) + fun `missing oob code is rejected when read`() { + EmailLinkParser("https://example.com/finish?ui_sid=sid123").oobCode + } +} From 681c2b70e8404a5e129c63b48a88d8b142307a0d Mon Sep 17 00:00:00 2001 From: Ademola Fadumo <48495111+demolaf@users.noreply.github.com> Date: Wed, 9 Sep 2026 12:56:20 +0100 Subject: [PATCH 2/4] fix(auth): don't read a literal ? in a query value as an open query --- .../ui/auth/util/ContinueUrlBuilder.kt | 12 ++-- .../ui/auth/util/ContinueUrlBuilderTest.kt | 57 ++++++++++++++++++- .../ui/auth/util/EmailLinkParserTest.kt | 29 ++++++++++ 3 files changed, 92 insertions(+), 6 deletions(-) diff --git a/auth/src/main/java/com/firebase/ui/auth/util/ContinueUrlBuilder.kt b/auth/src/main/java/com/firebase/ui/auth/util/ContinueUrlBuilder.kt index 5e7698830..517278af8 100644 --- a/auth/src/main/java/com/firebase/ui/auth/util/ContinueUrlBuilder.kt +++ b/auth/src/main/java/com/firebase/ui/auth/util/ContinueUrlBuilder.kt @@ -24,8 +24,8 @@ import com.firebase.ui.auth.util.EmailLinkParser.LinkParameters.SESSION_IDENTIFI * Used in email link sign-in flows to pass state between devices. * * The incoming URL comes from the consumer's [com.google.firebase.auth.ActionCodeSettings], so it - * may already carry a query string and/or a fragment. Appended parameters join an existing query - * with `&` and are always placed before the fragment. + * may already carry a query string and/or a fragment. Appended parameters join an existing + * query rather than starting a second one, and are always placed before the fragment. */ @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) class ContinueUrlBuilder(url: String) { @@ -80,10 +80,12 @@ class ContinueUrlBuilder(url: String) { // No params added, so the URL is handed back untouched. if (params.isEmpty()) return baseUrl + fragment + val queryStart = baseUrl.indexOf('?') val separator = when { - !baseUrl.contains('?') -> "?" - // The query is already open (`...?` or `...&`), so no separator is needed. - baseUrl.endsWith('?') || baseUrl.endsWith('&') -> "" + queryStart == -1 -> "?" + // The query is open, so no separator is needed. Only the first `?` marks the + // query; a later one is a literal inside a value and leaves it open to append. + queryStart == baseUrl.length - 1 || baseUrl.endsWith('&') -> "" else -> "&" } return baseUrl + separator + params + fragment diff --git a/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt b/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt index bc2d62844..1cbde2eab 100644 --- a/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt @@ -159,7 +159,7 @@ class ContinueUrlBuilderTest { } @Test - fun `path encoded url is untouched`() { + fun `multi segment path is untouched`() { val url = ContinueUrlBuilder("https://example.com/finish/fullcustomization") .appendSessionId("sid123") .build() @@ -182,6 +182,61 @@ class ContinueUrlBuilderTest { ).isEqualTo("https://example.com/finish?ui_sd=0") } + @Test + fun `query ending in an unencoded question mark still keeps every param`() { + // An unencoded '?' is legal inside a query (RFC 3986), so a trailing one does + // not mean the query is still open. + val input = "https://example.com/finish?next=/search?" + val url = ContinueUrlBuilder(input) + .appendSessionId("sid123") + .appendForceSameDeviceBit(true) + .build() + + val uri = url.toUri() + assertThat(uri.getQueryParameter("ui_sid")).isEqualTo("sid123") + assertThat(uri.getQueryParameter("ui_sd")).isEqualTo("1") + assertThat(uri.getQueryParameter("next")).isEqualTo("/search?") + } + + @Test + fun `url ending in a bare double question mark keeps every param`() { + val url = ContinueUrlBuilder("https://example.com/finish??") + .appendSessionId("sid123") + .build() + + assertThat(url.toUri().getQueryParameter("ui_sid")).isEqualTo("sid123") + } + + @Test + fun `fragment before a query keeps the params in the query`() { + // Everything after the first '#' is the fragment, query-looking or not. + val url = ContinueUrlBuilder("https://example.com/finish#section?x=1") + .appendSessionId("sid123") + .build() + + val uri = url.toUri() + assertThat(uri.getQueryParameter("ui_sid")).isEqualTo("sid123") + assertThat(uri.fragment).isEqualTo("section?x=1") + } + + @Test + fun `percent encoded url in a query value survives untouched`() { + val input = "https://example.com/finish?redirect=https%3A%2F%2Ffoo.com%2Fa%3Fb%3Dc" + val url = ContinueUrlBuilder(input).appendSessionId("sid123").build() + + val uri = url.toUri() + assertThat(uri.getQueryParameter("redirect")).isEqualTo("https://foo.com/a?b=c") + assertThat(uri.getQueryParameter("ui_sid")).isEqualTo("sid123") + } + + @Test + fun `the consumer's url is preserved verbatim ahead of the appended params`() { + val input = "https://example.com/finish?demo=full&lang=en" + val url = ContinueUrlBuilder(input).appendSessionId("sid123").build() + + assertThat(url).startsWith(input) + } + @Test(expected = IllegalArgumentException::class) fun `blank url is rejected`() { ContinueUrlBuilder(" ") diff --git a/auth/src/test/java/com/firebase/ui/auth/util/EmailLinkParserTest.kt b/auth/src/test/java/com/firebase/ui/auth/util/EmailLinkParserTest.kt index 76c3a9d7f..ee831f5d0 100644 --- a/auth/src/test/java/com/firebase/ui/auth/util/EmailLinkParserTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/util/EmailLinkParserTest.kt @@ -115,6 +115,35 @@ class EmailLinkParserTest { assertThat(parser.forceSameDeviceBit).isFalse() } + @Test + fun `parses the parameters out of a continue url nested behind a dynamic link`() { + val continueUrl = ContinueUrlBuilder("https://example.com/finish?demo=fullcustomization") + .appendSessionId("sid123") + .appendAnonymousUserId("auid456") + .build() + + val actionLink = "https://example.firebaseapp.com/__/auth/action".toUri() + .buildUpon() + .appendQueryParameter("mode", "signIn") + .appendQueryParameter("oobCode", "oob789") + .appendQueryParameter("continueUrl", continueUrl) + .build() + .toString() + + // Exercises parseUri's `link=` branch, not just `continueUrl=`. + val dynamicLink = "https://example.page.link/x".toUri() + .buildUpon() + .appendQueryParameter("link", actionLink) + .build() + .toString() + + val parser = EmailLinkParser(dynamicLink) + + assertThat(parser.oobCode).isEqualTo("oob789") + assertThat(parser.sessionId).isEqualTo("sid123") + assertThat(parser.anonymousUserId).isEqualTo("auid456") + } + @Test(expected = IllegalArgumentException::class) fun `blank link is rejected`() { EmailLinkParser(" ") From 2750351462bee3c0f9d7ab99b0bfd621d7494daf Mon Sep 17 00:00:00 2001 From: Ademola Fadumo <48495111+demolaf@users.noreply.github.com> Date: Wed, 9 Sep 2026 13:20:10 +0100 Subject: [PATCH 3/4] fix(auth): percent-encode continue URL params by building through Uri --- .../ui/auth/util/ContinueUrlBuilder.kt | 41 +++++-------------- .../ui/auth/util/ContinueUrlBuilderTest.kt | 34 ++++++++++++--- 2 files changed, 38 insertions(+), 37 deletions(-) diff --git a/auth/src/main/java/com/firebase/ui/auth/util/ContinueUrlBuilder.kt b/auth/src/main/java/com/firebase/ui/auth/util/ContinueUrlBuilder.kt index 517278af8..427cef241 100644 --- a/auth/src/main/java/com/firebase/ui/auth/util/ContinueUrlBuilder.kt +++ b/auth/src/main/java/com/firebase/ui/auth/util/ContinueUrlBuilder.kt @@ -13,7 +13,9 @@ */ package com.firebase.ui.auth.util +import android.net.Uri import androidx.annotation.RestrictTo +import androidx.core.net.toUri import com.firebase.ui.auth.util.EmailLinkParser.LinkParameters.ANONYMOUS_USER_ID_IDENTIFIER import com.firebase.ui.auth.util.EmailLinkParser.LinkParameters.FORCE_SAME_DEVICE_IDENTIFIER import com.firebase.ui.auth.util.EmailLinkParser.LinkParameters.PROVIDER_ID_IDENTIFIER @@ -24,26 +26,18 @@ import com.firebase.ui.auth.util.EmailLinkParser.LinkParameters.SESSION_IDENTIFI * Used in email link sign-in flows to pass state between devices. * * The incoming URL comes from the consumer's [com.google.firebase.auth.ActionCodeSettings], so it - * may already carry a query string and/or a fragment. Appended parameters join an existing - * query rather than starting a second one, and are always placed before the fragment. + * may already carry a query string and/or a fragment. Parameters are appended through [Uri], the + * same parser [EmailLinkParser] reads them back with, which places them in the query whatever the + * URL's shape and percent-encodes their values. */ @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) class ContinueUrlBuilder(url: String) { - /** The incoming URL up to, but not including, the fragment. */ - private val baseUrl: String - - /** The fragment including its leading `#`, or empty when the URL has none. */ - private val fragment: String - - private val params = StringBuilder() + private var continueUrl: Uri init { require(url.isNotBlank()) { "URL cannot be empty" } - - val fragmentStart = url.indexOf('#') - baseUrl = if (fragmentStart == -1) url else url.substring(0, fragmentStart) - fragment = if (fragmentStart == -1) "" else url.substring(fragmentStart) + continueUrl = url.toUri() } fun appendSessionId(sessionId: String): ContinueUrlBuilder { @@ -70,24 +64,9 @@ class ContinueUrlBuilder(url: String) { private fun addQueryParam(key: String, value: String) { if (value.isBlank()) return - if (params.isNotEmpty()) { - params.append("&") - } - params.append("$key=$value") + continueUrl = continueUrl.buildUpon().appendQueryParameter(key, value).build() } - fun build(): String { - // No params added, so the URL is handed back untouched. - if (params.isEmpty()) return baseUrl + fragment - - val queryStart = baseUrl.indexOf('?') - val separator = when { - queryStart == -1 -> "?" - // The query is open, so no separator is needed. Only the first `?` marks the - // query; a later one is a literal inside a value and leaves it open to append. - queryStart == baseUrl.length - 1 || baseUrl.endsWith('&') -> "" - else -> "&" - } - return baseUrl + separator + params + fragment - } + // Untouched when nothing was appended: Uri hands back the string it was parsed from. + fun build(): String = continueUrl.toString() } diff --git a/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt b/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt index 1cbde2eab..baa7a0c59 100644 --- a/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt @@ -83,16 +83,38 @@ class ContinueUrlBuilderTest { } @Test - fun `url ending in an open query marker is not given a second separator`() { + fun `url ending in an open query marker keeps every param retrievable`() { assertThat( ContinueUrlBuilder("https://example.com/finish?").appendSessionId("sid123").build() ).isEqualTo("https://example.com/finish?ui_sid=sid123") - assertThat( - ContinueUrlBuilder("https://example.com/finish?demo=full&") - .appendSessionId("sid123") - .build() - ).isEqualTo("https://example.com/finish?demo=full&ui_sid=sid123") + // A trailing `&` is an empty param, which Uri keeps rather than folding away. Cosmetic + // only: both the consumer's params and ours still parse out. + val url = ContinueUrlBuilder("https://example.com/finish?demo=full&") + .appendSessionId("sid123") + .build() + + assertThat(url).isEqualTo("https://example.com/finish?demo=full&&ui_sid=sid123") + + val uri = url.toUri() + assertThat(uri.getQueryParameter("demo")).isEqualTo("full") + assertThat(uri.getQueryParameter("ui_sid")).isEqualTo("sid123") + } + + @Test + fun `param values are percent encoded`() { + val hostile = "a&b=c d#e" + val url = ContinueUrlBuilder("https://example.com/finish?demo=full") + .appendAnonymousUserId(hostile) + .build() + + // Interpolated raw, this value would split the query and open a fragment. + assertThat(url).doesNotContain(hostile) + + val uri = url.toUri() + assertThat(uri.getQueryParameter("ui_auid")).isEqualTo(hostile) + assertThat(uri.getQueryParameter("demo")).isEqualTo("full") + assertThat(uri.fragment).isNull() } @Test From 48a99f205d6376d2a160dedac39423127fc5d4fa Mon Sep 17 00:00:00 2001 From: Ademola Fadumo <48495111+demolaf@users.noreply.github.com> Date: Wed, 9 Sep 2026 13:31:54 +0100 Subject: [PATCH 4/4] test(auth): pin the encoded form of a continue URL param value --- .../java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt b/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt index baa7a0c59..6b1b57813 100644 --- a/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/util/ContinueUrlBuilderTest.kt @@ -111,6 +111,10 @@ class ContinueUrlBuilderTest { // Interpolated raw, this value would split the query and open a fragment. assertThat(url).doesNotContain(hostile) + // Pin the encoded form on the wire, not just that it survives a round trip. + assertThat(url) + .isEqualTo("https://example.com/finish?demo=full&ui_auid=a%26b%3Dc%20d%23e") + val uri = url.toUri() assertThat(uri.getQueryParameter("ui_auid")).isEqualTo(hostile) assertThat(uri.getQueryParameter("demo")).isEqualTo("full")