Patchwork hgweb: emit a valid, weak ETag

login
register
mail settings
Submitter Anton Shestakov
Date July 8, 2016, 7:43 p.m.
Message ID <47eb9964a1d6e5453420.1468007008@neuro>
Download mbox | patch
Permalink /patch/15777/
State Accepted
Delegated to: Matt Mackall
Headers show

Comments

Anton Shestakov - July 8, 2016, 7:43 p.m.
# HG changeset patch
# User Anton Shestakov <av6@dwimlabs.net>
# Date 1468005984 -28800
#      Sat Jul 09 03:26:24 2016 +0800
# Node ID 47eb9964a1d6e545342011f4bf2574695dd87169
# Parent  b4d117cee636be8a566f56e84d4b351a736a1299
hgweb: emit a valid, weak ETag

Previously, ETag headers from hgweb weren't correctly formed, because rfc2616
(section 14, header definitions) requires double quotes around the content of
the header. str(web.mtime) didn't do that.

Additionally, strong ETags signify that the resource representations are
byte-for-byte identical. That is, they can be reconstructed from byte ranges if
client so wishes. Considering ETags for all hgweb pages is just mtime of
00changelog.i and doesn't consider of e.g. .hg/hgrc with description, contact
and other fields, it's clearly shouldn't be strong. The W/ prefix marks it as
weak, which still allows caching the whole served file/page, but doesn't allow
byte-range requests.
Jun Wu - July 8, 2016, 8:04 p.m.
This looks good to me. Rails is doing the same thing in practice and I
confirmed the behavior with the RFC. Marking as Pre-Reviewed.

Excerpts from Anton Shestakov's message of 2016-07-09 03:43:28 +0800:
> # HG changeset patch
> # User Anton Shestakov <av6@dwimlabs.net>
> # Date 1468005984 -28800
> #      Sat Jul 09 03:26:24 2016 +0800
> # Node ID 47eb9964a1d6e545342011f4bf2574695dd87169
> # Parent  b4d117cee636be8a566f56e84d4b351a736a1299
> hgweb: emit a valid, weak ETag
> 
> Previously, ETag headers from hgweb weren't correctly formed, because rfc2616
> (section 14, header definitions) requires double quotes around the content of
> the header. str(web.mtime) didn't do that.
> 
> Additionally, strong ETags signify that the resource representations are
> byte-for-byte identical. That is, they can be reconstructed from byte ranges if
> client so wishes. Considering ETags for all hgweb pages is just mtime of
> 00changelog.i and doesn't consider of e.g. .hg/hgrc with description, contact
> and other fields, it's clearly shouldn't be strong. The W/ prefix marks it as
> weak, which still allows caching the whole served file/page, but doesn't allow
> byte-range requests.
> 
> diff --git a/mercurial/hgweb/common.py b/mercurial/hgweb/common.py
> --- a/mercurial/hgweb/common.py
> +++ b/mercurial/hgweb/common.py
> @@ -187,7 +187,7 @@ def get_contact(config):
>              os.environ.get("EMAIL") or "")
>  
>  def caching(web, req):
> -    tag = str(web.mtime)
> +    tag = 'W/"%s"' % web.mtime
>      if req.env.get('HTTP_IF_NONE_MATCH') == tag:
>          raise ErrorResponse(HTTP_NOT_MODIFIED)
>      req.headers.append(('ETag', tag))
Matt Mackall - July 8, 2016, 9:06 p.m.
On Sat, 2016-07-09 at 03:43 +0800, Anton Shestakov wrote:
> # HG changeset patch
> # User Anton Shestakov <av6@dwimlabs.net>
> # Date 1468005984 -28800
> #      Sat Jul 09 03:26:24 2016 +0800
> # Node ID 47eb9964a1d6e545342011f4bf2574695dd87169
> # Parent  b4d117cee636be8a566f56e84d4b351a736a1299
> hgweb: emit a valid, weak ETag

Queued for default, thanks.

-- 
Mathematics is the supreme nostalgia of our time.

Patch

diff --git a/mercurial/hgweb/common.py b/mercurial/hgweb/common.py
--- a/mercurial/hgweb/common.py
+++ b/mercurial/hgweb/common.py
@@ -187,7 +187,7 @@  def get_contact(config):
             os.environ.get("EMAIL") or "")
 
 def caching(web, req):
-    tag = str(web.mtime)
+    tag = 'W/"%s"' % web.mtime
     if req.env.get('HTTP_IF_NONE_MATCH') == tag:
         raise ErrorResponse(HTTP_NOT_MODIFIED)
     req.headers.append(('ETag', tag))