Kamel-Media / Kamel

Kotlin asynchronous media loading and caching library for Compose.
Apache License 2.0
592 stars 23 forks source link

Use URLBuilder instead of Url in mappers and HttpFetcher #78

Open ayanyev opened 7 months ago

ayanyev commented 7 months ago

There is an issue when full resource url is split between httpClient config and asyncPainterResource call:

defaultRequest {
     url("https://kamel.media")
}
asyncPainterResource("/image.jpg")

If mapper and fetcher use Url respectively as output and input data, the resulting resource url will look like: https://localhost:80/image.jpg

This happens due to DefaultRequest.mergeUrls(...) implementation.

DefaultRequestConfigTest is added.

luca992 commented 7 months ago

Does fun URLBuilder.takeFrom(url: Url): URLBuilder not work? (instead of URLBuilder.takeFrom(url: URLBuilder))

Screenshot 2023-12-06 at 6 08 13 PM
ayanyev commented 7 months ago

The problem is when mapper maps to Url, localhost:80 added to /image.jpg with applyOrigin().

    public fun build(): Url {
        applyOrigin()
        return Url(
            protocol = protocol,
            host = host,

When it comes to DefaultRequest.mergeUrls(), execution is stopped at the very beginning because host is not empty.

        private fun mergeUrls(baseUrl: Url, requestUrl: URLBuilder) {
            if (requestUrl.protocol == URLProtocol.HTTP) {
                requestUrl.protocol = baseUrl.protocol
            }
            if (requestUrl.host.isNotEmpty()) return

It ends up fetching image from localhost.

eskatos commented 5 months ago

This might be related to the upstream ktor issues