Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix reading responses with non-ascii headers from the disk cache. #1468

Merged
merged 1 commit into from Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -47,6 +47,7 @@ class FileUriMapperTest {
assertNull(mapper.map(uri, Options(context)))
}

/** Regression test: https://github.com/coil-kt/coil/issues/1344 */
@Test
fun parsesPoundCharacterCorrectly() {
val path = "/sdcard/fi#le.jpg"
Expand Down
3 changes: 2 additions & 1 deletion coil-base/src/main/java/coil/network/CacheResponse.kt
@@ -1,5 +1,6 @@
package coil.network

import coil.util.addUnsafeNonAscii
import okhttp3.CacheControl
import okhttp3.Headers
import okhttp3.MediaType.Companion.toMediaTypeOrNull
Expand All @@ -25,7 +26,7 @@ internal class CacheResponse {
val responseHeadersLineCount = source.readUtf8LineStrict().toInt()
val responseHeaders = Headers.Builder()
for (i in 0 until responseHeadersLineCount) {
responseHeaders.add(source.readUtf8LineStrict())
responseHeaders.addUnsafeNonAscii(source.readUtf8LineStrict())
}
this.responseHeaders = responseHeaders.build()
}
Expand Down
7 changes: 7 additions & 0 deletions coil-base/src/main/java/coil/util/Utils.kt
Expand Up @@ -232,6 +232,13 @@ internal fun isAssetUri(uri: Uri): Boolean {
return uri.scheme == SCHEME_FILE && uri.firstPathSegment == ASSET_FILE_PATH_ROOT
}

/** Modified from [Headers.Builder.add] */
internal fun Headers.Builder.addUnsafeNonAscii(line: String) = apply {
val index = line.indexOf(':')
require(index != -1) { "Unexpected header: $line" }
addUnsafeNonAscii(line.substring(0, index).trim(), line.substring(index + 1))
}

private const val STANDARD_MEMORY_MULTIPLIER = 0.2
private const val LOW_MEMORY_MULTIPLIER = 0.15

Expand Down
39 changes: 26 additions & 13 deletions coil-base/src/test/java/coil/network/CacheResponseTest.kt
@@ -1,13 +1,11 @@
package coil.network

import android.content.Context
import androidx.test.core.app.ApplicationProvider
import coil.util.createMockWebServer
import okhttp3.Headers
import okhttp3.OkHttpClient
import okhttp3.Request
import okhttp3.mockwebserver.MockWebServer
import okhttp3.mockwebserver.MockResponse
import okio.Buffer
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
Expand All @@ -16,17 +14,9 @@ import kotlin.test.assertEquals
@RunWith(RobolectricTestRunner::class)
class CacheResponseTest {

private lateinit var context: Context
private lateinit var server: MockWebServer

@Before
fun before() {
context = ApplicationProvider.getApplicationContext()
server = createMockWebServer(IMAGE)
}

@Test
fun `can serialize and deserialize cache response`() {
val server = createMockWebServer(IMAGE)
val url = server.url(IMAGE)
val request = Request.Builder().url(url).build()
val response = OkHttpClient().newCall(request).execute()
Expand All @@ -42,6 +32,29 @@ class CacheResponseTest {
assertEquals(expected.responseHeaders, actual.responseHeaders)
}

/** Regression test: https://github.com/coil-kt/coil/issues/1467 */
@Test
fun `can deserialize cache response with non ascii characters in headers`() {
val headerName = "name"
val headerValue = "微信图片"
val server = createMockWebServer()
val responseHeaders = Headers.Builder()
.addUnsafeNonAscii(headerName, headerValue)
.build()
server.enqueue(MockResponse().setHeaders(responseHeaders).setBody(Buffer()))
val url = server.url("微信图片.jpg")
val request = Request.Builder().url(url).build()
val response = OkHttpClient().newCall(request).execute()
val expected = CacheResponse(response)

val buffer = Buffer()
expected.writeTo(buffer)
val actual = CacheResponse(buffer)

assertEquals(headerValue, actual.responseHeaders[headerName])
assertEquals(expected.responseHeaders, actual.responseHeaders)
}

companion object {
private const val IMAGE = "normal.jpg"
}
Expand Down