You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2013/03/15 18:31:38 UTC

[Bug 54706] New: mod_cache stores and serves the headers specified in Cache-Control: no-cache= or private=

https://issues.apache.org/bugzilla/show_bug.cgi?id=54706

            Bug ID: 54706
           Summary: mod_cache stores and serves the headers specified in
                    Cache-Control: no-cache= or private=
           Product: Apache httpd-2
           Version: 2.5-HEAD
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: mod_cache
          Assignee: bugs@httpd.apache.org
          Reporter: ylavic.dev@gmail.com
    Classification: Unclassified

The mod_cache module handles the Cache-Control directive "no-cache" the same
way whether it specifies a field-name (response header) or not, and "private"
with a field-name is also handled the same way, that is: treat as stale causing
revalidation (ie. send a conditional request to the origin).

According to the RFC2616 14.9.1 :
   no-cache
       If the no-cache directive does not specify a field-name, then a
      cache MUST NOT use the response to satisfy a subsequent request
      without successful revalidation with the origin server. This
      allows an origin server to prevent caching even by caches that
      have been configured to return stale responses to client requests.

      If the no-cache directive does specify one or more field-names,
      then a cache MAY use the response to satisfy a subsequent request,
      subject to any other restrictions on caching. However, the
      specified field-name(s) MUST NOT be sent in the response to a
      subsequent request without successful revalidation with the origin
      server. This allows an origin server to prevent the re-use of
      certain header fields in a response, while still allowing caching
      of the rest of the response.

My understanding is that no-cache with a field-name does concern that
field-name, not the whole response, hence mod_cache is allowed to serve the
cached response if the specified header is not there (with regard to the other
restrictions).

When the specified header is cached, a revalidation is required, and now the
problem is when the "revalidated" response is a 304 (Not Modified), but that
304 does not "overwrite" the headers specified in the no-cache or private
directives. Should mod_cache serve its cached headers or not ?

The RFC does not say anything about the private directive with a field-name,
can one consider it is the same as no-cache, or is it the same difference than
without a field-name (no-cache = revalidation, private = no store, hence
mod_cache would serve the cached headers with no-cache,but not with private...)
?

IMHO, either private or no-cache,mod_proxy should never store (or at least
serve) the headers specified with these directives.

The only applications I've seen so far implementing that feature use :
 Cache-Control: no-cache="Set-Cookie",
none uses private, or anything else than Set-Cookie,and of course these apps
don't overwrite the Set-Cookie on 304, how could they ?

Now consider the following scenario :

clientA => mod_cache:
GET /foo HTTP/1.1

mod_cache => origin:
GET /foo HTTP/1.1

origin => mod_cache:
HTTP/1.1 200
ETag: "etag"
Date: "date"
Expires: "expiry"
Cache-Control: private="Set-Cookie"
Set-Cookie: data=A

mod_cache => client:
HTTP/1.1 200
ETag: "etag"
Date: "date"
Expires: "expiry"
Cache-Control: private="Set-Cookie", max-age=3600
Set-Cookie: data=A

Later on :

clientB => mod_cache:
GET /foo HTTP/1.1
Cookie: data=B

mod_cache => origin:
GET /foo HTTP/1.1
Cookie: data=B
If-None-Match: "etag"
If-Modified-Since: "date"

origin => mod_cache (no Set-Cookie here, the origin wants to continue with
data=B) :
HTTP/1.1 304
ETag: "etag"

mod_cache => clientB:
HTTP/1.1 200 OK
ETag: "etag"
Date: "date"
Expires: "expiry"
Cache-Control: private="Set-Cookie", max-age=3600
Set-Cookie: data=A

That's actually how mod_cache is handling the scenario (with private or
no-cache), and clientB becomes clientA!

Both versions trunk, 2.4 and 2.2 handle no-cache with or without a header as
stale response as a whole.
Both trunk and 2.4 handle private with a header as stale response (as a whole).
Only 2.2 handles private with or without a header as no store (as a whole).

I have got different patches (for all the versions) to strip these headers,
should this bug be valid, but I need to know if they better not be stored or
stripped from the cache (private vs no-cache?).

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54706] mod_cache stores and serves the headers specified in Cache-Control: no-cache= or private=

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54706

Yann Ylavic <yl...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |NEW

--- Comment #8 from Yann Ylavic <yl...@gmail.com> ---
I missed to change the status back.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54706] mod_cache stores and serves the headers specified in Cache-Control: no-cache= or private=

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54706

Yann Ylavic <yl...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #30258|0                           |1
        is obsolete|                            |

--- Comment #6 from Yann Ylavic <yl...@gmail.com> ---
Created attachment 30259
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=30259&action=edit
This patch fixes the forwarding of the original headers (304) with stale entry
and computes/parses the cached headers once (no tabs)

Sorry, the previous patch introduced some tabulations.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54706] mod_cache stores and serves the headers specified in Cache-Control: no-cache= or private=

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54706

Yann Ylavic <yl...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #30259|0                           |1
        is obsolete|                            |

--- Comment #7 from Yann Ylavic <yl...@gmail.com> ---
Created attachment 30260
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=30260&action=edit
Compute cached headers once and merge them to the request headers when needed

Finally the previous patch is no good either.

The cached headers (out) have to be merged to the response headers on 304 for
ap_meet_conditions to take the good decision, hence this new patch.

Also, since the cached headers contain entity-headers, these have to be
(re)unset from the response when ap_meet_conditions returns 304.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54706] mod_cache stores and serves the headers specified in Cache-Control: no-cache= or private=

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54706

