square / okhttp

Square’s meticulous HTTP client for the JVM, Android, and GraalVM.
https://square.github.io/okhttp/
Apache License 2.0
45.72k stars 9.15k forks source link

Difference in URL handling between ktor and OkHttp #6498

Closed yschimke closed 3 years ago

yschimke commented 3 years ago

Logging here to confirm that we are wrong, or accept differences. Calling the following code is enough to get the error

val url: okhttp3.HttpUrl = ...
val client: io.ktor.client.HttpClient = ...
client.get<HttpResponse>(url.toString())
Caused by: io.ktor.http.URLParserException: Fail to parse url: https://oidc.engadget.com/login?dest=https%3A%2F%www.engadget.com
    at io.ktor.http.URLParserKt.takeFrom(URLParser.kt:17)
    at ee.baulsupp.crawl.FetchKt.fetchAndParsePage(fetch.kt:42)
    at ee.baulsupp.crawl.AppKt$main$2$2$1.invoke(app.kt:57)
    at ee.baulsupp.crawl.AppKt$main$2$2$1.invoke(app.kt)
    at ee.baulsupp.crawl.CrawlSession.fetchAndCrawl(CrawlSession.kt:43)
    at ee.baulsupp.crawl.CrawlSession$fetchAndCrawl$$inlined$map$lambda$1.invokeSuspend(CrawlSession.kt:48)
    Caused by: io.ktor.http.URLDecodeException: Wrong HEX escape: %ww, in dest=https%3A%2F%www.engadget.com, at 16
        at io.ktor.http.CodecsKt.decodeImpl(Codecs.kt:225)
        at io.ktor.http.CodecsKt.decodeScan(Codecs.kt:173)
        at io.ktor.http.CodecsKt.decodeURLQueryComponent(Codecs.kt:157)
        at io.ktor.http.CodecsKt.decodeURLQueryComponent$default(Codecs.kt:156)
        at io.ktor.http.QueryKt.appendParam(Query.kt:63)
        at io.ktor.http.QueryKt.parse(Query.kt:43)
        at io.ktor.http.QueryKt.parseQueryString(Query.kt:14)
        at io.ktor.http.QueryKt.parseQueryString$default(Query.kt:10)
        at io.ktor.http.URLParserKt.parseQuery(URLParser.kt:158)
        at io.ktor.http.URLParserKt.takeFromUnsafe(URLParser.kt:112)
        at io.ktor.http.URLParserKt.takeFrom(URLParser.kt:15)
        ... 11 more
yschimke commented 3 years ago

Happy to close, but logging the difference.

swankjesse commented 3 years ago

Firefox and Chrome can both navigate to it.

yschimke commented 3 years ago

I'll raise with ktor

swankjesse commented 3 years ago

If they’re interested in being consistent with browsers, they should probably use this test suite. https://github.com/square/okhttp/blob/master/okhttp/src/test/resources/web-platform-test-urltestdata.txt