Details
-
New Feature
-
Status: Closed
-
Major
-
Resolution: Fixed
-
trunk
-
None
Description
When accessing a Subversion repository via http:// URL using a web browser, mod_dav_svn returns a "Cache-Control: max-age=604800" header (only on files, not on directories).
This makes the web browser cache the file content for up to a week.
This makes a lot of sense when a fixed revision was requested, but URLs that imply HEAD, no Cache-Control should be sent. (Note that a Last-Modified header is always sent as well, which should be good enough AFAICT.)
Example:
▶ wget -Sq -O /dev/null http://svn.apache.org/repos/asf/subversion/trunk/subversion/mod_dav_svn/repos.c HTTP/1.1 200 OK Date: Thu, 10 Jul 2014 15:16:28 GMT Server: Apache/2.2.25 (Unix) DAV/2 mod_wsgi/3.1 Python/2.7.3 SVN/1.8.1 mod_ssl/2.2.25 OpenSSL/1.0.1h Last-Modified: Mon, 23 Jun 2014 20:50:11 GMT ETag: "1604933//subversion/trunk/subversion/mod_dav_svn/repos.c" Cache-Control: max-age=604800 Accept-Ranges: bytes Content-Length: 164114 Vary: Accept-Encoding Keep-Alive: timeout=15, max=1000 Connection: Keep-Alive Content-Type: text/plain; charset=ISO-8859-1
(Cache-Control is sent but should not be, as no revision was requested.)
URLs like these should still return a Cache-Control header, as they contain an explicit revision:
http://svn.apache.org/repos/asf/subversion/trunk/subversion/mod_dav_svn/repos.c?p=1000000
http://svn.apache.org/repos/asf/subversion/trunk/subversion/mod_dav_svn/repos.c?r=1000000
URLs that point at directories never include a Cache-Control header. IMHO directories with explicit revisions should also have a Cache-Control header.
The main point of this issue however: do not confuse web browsers & force users to hard-reload a file URL of HEAD to see recent modifications.
The header is conditionally hardcoded in subversion/mod_dav_svn/repos.c around line 3050 as:
/* As version resources don't change, encourage caching. */ if ((resource->type == DAV_RESOURCE_TYPE_REGULAR && resource->versioned && !resource->collection) || resource->type == DAV_RESOURCE_TYPE_VERSION) /* Cache resource for one week (specified in seconds). */ apr_table_setn(r->headers_out, "Cache-Control", "max-age=604800");
In pseudo code, I think this should become:
if ((...) && an_explicit_revision_number_was_passed_as_GET_param) apr_table_setn(r->headers_out, "Cache-Control", "max-age=604800");
The GET params are 'r' and 'p'. It seems they don't accept "HEAD", so it might be enough to include the Cache-Control header iff a revision param is non-empty (not having looked through the code too closely yet).
Original issue reported by neels