--- Comment #3 from Yann Ylavic <yl...@gmail.com> ---
Sorry for my late answer.

The patch works well exept in one case: mod_proxy has a stale entry which is
not "fresh" (according to. cache_check_freshness), and the origin responds to
the validation request with a 304.

In that case (that is whenever "r->headers_out =
ap_cache_cacheable_headers_out(r)" is executed in cache_save_filter), the
original response headers concerned by "no-cache=" and/or "private=" won't be
stored in the cache (legitimately), but they won't be forwarded to the client
either.

The same arise for the headers specified with the CacheIgnoreHeaders directive,
but that was already the case before your patch(es).

Finally, the macro CACHE_SEPARATOR is defined as ",   " (with 3 spaces), I
guess some tab might have been expanded in there...

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54706] mod_cache stores and serves the headers specified in Cache-Control: no-cache= or private=

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54706

--- Comment #1 from Yann Ylavic <yl...@gmail.com> ---
Created attachment 30107
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=30107&action=edit
Patch against trunk and 2.4

This patch will not store the headers specified by Cache-Control private= or
no-cache= and allow mod_cache to cache (and serve) the rest of the response
(with regard to other restrictions).

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54706] mod_cache stores and serves the headers specified in Cache-Control: no-cache= or private=

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54706

Yann Ylavic <yl...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ylavic.dev@gmail.com

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54706] mod_cache stores and serves the headers specified in Cache-Control: no-cache= or private=

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54706

--- Comment #4 from Yann Ylavic <yl...@gmail.com> ---
What about maintaining the r->headers_out and the h->resp_hdrs separately, once
(currently ap_cache_cacheable_headers_out can be called multiple times), and
with no API change by setting the current cache_handle_t in the
r->request_config of cache_module (which does not seem to be used) ?

Thereby, by defining a new function cache_parse_out(h, r) like :
{
    if (!h->cache_obj->info.control.parsed) {
         /* once, parse the Cache-Control header,
            put the right things in r->headers_out,
            when applyable, stuff from cache_accept_headers(),
            together with the right things in h->resp_hdrs */
    }
}

and modifying ap_cache_cacheable_headers_out(r) like :
{
    cache_handle_t *h = (cache_handle_t *)
         ap_get_module_config(r->request_config, &cache_module);
    cache_parse_out(h, r);
    return h->resp_hdrs;
}

and then using ap_set_module_config at the right place(s)...

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54706] mod_cache stores and serves the headers specified in Cache-Control: no-cache= or private=

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54706

--- Comment #5 from Yann Ylavic <yl...@gmail.com> ---
Created attachment 30258
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=30258&action=edit
Compute cached headers once and keep request headers untouched

This patch fixes the forwarding of the original headers (304) with stale entry
and computes/parses the cached headers once.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54706] mod_cache stores and serves the headers specified in Cache-Control: no-cache= or private=

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54706

Yann Ylavic <yl...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #30107|0                           |1
        is obsolete|                            |
  Attachment #30260|0                           |1
        is obsolete|                            |

--- Comment #9 from Yann Ylavic <yl...@gmail.com> ---
Created attachment 30330
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=30330&action=edit
Compute the cacheable headers once

An update to the patch since the code has changed in trunk.

Any news here? Maybe I should comment a bit more about the patch, so here...

This patch (re)associates the cache_request_rec to the request_config, not that
I'm nostalgic for the 2.x code (I guess it was avoided in the new code for good
reasons), but to compute the cacheable headers once per request (ie. strip
CacheIgnoreHeaders/cache-control'ed/hop-by-hop headers once), without breaking
the API.

The new cache_prepare_headers() function does what was done by
ap_cache_cacheable_headers_in/out() before and, optionally, also calls
cache_accept_headers() to merge the stale response headers.

I'm not sure whether this function should be in cache_storage or cache_util
though, and chose the former (near cache_accept_headers()) but that required a
move of the static cache_control_remove() from cache_util to there, and
#include "cache_storage.h" in "cache_util.c". The other option is to move
cache_accept_headers() to cache_util since "cache_util.h" is already included
by "cache_storage.c", but my feeling is that cacheable headers relate more to
storage...

By keeping the cacheable headers separatly from the request's headers_in/out
(actually in the cache_request_rec), the patch fixes the problem raised in
comment #3.

It also fixes the CACHE_SEPARATOR macro which contains multiple spaces and no
tabulation (unfortunate ^I replacement?).

Finally, in the case where a revalidated response leads to a 304
(ap_meet_conditions), the patch unsets the entity headers which have been
merged from the stale entry before passing the response to the filter stack.

Maybe all this changes should be splitted in different patches, or this patch
is broken, let me know...

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54706] mod_cache stores and serves the headers specified in Cache-Control: no-cache= or private=

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54706

Graham Leggett <mi...@sharp.fm> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO

--- Comment #2 from Graham Leggett <mi...@sharp.fm> ---
Fix applied to trunk in r1478382, and proposed for backport to v2.4.

The fix applied supports the option that multiple Cache-Control headers may be
present at once.

Would it be possible to verify that this fixes it for you?

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54706] mod_cache stores and serves the headers specified in Cache-Control: no-cache= or private=

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54706

Graham Leggett <mi...@sharp.fm> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Graham Leggett <mi...@sharp.fm> ---
Fixed on trunk, backported to v2.4.5.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org