Currently mod_dav_svn sets ETag and Last-Modified HTTP headers for GET
responses. These headers are optional and and are not used by
Subversion client. But they used by browsers and intermediate proxies
to cache responses.

ETag header is cheap to construct: it's just last modification
revision number of the node.

Last-Modified header is relatively expensive to calculate: it's
svn:date revision property of revision where path was modified.
Revision properties are mutable and cannot be cached effectively.The
other minor problem with Last-Modified header: svn:date revision
property can be changed to any value, while RFC 7223 requires
Last-Modified date to be earlier than the server's time of message
origination (Date) [1]

All browsers support ETag header (it's HTTP/1.1 header) and RFC 7232
recommends to prefer using ETag instead of Last-Modified [2].

Given all above I propose to stop adding Last-Modified header for HTTP
GET responses.

[1] https://tools.ietf.org/html/rfc7232#section-2.2.1
[2] https://tools.ietf.org/html/rfc7232#section-3.3

-- 
Ivan Zhakov
Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c      (revision 1721898)
+++ subversion/mod_dav_svn/repos.c      (working copy)
@@ -3036,50 +3036,6 @@
        && resource->baselined))
 
 
-/* Return the last modification time of RESOURCE, or -1 if the DAV
-   resource type is not handled, or if an error occurs.  Temporary
-   allocations are made from RESOURCE->POOL. */
-static apr_time_t
-get_last_modified(const dav_resource *resource)
-{
-  apr_time_t last_modified;
-  svn_error_t *serr;
-  svn_revnum_t created_rev;
-  svn_string_t *date_time;
-
-  if (RESOURCE_LACKS_ETAG_POTENTIAL(resource))
-    return -1;
-
-  if ((serr = svn_fs_node_created_rev(&created_rev, resource->info->root.root,
-                                      resource->info->repos_path,
-                                      resource->pool)))
-    {
-      svn_error_clear(serr);
-      return -1;
-    }
-
-  if ((serr = svn_fs_revision_prop2(&date_time, resource->info->repos->fs,
-                                    created_rev, SVN_PROP_REVISION_DATE,
-                                    TRUE, resource->pool, resource->pool)))
-    {
-      svn_error_clear(serr);
-      return -1;
-    }
-
-  if (date_time == NULL || date_time->data == NULL)
-    return -1;
-
-  if ((serr = svn_time_from_cstring(&last_modified, date_time->data,
-                                    resource->pool)))
-    {
-      svn_error_clear(serr);
-      return -1;
-    }
-
-  return last_modified;
-}
-
-
 const char *
 dav_svn__getetag(const dav_resource *resource, apr_pool_t *pool)
 {
@@ -3149,7 +3105,6 @@
   svn_error_t *serr;
   svn_filesize_t length;
   const char *mimetype = NULL;
-  apr_time_t last_modified;
 
   /* As version resources don't change, encourage caching. */
   if (is_cacheable(r, resource))
@@ -3161,15 +3116,6 @@
   if (!resource->exists)
     return NULL;
 
-  last_modified = get_last_modified(resource);
-  if (last_modified != -1)
-    {
-      /* Note the modification time for the requested resource, and
-         include the Last-Modified header in the response. */
-      ap_update_mtime(r, last_modified);
-      ap_set_last_modified(r);
-    }
-
   /* generate our etag and place it into the output */
   apr_table_setn(r->headers_out, "ETag",
                  dav_svn__getetag(resource, resource->pool));

Reply via email to