Duplicate network request for images with chunked response

[Fayth]

Describe the bug Scenario:

• image caching disabled in coil
• requesting image via url (HttpUrlFetcher)
• server returns valid chunked response (doesn't contain Content-Length header)

results in two requests to given url (despite the first one being successful)

To Reproduce

val request = ImageRequest.Builder(context)
            .data("https://www.httpwatch.com/httpgallery/chunked/chunkedimage.aspx")
            .memoryCachePolicy(CachePolicy.DISABLED)
            .diskCachePolicy(CachePolicy.DISABLED)
            .build()
context.imageLoader.execute(request)

Logs/Screenshots The cause seems to be the verification in https://github.com/coil-kt/coil/blob/main/coil-base/src/main/java/coil/fetch/HttpUriFetcher.kt#L95 which relies on explicit content length being provided in response. In the scenario above the length isn't known (okhttp reports it as -1)

Version 2.2.2

It might be enough to allow contentLength() == -1 in the check mentioned above (it seems to fix the problem in my case) or possibly also peeking the response body buffer to make sure it's not empty

[colinrtwhite]

@Fayth Does OkHttp have the same behaviour for that URL? Coil's HttpUriFetcher code is based off of OkHttp's behaviour and tries to adhere pretty closely to it.

[Fayth]

I haven't used "raw" okhttp much, but from what I tried, there's only one GET request when using this sample request:

val request: Request = Request.Builder()
            .url(url)
            .cacheControl(CacheControl.FORCE_NETWORK)
            .build()
OkHttpClient().newCall(request).execute().use { response -> return response.body?.string() }

Looking through okhttp source, I think the only place where it uses content length in higher-level logic is ensuring there's no body for 204/205 responses.

[ferinagy]

Hello, we just ran into this for 2.6.0. I tried to create a failing test in HttpUriFetcherTest:

@Test
fun `chunked response`() = runTestAsync {
    val url = server.url(IMAGE).toString()

    server.enqueueChunkedImage(IMAGE)
    server.enqueueChunkedImage(IMAGE)

    val fetcher = newFetcher(url, Options(context, diskCachePolicy = CachePolicy.DISABLED))
    fetcher.fetch()

    assertEquals(1, server.requestCount)
}

fun MockWebServer.enqueueChunkedImage(image: String) {
    val buffer = Buffer()
    val context = ApplicationProvider.getApplicationContext<Context>()
    context.assets.open(image).source().buffer().readAll(buffer)
    val response = MockResponse().setBody(buffer).removeHeader("Content-Length")
    enqueue(response)
}

I also tried it with current 3.0.0 on main and there it does not happen as the real size of the response is checked instead of the Content-length header.

https://github.com/coil-kt/coil/blob/e553267d51ff63aeb271fc12d7fbef43c19a99de/coil-network-core/src/commonMain/kotlin/coil3/network/NetworkFetcher.kt#L70-L71

However, as 3.0 is still in alpha, we'd prefer to have a fix in 2.x. I am open to creating a PR if there is a chance to get it merged.

[colinrtwhite]

This is fixed in 2.7.0.