problem with cache.cache_responses_to_cookies value 0

[Paul Marquess]

I'm attempting to configure TS (3.2.0) to disable caching for all content where 
a cookie is present.  Setting cache.cache_responses_to_cookies to 0 looks like 
it should do that according to the comment in records.config


   # cache responses to cookies has 5 options:

   #   0 - do not cache any responses to cookies

   #   1 - cache for any content-type

   #   2 - cache only for image types

   #   3 - cache for all but text content-types

   #   4 - cache for all but text content-types except OS response

   #       without "Set-Cookie" or with "Cache-Control: public"

   # See also cache-responses-to-cookies in cache.config.

CONFIG proxy.config.http.cache.cache_responses_to_cookies INT 1

Unfortunately when I set cache.cache_responses_to_cookies to 0 in 
records.config I don't see anything written to the cache at all.

Am I correct in assuming that cache.cache_responses_to_cookies is intended to 
influence the caching of content only when a cookie is in play? So the 
behaviour I'm seeing is wrong?

Looking in do_cookiesprevent_caching in HttpTransact.cc it looks like the test 
for the 0 use case (COOKIES_CACHE_NONE) is done too early. Below is the code

  // Can cache all regardless of cookie header - just ignore all cookie headers
  if ((CookiesConfig) cookies_conf == COOKIES_CACHE_ALL) {
    return false;
  }
  // Do not cache if cookies option is COOKIES_CACHE_NONE
  if ((CookiesConfig) cookies_conf == COOKIES_CACHE_NONE) {
    return true;
  }

  ...
  if (!response->presence(MIME_PRESENCE_SET_COOKIE) &&
      !request->presence(MIME_PRESENCE_COOKIE) && (cached_request == NULL
                                                   || 
!cached_request->presence(MIME_PRESENCE_COOKIE))) {
    return false;
  }

I don't see any other tests in the code that check for cookies that would be 
triggered before do_cookiesprevent_caching is invoked, so surely the 
COOKIES_CACHE_NONE test needs to be done after the presence of cookies headers 
has been determined?

So the code would become

  // Can cache all regardless of cookie header - just ignore all cookie headers
  if ((CookiesConfig) cookies_conf == COOKIES_CACHE_ALL) {
    return false;
  }
...
  if (!response->presence(MIME_PRESENCE_SET_COOKIE) &&
      !request->presence(MIME_PRESENCE_COOKIE) && (cached_request == NULL
                                                   || 
!cached_request->presence(MIME_PRESENCE_COOKIE))) {
    return false;
  }

  // Know we have a cookie present at this point

  // Do not cache if cookies option is COOKIES_CACHE_NONE
  // and cookie detected
  if ((CookiesConfig) cookies_conf == COOKIES_CACHE_NONE) {
    return true;
  }

Looks like the same issue is present in TS 4.0.2. Tentative patch enclosed.

cheers
Paul