square / okhttp

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

Do not save cookie on cached requests #7502

Open alzhuravlev opened 1 year ago

alzhuravlev commented 1 year ago

Hello,

I faced a problem. When response comes from the cache all it "Set-Cookies" goes to cookirJar.

Consider following scenario:

  1. Request 1. Response comes from network with Set-Cookie: c1="" and goes to cache.
  2. Request 2. Response comes from network with Set-Cookie: c1="v1"
  3. Request 1. Response comes from cache with Set-Cookie: c1=""

Now cookie c1="", but last time it comes from server with c1="v1" and I'd expext "v1" in my app.

Is it possible to skip saving Set-Cookies from cached responses? Is it a good idea?

Something like this (code from BridgeInterceptor):

    Response networkResponse = chain.proceed(requestBuilder.build());

    if (networkResponse.cacheResponse == null)
      HttpHeaders.receiveHeaders(cookieJar, userRequest.url(), networkResponse.headers());

Thank you!

yschimke commented 1 year ago

I think this makes sense. But I can't find any definitive rules on it.

From https://developer.mozilla.org/en-US/docs/WebC/HTTP/Caching#default_settings it isn't clear that there is any specific behaviour that a user agent should do stripping of Set-Cookie. It seems more likely content should have

Cache-Control: no-cache, private

in that case.

But https://developers.cloudflare.com/cache/best-practices/cache-behavior/ discusses the handling to avoid these types of issues. So I suspect we should make this change.

cc @swankjesse

alzhuravlev commented 1 year ago

Hi again,

@yschimke, thank you for your investigation!

thanks to good okhttp design I am able make a workaroung for this.

  1. do not set // clientBuilder.cookieJar(cookieJar)
  2. add network interceptor:
class CookieJarInterceptor : Interceptor {
    override fun intercept(chain: Interceptor.Chain): Response {

        val requestBuilder = chain.request().newBuilder()

        val cookies = CookieUtils.cookieJar.loadForRequest(chain.request().url())
        if (cookies.isNotEmpty())
            requestBuilder.header("Cookie", cookieHeader(cookies))

        val request = requestBuilder.build()

        val response = chain.proceed(request)

        // this condition depends on your app/server needs. in our case we do not want Set-Cookies on 304 responses
        if (response.code() != 304)
            HttpHeaders.receiveHeaders(CookieUtils.cookieJar, request.url(), response.headers())

        return response
    }

all the best! Aleksei