square / okhttp

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

etag does not work #8543

Closed puresprout closed 2 months ago

puresprout commented 2 months ago

etag does not work in okhttp 4.12 version It also does not work in the latest alpha.14 version

In server-side implementations, etag works fine.

curl -i "http://localhost:8080/api-mock/hrc/fixed-accounts?count=1"

HTTP/1.1 200 
ETag: "05099b4b846c91908f7b65d1affda405d"
Content-Type: application/json
Content-Length: 191
Date: Tue, 24 Sep 2024 02:20:37 GMT

[ {
  "accountNumber" : "9999999990",
  "accountName" : "John Doe",
  "balance" : 1000000.0,
  "lastTransactionDate" : [ 2024, 8, 19, 10, 0 ],
  "description" : "Personal savings account"
} ]
curl -H "If-None-Match: \"05099b4b846c91908f7b65d1affda405d\"" -i "http://localhost:8080/api-mock/hrc/fixed-accounts?count=1" -v

# If you send an if-none-match header in the request header in curl, you can see that the server responds with a 304 Not Modified response.

Host: localhost:8080
User-Agent: curl/8.7.1
Accept: */*
If-None-Match: "05099b4b846c91908f7b65d1affda405d"

HTTP/1.1 304 
ETag: "05099b4b846c91908f7b65d1affda405d"
Date: Tue, 24 Sep 2024 02:20:59 GMT

In other words, the server implementation is completely problem-free.

But on Android

I have structured my app on Android like this:

    implementation("com.squareup.retrofit2:retrofit:2.11.0")
    implementation("com.squareup.retrofit2:converter-gson:2.10.0")
    implementation("com.squareup.okhttp3:logging-interceptor:4.12.0")

---
        val logging = HttpLoggingInterceptor().apply {
            level = HttpLoggingInterceptor.Level.BODY
        }

        val okHttpClient = OkHttpClient.Builder()
            .cache(cache)
            .addInterceptor(logging)
            .build()

        okHttpClient = builder
            .cache(Cache(context.getCacheDir(), 10 * 1024 * 1024))
            .build()

With the above settings, the actual cache file is created and has contents.

/data/data/com.example.mytestapplication.cache
eb...cabe.0
eb...cabe.1
journal

...cabe.0 file content

http://172.19.111.149:8080/api-mock/hrc/fixed-accounts?count=1
GET
0
HTTP/1.1 200 
8
Content-Type: application/json
Content-Length: 191
Keep-Alive: timeout=60
Connection: keep-alive
ETag: "05099b4b846c91908f7b65d1affda405d"
Date: Tue, 24 Sep 2024 02:59:31 GMT
OkHttp-Sent-Millis: 1727146771482
OkHttp-Received-Millis: 1727146771525

...cabe.1 file content

[ {
  "accountNumber" : "9999999990",
  "accountName" : "John Doe",
  "balance" : 1000000.0,
  "lastTransactionDate" : [ 2024, 8, 19, 10, 0 ],
  "description" : "Personal savings account"
} ]

logcat

# first request

--> GET http://172.19.111.149:8080/api-mock/hrc/fixed-accounts?count=1
--> END GET
<-- 200 http://172.19.111.149:8080/api-mock/hrc/fixed-accounts?count=1 (61ms)
Content-Type: application/json
Content-Length: 191
Keep-Alive: timeout=60
Connection: keep-alive
ETag: "05099b4b846c91908f7b65d1affda405d"
Date: Tue, 24 Sep 2024 02:59:02 GMT
[ {
    "accountNumber" : "9999999990",
    "accountName" : "John Doe",
    "balance" : 1000000.0,
    "lastTransactionDate" : [ 2024, 8, 19, 10, 0 ],
    "description" : "Personal savings account"
} ]
<-- END HTTP (191-byte body)

# second request. However, the if-none-match header is not added to the request header.

--> GET http://172.19.111.149:8080/api-mock/hrc/fixed-accounts?count=1
--> END GET
<-- 200 http://172.19.111.149:8080/api-mock/hrc/fixed-accounts?count=1 (56ms)
...
...
...
<-- END HTTP (191-byte body)

# Even if the server sends a response header like Cache-Control: max-age=0, okhttp still does not send if-none-matched in the request header in the next request.

Is etag not working due to a bug in okhttp? Or am I using okhttp incorrectly? If so, please let me know how to use it correctly. There doesn't seem to be any documentation on how to use etag in the okhttp documentation.

Thanks.

yschimke commented 2 months ago

I suspect you are observing the request headers via an application interceptor, not a network interceptor.

https://square.github.io/okhttp/features/interceptors/#choosing-between-application-and-network-interceptors

Observe the application’s original intent. Unconcerned with OkHttp-injected headers like If-None-Match.

This should be supported, and the tests cover this case.

See https://github.com/square/okhttp/blob/f2771425cb714a5b0b27238bd081b2516b4d640f/okhttp/src/test/java/okhttp3/CallTest.kt#L1793

Generally, it's not documented because it should be following the HTTP caching specs. You shouldn't need to change your client after setting the cache.

I confirmed with the following test against https://httpbin.org/etag/v1

  @Test
  @Flaky
  fun conditionalCacheHit() {
    client =
      client.newBuilder()
        .cache(cache)
        .addNetworkInterceptor {
          println("request: " + it.request().headers)
          it.proceed(it.request()).also {
            println("response: " + it.headers)
          }
        }
        .build()

    // Store a response in the cache.
    val request = Request("https://httpbin.org/etag/v1".toHttpUrl())
    executeSynchronously(request)
      .assertCode(200)

    // Hit that stored response. It's different, but Vary says it doesn't matter.
    Thread.sleep(10) // Make sure the timestamps are unique.
    val cacheHit =
      executeSynchronously(
        request,
      )

    // Check the merged response. The request is the application's original request.
    cacheHit.assertCode(200)
      .assertRequestUrl(request.url)
      .assertRequestHeader("If-None-Match") // No If-None-Match on the user's request.

    // Check the cached response. Its request contains only the saved Vary headers.
    cacheHit.cacheResponse()
      .assertCode(200)
      .assertHeader("ETag", "v1")
      .assertRequestUrl(request.url)
      .assertRequestHeader("If-None-Match") // This wasn't present in the original request.

    // Check the network response. It has the caller's request, plus some caching headers.
    cacheHit.networkResponse()
      .assertCode(304)
      .assertRequestHeader("If-None-Match", "v1") // If-None-Match in the validation request.
  }

Please reopen with a reproducible test case.