fix: url encode query prameters#3451
Conversation
| } | ||
|
|
||
| private fun String.encodeQueryParameterComponent(): String = | ||
| URLEncoder.encode(this, StandardCharsets.UTF_8.name()).replace("+", "%20") |
There was a problem hiding this comment.
Just curious, why was the manual .replace("+", "%20") added? A + is allowed in URLs afaik.
There was a problem hiding this comment.
This was a GPT 5.5 change. The reasoning made sense to me. It is:
URLEncoder is for application/x-www-form-urlencoded, where spaces become +. In a URI query string, + is legal but not universally equivalent to a space unless the receiver applies form decoding. %20 is the safer URI encoding and avoids ambiguity.
This replacement is safe for literal plus signs too: URLEncoder turns + into %2B before our .replace("+", "%20"), so:
"A+B C" -> "A%2BB%20C"
So the current .replace("+", "%20") is intentional.
There was a problem hiding this comment.
but @tonidero had good feedback and I changed this
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3451 +/- ##
=======================================
Coverage 79.48% 79.48%
=======================================
Files 363 363
Lines 14562 14562
Branches 1979 1979
=======================================
Hits 11574 11574
Misses 2190 2190
Partials 798 798 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tonidero
left a comment
There was a problem hiding this comment.
Just some thoughts... But not blockers I would say. Thanks for fixing this!
| return URI("$this$separator$name=$value") | ||
| val encodedParameter = "${name.encodeQueryParameterComponent()}=${value.encodeQueryParameterComponent()}" | ||
| val uriString = toString() | ||
| val fragmentIndex = uriString.indexOf('#') |
There was a problem hiding this comment.
Hmm I'm thinking, do we support fragment in these urls? Just wondering if this is needed at all... If we have it in iOS though, I think it might make sense to support it. Otherwise, might be better to reduce complexity and leave it out? NABD.
There was a problem hiding this comment.
I believe this to be how URLComponents handles things on iOS actually
| return URL("$this$separator$name=$value") | ||
| } | ||
| internal fun URL.appendQueryParameter(name: String, value: String): URL = | ||
| toURI().appendQueryParameter(name, value).toURL() |
There was a problem hiding this comment.
Good to avoid repeating the logic 👍 Thanks!
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a4a604b. Configure here.
| assertEquals(updatedUri.toString(), "https://example.com/path?key=value") | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Test missing @RunWith for Android framework dependency
Medium Severity
URIExtensionsTest now exercises appendQueryParameter which calls android.net.Uri.encode(), but the test class lacks @RunWith(AndroidJUnit4::class). Every other test in this project that uses android.net.Uri includes this annotation to activate Robolectric. Without it, Uri.encode() may throw a stub RuntimeException depending on the test runner configuration, causing all tests in this file to fail.
Reviewed by Cursor Bugbot for commit a4a604b. Configure here.
**This is an automatic release.** ## RevenueCat SDK ### 🐞 Bugfixes * fix: url encode query prameters (RevenueCat#3451) via Jacob Rakidzich (@JZDesign) ## RevenueCatUI SDK ### 🐞 Bugfixes * Fix: dismiss was called before onPurchaseComplete callback invocation (RevenueCat#3353) via Jacob Rakidzich (@JZDesign) * Propagate default package across workflow steps (RevenueCat#3431) via Cesar de la Vega (@vegaro) ### Paywallv2 #### ✨ New Features * feat: Allow disabling of automatic font scaling (RevenueCat#3438) via Jacob Rakidzich (@JZDesign) ### 🔄 Other Changes * Extract `PaywallComponentsImagePreDownloader` (RevenueCat#3448) via Cesar de la Vega (@vegaro) * Simplify `WorkflowTransitionState` with explicit from/to step fields (RevenueCat#3441) via Cesar de la Vega (@vegaro) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk release bookkeeping: primarily flips version strings from `10.5.0-SNAPSHOT` to `10.5.0` and updates docs/changelogs, with no functional code changes beyond the reported version constant. > > **Overview** > Cuts the `10.5.0` release by switching the project from `10.5.0-SNAPSHOT` to `10.5.0` across build metadata (`.version`, `gradle.properties`, sample/test app `libs.versions.toml`, and `Config.frameworkVersion`). > > Updates release artifacts and documentation pointers: CircleCI docs deploy now syncs the `10.5.0` docs folder to S3, `docs/index.html` redirects to `10.5.0`, and changelogs are rolled forward with the `10.5.0` entries in `CHANGELOG.md`/`CHANGELOG.latest.md`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 48537d6. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Checklist
purchases-iosand hybridsMotivation
URL query parameters with spaces caused crashes
Description
url encode query parameters to prevent crashes
Note
Medium Risk
Changes how web checkout/custom URLs are constructed by encoding query components and inserting parameters before fragments, which could subtly affect downstream URL parsing and expectations in integrations.
Overview
Fixes URL construction when appending query params by URL-encoding parameter names/values and ensuring new params are inserted before any
#fragmentinURI.appendQueryParameter.URL.appendQueryParameteris simplified to delegate through theURIimplementation, and tests are updated/added to assert encoding behavior (including whitespace) and fragment handling; web-checkout URL expectations now reflect encoded values (e.g.%24rc_monthly).Reviewed by Cursor Bugbot for commit a4a604b. Bugbot is set up for automated code reviews on this repo. Configure here.