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);
|