mod_proxy/4415: Cache entries are unnecessarily deleted as a result of pragma: no-proxy

[Victor Pulver <vi...@latimes.com>]

>Number:         4415
>Category:       mod_proxy
>Synopsis:       Cache entries are unnecessarily deleted as a result of pragma: no-proxy
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    apache
>State:          open
>Class:          sw-bug
>Submitter-Id:   apache
>Arrival-Date:   Fri May 14 16:20:00 PDT 1999
>Last-Modified:
>Originator:     victor.pulver@latimes.com
>Organization:
apache
>Release:        1.3.6
>Environment:
uname -a
SunOS latimes9.su-colo.bbnplanet.com 5.6 Generic_105181-08 sun4u sparc SUNW,Ultra-Enterprise

Apache configured as a caching reverse proxy
>Description:
We noticed that files which do not change were constantly being removed and
recreated in the cache. We enabled the 'explain' diagnostic messages and found
that this happened when a 304 response was returned for a pragma: no-cache
request. Here's a sample log:

--- a no-cache, if-modified-since request which gives a 304:

[Fri May 14 16:03:26 1999] [debug] proxy_cache.c(662): Request for http://205.180.59.54/GIFS/PIXELS/blankpix.gif, pragma=no-cache, auth=(null), ims=926703571, imstr=Fri, 14 May 1999 17:39:31 GMT
[Fri May 14 16:03:26 1999] [debug] proxy_cache.c(760): Local copy not present or expired. Declining.
[Fri May 14 16:03:26 1999] [debug] proxy_cache.c(838): Response is not cacheable, unlinking /usr/local/apache/cache/f/9/E/pHLyjQEuHg0bTAzmyjA

--- a no-cache, if-modified-since request which gives a 200:

[Fri May 14 16:03:31 1999] [debug] proxy_cache.c(662): Request for http://205.180.59.54/GIFS/PIXELS/blankpix.gif, pragma=no-cache, auth=(null), ims=923878800, imstr=Mon, 12 Apr 1999 01:00:00 GMT
[Fri May 14 16:03:31 1999] [debug] proxy_cache.c(760): Local copy not present or expired. Declining.
[Fri May 14 16:03:31 1999] [debug] proxy_cache.c(899): Expiry date is 0
[Fri May 14 16:03:31 1999] [debug] proxy_cache.c(910): Expiry date calculated 926724957
[Fri May 14 16:03:31 1999] [debug] proxy_cache.c(1006): Create temporary file /usr/local/apache/cache/tmp0rV_Ot

In ap_proxy_cache_check, the cache file is not opened if the request cannot
access the cache; one of the conditions in which this happens is if there is a 
pragma: no-cache. In ap_proxy_cache_update, if it is determined that the
response should not be cached, the current cache file is unlinked. One of the
rules is a 304 response when there is no cache file; the check for a cache 
file examines a filepointer which is set to NULL if the cache file was
not examined:

 * what responses should we not cache?
 * Unknown status responses and those known to be uncacheable
 * 304 HTTP_NOT_MODIFIED response when we have no valid cache file, or
 * 200 HTTP_OK response from HTTP/1.0 and up without a Last-Modified header, or
 * HEAD requests, or
 * requests with an Authorization header, or
 * protocol requests nocache (e.g. ftp with user/password)
 */
/* @@@ XXX FIXME: is the test "r->status != HTTP_MOVED_PERMANENTLY" correct?
 * or shouldn't it be "ap_is_HTTP_REDIRECT(r->status)" ? -MnKr */
    if ((r->status != HTTP_OK && r->status != HTTP_MOVED_PERMANENTLY && r->status != HTTP_NOT_MODIFIED) ||
	(expire != NULL && expc == BAD_DATE) ||
	(r->status == HTTP_NOT_MODIFIED && (c == NULL || c->fp == NULL)) ||
	(r->status == HTTP_OK && lmods == NULL && is_HTTP1) ||
	r->header_only ||
	ap_table_get(r->headers_in, "Authorization") != NULL ||
	nocache) {

I'm not sure about all the conditions tested for, but in the case of a 304
response to a no-pragma request, it doesn't seem appropriate to delete a
previously cached response. The proposed change bypasses the unlinking if
this is a pragma: no-cache request and the response was HTTP_NOT_MODIFIED;
perhaps the unlink just be deleted.
>How-To-Repeat:

>Fix:
Make the changes below proxy_cache.c

Note: the 'new' line numbers include the changes proposed in PR#4316; the change
at line 1095 corrects the file name displayed when an unlink of a cached
file fails

681a720,725
> /* 5/11/99 add explain to better indicate reason for not using cache */
>    else {
>       Explain0("Request is not cacheable. Declining.");
>       return DECLINED;
>       }
> /* end 5/11/99 fix */
838d881
< 	Explain1("Response is not cacheable, unlinking %s", c->filename);
845c888,892
<         if (c->filename)
---
> /* 5/14/99 don't delete if this was a 304 and we had a pragma: no-cache */
>     	if (c->filename && 
>     	    ((r->status != HTTP_NOT_MODIFIED) ||
>     	     (! ap_proxy_liststr(ap_table_get(r->headers_in, "Pragma"), "no-cache")))) {
>            Explain1("Response is not cacheable, unlinking %s", c->filename);
846a894
> 	}
1095c1143
< 		     c->tempfile);
---
> 		     c->filename);
>Audit-Trail:
>Unformatted:
[In order for any reply to be added to the PR database, ]
[you need to include <ap...@Apache.Org> in the Cc line ]
[and leave the subject line UNCHANGED.  This is not done]
[automatically because of the potential for mail loops. ]
[If you do not include this Cc, your reply may be ig-   ]
[nored unless you are responding to an explicit request ]
[from a developer.                                      ]
[Reply only with text; DO NOT SEND ATTACHMENTS!         ]