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

Disk cached image with non-ASCII headers can't be updated after 304 Not Modified #1838

Closed
fingertricks opened this issue Aug 8, 2023 · 0 comments · Fixed by #1839
Closed

Comments

@fingertricks
Copy link
Contributor

fingertricks commented Aug 8, 2023

Describe the bug
When a disk cached image includes header values that are non-ASCII, updating the disk cache after a 304 Not Modified fails with an exception which causes image loading to fail.

I also have a fix for this, which I'll submit shortly, but here's the details of the issue below.

To Reproduce
Example image: https://just-eat-prod-eu-res.cloudinary.com/image/upload/c_fill,g_center,h_400,w_500/v1/experiments/projecticing/es/cuisine-icons/alimentaci%C3%B3n.webp

Returns headers which are correctly persisted to the disk cache as follows:

1691404585044
1691404585064
1
16
content-disposition: inline; filename="alimentación.webp"
content-type: image/webp
etag: "232087ad43c9820865f16ce05438fa59"
last-modified: Wed, 23 Nov 2022 14:43:23 GMT
x-request-id: f844e7f14d102b27ede9c531c65c51ba
date: Mon, 07 Aug 2023 10:36:27 GMT
strict-transport-security: max-age=604800
cache-control: public, no-transform, max-age=86400
server-timing: cld-fastly;mitm=p;dur=1;cpu=0;start=2023-08-07T10:36:27.159Z;desc=hit,rtt;dur=0
server: Cloudinary
timing-allow-origin: *
access-control-allow-origin: *
accept-ranges: bytes
x-content-type-options: nosniff
access-control-expose-headers: Content-Length,Content-Disposition,ETag,Server-Timing,X-Content-Type-Options
content-length: 17388

When the max-age expires 1 day later (or the disk cached max-age is modified to say 60s), a new server request is sent which returns a 304. However Coil fails as follows:

2023-08-07 16:59:00.454 22744-23063 okhttp.OkHttpClient     com.***             I  --> GET https://just-eat-prod-eu-res.cloudinary.com/image/upload/c_fill,g_center,h_400,w_500/v1/experiments/projecticing/es/cuisine-icons/alimentaci%C3%B3n.webp
2023-08-07 16:59:00.706 22744-23063 okhttp.OkHttpClient     com.***            I  <-- 304 https://just-eat-prod-eu-res.cloudinary.com/image/upload/c_fill,g_center,h_400,w_500/v1/experiments/projecticing/es/cuisine-icons/alimentaci%C3%B3n.webp (251ms)
2023-08-07 16:59:00.736 22744-22744 RealImageLoader         com.***             I  🚨 Failed - https://just-eat-prod-eu-res.cloudinary.com/image/upload/c_fill,g_center,h_400,w_500/v1/experiments/projecticing/es/cuisine-icons/alimentaci%C3%B3n.webp - java.lang.IllegalArgumentException: Unexpected char 0x301 at 29 in content-disposition value: inline; filename="alimentación.webp"

Logs/Screenshots
Stack as follows points to issue within CacheStrategy.combineHeaders():

cause = {IllegalArgumentException@44000} "java.lang.IllegalArgumentException: Unexpected char 0x301 at 29 in content-disposition value: inline; filename="alimentación.webp""
detailMessage = "Unexpected char 0x301 at 29 in content-disposition value: inline; filename="alimentación.webp""
stackTrace = {StackTraceElement[15]@44021} 
 0 = {StackTraceElement@44024} "okhttp3.Headers$Companion.checkValue(Headers.kt:450)"
 1 = {StackTraceElement@44025} "okhttp3.Headers$Companion.access$checkValue(Headers.kt:362)"
 2 = {StackTraceElement@44026} "okhttp3.Headers$Builder.add(Headers.kt:261)"
 3 = {StackTraceElement@44027} "coil.network.CacheStrategy$Companion.combineHeaders(CacheStrategy.kt:245)"
 4 = {StackTraceElement@44028} "coil.fetch.HttpUriFetcher.writeToDiskCache(HttpUriFetcher.kt:159)"
 5 = {StackTraceElement@44029} "coil.fetch.HttpUriFetcher.fetch(HttpUriFetcher.kt:80)"
 6 = {StackTraceElement@44030} "coil.fetch.HttpUriFetcher$fetch$1.invokeSuspend(Unknown Source:14)"
 7 = {StackTraceElement@44031} "kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)"
 8 = {StackTraceElement@44032} "kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)"
 9 = {StackTraceElement@44033} "kotlinx.coroutines.internal.LimitedDispatcher.run(LimitedDispatcher.kt:42)"

Version
2.3.0, but have also reproduced on latest main.

fingertricks pushed a commit to fingertricks/coil that referenced this issue Aug 8, 2023
fingertricks pushed a commit to fingertricks/coil that referenced this issue Aug 8, 2023
…haracters coil-kt#1838

Allows disk cache headers to be updated after a 304 Not Modified, as detailed in coil-kt#1838
colinrtwhite pushed a commit that referenced this issue Aug 10, 2023
…ude non-ASCII characters (#1839)

* Add failing test for #1838

* Fixes loading from disk cache when cached headers contain non-ASCII characters #1838

Allows disk cache headers to be updated after a 304 Not Modified, as detailed in #1838

* Fix deprecated diskCache method call

---------

Co-authored-by: Dave Jones <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant