Hi there, I ran into a weird case where *I think* mod_cache should be caching a request that it is not. Thought I would try to fix it myself, but would like to seek your feedback as it is my first httpd patch (thanks to pquerna for the help/encouragement).
Caching a 302 is generally not valid (RFC2616 13.4), unless the response headers includes an Expires or Cache Control header (section 13.4, last paragraph). This makes the fix a matter of messing with the cacheability logic. I optimized for least amount of code change, but there are surely different ways to do this. Feedback on the best approach would be greatly appreciated! Thanks, -Alex PS: I also filed a bug, if that is a better forum for this discussion: https://issues.apache.org/bugzilla/show_bug.cgi?id=46346 -- Index: modules/cache/mod_cache.c =================================================================== --- modules/cache/mod_cache.c (revision 723584) +++ modules/cache/mod_cache.c (working copy) @@ -438,8 +438,27 @@ * We include 304 Not Modified here too as this is the origin server * telling us to serve the cached copy. */ - reason = apr_psprintf(p, "Response status %d", r->status); + if (exps != NULL || cc_out != NULL) { + /* We are also allowed to cache any response given that it has a valid + * Expires or Cache Control header. If we find a either of those here, + * we pass request through the rest of the tests. From the RFC: + * + * A response received with any other status code (e.g. status codes 302 + * and 307) MUST NOT be returned in a reply to a subsequent request + * unless there are cache-control directives or another header(s) that + * explicitly allow it. For example, these include the following: an + * Expires header (section 14.21); a "max-age", "s-maxage", "must- + * revalidate", "proxy-revalidate", "public" or "private" cache-control + * directive (section 14.9). + */ + } + else { + reason = apr_psprintf(p, "Response status %d", r->status); + } } + if (reason) { + /* noop */ + } else if (exps != NULL && exp == APR_DATE_BAD) { /* if a broken Expires header is present, don't cache it */ reason = apr_pstrcat(p, "Broken expires header: ", exps, NULL);
