Fix crash when URLDecodeException is thrown from parseUrl

[edenman]

URLBuilder.takeFrom has a catch (Throwable) but the build() method does a parameters.build() which can throw a URLDecodeException (and that is not a URLParserException so the parseUrl catch block wouldn't catch it

Subsystem
Common

Motivation
parseUrl should not crash my app

Solution
Catch the exception

[coderabbitai]

Walkthrough

parseUrl in URLUtils.kt now catches all Exceptions (previously only URLParserException), making it return null for any parsing error. A unit test was added to assert parseUrl returns null for a URL with an encoded embedded URL in a query parameter.

Changes

Cohort / File(s)	Summary
Implementation change ktor-http/common/src/io/ktor/http/URLUtils.kt	Changed parseUrl exception handling to catch all Exception types instead of only URLParserException, causing the function to return null on any parsing exception.
Test addition ktor-http/common/test/io/ktor/tests/http/UrlTest.kt	Added an assertion in testParseUrl that parseUrl("https://example.com?url=https%3A%2F%2Fwww.google.com%2") returns null.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify intent for broadening from a specific parser exception to all Exception and assess whether some exceptions should propagate.
• Check for other callers that rely on specific exceptions or expect thrown errors.
• Ensure the new test is robust and consider edge cases (different encodings, malformed inputs).

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing a crash caused by URLDecodeException in parseUrl by expanding exception handling.
Description check	✅ Passed	The description follows the required template with all sections present: Subsystem, Motivation, and Solution are clearly provided with specific technical context.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

ktor-http/common/test/io/ktor/tests/http/UrlTest.kt (1)

355-364: New malformed-URL regression case looks correct

The new assertion for parseUrl("https://example.com?url=https%3A%2F%2Fwww.google.com%2") returning null is consistent with the documented behavior of parseUrl (“returns null otherwise”) and with the updated implementation that maps decoding failures to null. This is a good targeted regression to cover the URLDecodeException path.

If you ever touch this block again, consider switching all three assertEquals(null, parseUrl(...)) calls in this test to assertNull(...) for readability, but that’s purely optional and not required for this PR.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eaddf43 and 8c126df.

📒 Files selected for processing (2)

• ktor-http/common/src/io/ktor/http/URLUtils.kt (1 hunks)
• ktor-http/common/test/io/ktor/tests/http/UrlTest.kt (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.kt

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports for io.ktor.* packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with @InternalAPI annotation
Run ./gradlew lintKotlin and fix all linting issues before giving control back to the user
Use ./gradlew formatKotlin to automatically fix formatting issues
Run ./gradlew updateLegacyAbi after making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the /api/ directories
Validate ABI with ./gradlew checkLegacyAbi and update with ./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions

Files:

• ktor-http/common/test/io/ktor/tests/http/UrlTest.kt
• ktor-http/common/src/io/ktor/http/URLUtils.kt

**/test/**/*.kt

📄 CodeRabbit inference engine (CLAUDE.md)

Follow test naming pattern: DescribeWhatIsBeingTested

Files:

• ktor-http/common/test/io/ktor/tests/http/UrlTest.kt

🧠 Learnings (2)

📚 Learning: 2025-11-25T09:38:19.393Z

Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Error handling follows Kotlin conventions with specific Ktor exceptions

Applied to files:

• ktor-http/common/src/io/ktor/http/URLUtils.kt

📚 Learning: 2025-09-30T07:52:14.769Z

Learnt from: yschimke
Repo: ktorio/ktor PR: 4013
File: ktor-client/ktor-client-android/jvm/src/io/ktor/client/engine/android/Android14URLConnectionFactory.kt:24-26
Timestamp: 2025-09-30T07:52:14.769Z
Learning: In the Ktor Android HTTP client engine (ktor-client-android), prefer using `URI.create(urlString).toURL()` over the `URL(urlString)` constructor when opening connections with Android's HttpEngine, as it avoids deprecated APIs and the different exception behavior (IllegalArgumentException vs MalformedURLException) is acceptable.

Applied to files:

• ktor-http/common/src/io/ktor/http/URLUtils.kt

🧬 Code graph analysis (1)

ktor-http/common/test/io/ktor/tests/http/UrlTest.kt (1)

ktor-http/common/src/io/ktor/http/URLUtils.kt (1)

• parseUrl (41-47)

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

ktor-http/common/src/io/ktor/http/URLUtils.kt (1)

41-46: parseUrl now safely handles decode failures without crashing callers

Catching Exception here makes parseUrl robust against URLDecodeException and other parsing-time failures, keeping its contract of returning null on invalid/unsafely encoded URLs while avoiding the previous crash. This aligns with the intent of a “safe parse” helper and avoids the earlier problem of catching Throwable.

If you want to be stricter about not masking unexpected bugs, you could optionally narrow this to Ktor’s known parsing/decode exceptions, e.g. URLParserException and URLDecodeException, instead of all Exception, but the current change is already an improvement and consistent with the API docs. Based on learnings, error handling should generally rely on Ktor-specific exceptions where practical.

ktor-http/common/test/io/ktor/tests/http/UrlTest.kt (1)

354-364: New parseUrl test case correctly validates behavior on invalid percent-encoding

The additional assertion covers the case where query decoding fails (truncated %2), and verifies that parseUrl now returns null instead of throwing, matching the updated error-handling behavior. This is a good regression test for the crash described in the PR.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8c126df and c9d243f.

📒 Files selected for processing (2)

• ktor-http/common/src/io/ktor/http/URLUtils.kt (1 hunks)
• ktor-http/common/test/io/ktor/tests/http/UrlTest.kt (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.kt

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports for io.ktor.* packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with @InternalAPI annotation
Run ./gradlew lintKotlin and fix all linting issues before giving control back to the user
Use ./gradlew formatKotlin to automatically fix formatting issues
Run ./gradlew updateLegacyAbi after making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the /api/ directories
Validate ABI with ./gradlew checkLegacyAbi and update with ./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions

Files:

• ktor-http/common/src/io/ktor/http/URLUtils.kt
• ktor-http/common/test/io/ktor/tests/http/UrlTest.kt

**/test/**/*.kt

📄 CodeRabbit inference engine (CLAUDE.md)

Follow test naming pattern: DescribeWhatIsBeingTested

Files:

• ktor-http/common/test/io/ktor/tests/http/UrlTest.kt

🧠 Learnings (5)

📚 Learning: 2025-11-25T09:38:19.393Z

Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Error handling follows Kotlin conventions with specific Ktor exceptions

Applied to files:

• ktor-http/common/src/io/ktor/http/URLUtils.kt

📚 Learning: 2025-09-30T07:52:14.769Z

Learnt from: yschimke
Repo: ktorio/ktor PR: 4013
File: ktor-client/ktor-client-android/jvm/src/io/ktor/client/engine/android/Android14URLConnectionFactory.kt:24-26
Timestamp: 2025-09-30T07:52:14.769Z
Learning: In the Ktor Android HTTP client engine (ktor-client-android), prefer using `URI.create(urlString).toURL()` over the `URL(urlString)` constructor when opening connections with Android's HttpEngine, as it avoids deprecated APIs and the different exception behavior (IllegalArgumentException vs MalformedURLException) is acceptable.

Applied to files:

• ktor-http/common/src/io/ktor/http/URLUtils.kt

📚 Learning: 2025-11-25T09:38:19.393Z

Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions

Applied to files:

• ktor-http/common/src/io/ktor/http/URLUtils.kt

📚 Learning: 2025-06-23T12:49:56.883Z

Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: Error handling should follow Kotlin conventions and use specific Ktor exceptions.

Applied to files:

• ktor-http/common/src/io/ktor/http/URLUtils.kt

📚 Learning: 2025-06-09T07:09:43.170Z

Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DeferredDependencyMap.kt:16-24
Timestamp: 2025-06-09T07:09:43.170Z
Learning: In the `tryGetCompleted` extension function on `Deferred<T>` in ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DeferredDependencyMap.kt, exceptions should be thrown if the Deferred completed with an error. The "try" in the function name refers to trying to get a completed value non-blockingly, not to exception handling. The function should return null if not completed, the value if completed successfully, or throw the exception if completed with an error.

Applied to files:

• ktor-http/common/src/io/ktor/http/URLUtils.kt

🧬 Code graph analysis (1)

ktor-http/common/test/io/ktor/tests/http/UrlTest.kt (1)

ktor-http/common/src/io/ktor/http/URLUtils.kt (1)

• parseUrl (41-47)

[edenman]

@bjhham thanks for the approval! The test failures don't seem related, how should I proceed?

[bjhham]

Don't worry about the tests, we've got some flakiness problems atm. I'll just rerun them until they pass then merge 👍