Polish, documentation, use workspace

src/vfp_etag.c

 /*-
- * Copyright 2017 UPLEX - Nils Goroll Systemoptimierung
+ * Copyright 2017,2019 UPLEX - Nils Goroll Systemoptimierung
  * All rights reserved.
  *
  * Author: Author: Nils Goroll <nils.goroll@uplex.de>
@@ -30,7 +30,6 @@
 #include <stdlib.h>
 #include <string.h>
 
-//#include "cache/cache_varnishd.h"
 #include "cache/cache.h"
 #include "cache/cache_filter.h"
 #include "vsha256.h"
@@ -43,28 +42,32 @@ struct etag {
 #define BODYHASH_MAGIC		0xb0d16a56
 
 	struct VSHA256Context	sha256ctx;
-	// unsigned char	hash[VSHA256_LEN];
 };
 
-const char placeholder[] = "\"vmod-esiextra magic placeholder "	\
-    "vmod-esiextra magic placeholder \"";
+const char const placeholder[] =
+    "\"vetag"					\
+    "VMOD-ETAG MAGIC ETAGPLACEHOLDER "	\
+    "VMOD-ETAG MAGIC ETAG PLACEHOLDER\"";
 const size_t placeholder_l = sizeof(placeholder) - 1;
 
 static enum vfp_status
 vfp_etag_init(struct vfp_ctx *vc, struct vfp_entry *vfe)
 {
-	struct etag *bh;
+	struct etag *bh = NULL;
 
 	CHECK_OBJ_NOTNULL(vc, VFP_CTX_MAGIC);
 	CHECK_OBJ_NOTNULL(vfe, VFP_ENTRY_MAGIC);
 
 	assert(vfe->vfp == &VFP_etag);
-	assert(placeholder_l == VSHA256_LEN * 2 + 2);
+	assert(placeholder_l == VSHA256_LEN * 2 + 2 /* " */ + 5 /* vetag */);
+	AZ(vfe->priv1);
 
-	// XXX workspace
-	ALLOC_OBJ(bh, BODYHASH_MAGIC);
+	AN(vc->resp);
+	bh = WS_Alloc(vc->resp->ws, sizeof(*bh));
 	if (bh == NULL)
 		return (VFP_ERROR);
+
+	INIT_OBJ(bh, BODYHASH_MAGIC);
 	VSHA256_Init(&bh->sha256ctx);
 
 	vfe->priv1 = bh;
@@ -92,19 +95,16 @@ vfp_etag_pull(struct vfp_ctx *vc, struct vfp_entry *vfe, void *p,
 	return (vp);
 }
 
-const char hexe[16] = {
+const char const hexe[16] = {
 	"0123456789abcdef",
 };
 
-// XXX make non-void
 static void
 vfp_etag_fini(struct vfp_ctx *vc, struct vfp_entry *vfe)
 {
 	struct etag *bh;
 	unsigned char	sha[VSHA256_LEN];
-	const ssize_t	hexl = VSHA256_LEN * 2 + 3;
-	char		hex[hexl];
-	char		*etag, *p;
+	char		*etag, *lim;
 	int		i;
 	ssize_t		l;
 
@@ -117,31 +117,18 @@ vfp_etag_fini(struct vfp_ctx *vc, struct vfp_entry *vfe)
 	CAST_OBJ_NOTNULL(bh, vfe->priv1, BODYHASH_MAGIC);
 	vfe->priv1 = NULL;
 
-	VSHA256_Final(sha, &bh->sha256ctx);
-
-	p = hex;
-	*p++ = '"';
-	for (i = 0; i < VSHA256_LEN; i++) {
-		*p++ = hexe[(sha[i] & 0xf0) >> 4];
-		*p++ = hexe[sha[i] & 0x0f];
-	}
-	*p++ = '"';
-	*p++ = '\0';
-	assert(pdiff(hex, p) == hexl);
-
 	// HACKY
-	p = TRUST_ME(ObjGetAttr(vc->wrk, vc->oc, OA_HEADERS, &l));
-	if (p == NULL) {
+	etag = TRUST_ME(ObjGetAttr(vc->wrk, vc->oc, OA_HEADERS, &l));
+	if (etag == NULL) {
 		VSLb(vc->wrk->vsl, SLT_Error, "etag: no object");
-		goto out;
+		return;
 	}
-	etag = p;
-	p = etag + l;
+	lim = etag + l;
 	do {
 		etag = memchr(etag, '"', l);
 		if (etag == NULL)
 			break;
-		l = p - etag;
+		l = lim - etag;
 		if (l < placeholder_l) {
 			etag = NULL;
 			break;
@@ -153,15 +140,25 @@ vfp_etag_fini(struct vfp_ctx *vc, struct vfp_entry *vfe)
 
 	if (etag == NULL) {
 		VSLb(vc->wrk->vsl, SLT_Error, "etag: no placeholder");
-		goto out;
+		return;
 	}
 
+	VSHA256_Final(sha, &bh->sha256ctx);
 
-	assert(*etag == '"');
-	memcpy(etag, hex, hexl);
+	assert(etag[0] == '"' &&
+	    etag[1] == 'v' &&
+	    etag[2] == 'e' &&
+	    etag[3] == 't' &&
+	    etag[4] == 'a' &&
+	    etag[5] == 'g');
 
-  out:
-	FREE_OBJ(bh);
+	etag += 6;
+	for (i = 0; i < VSHA256_LEN; i++) {
+		*etag++ = hexe[(sha[i] & 0xf0) >> 4];
+		*etag++ = hexe[sha[i] & 0x0f];
+	}
+	*etag++ = '"';
+	assert(*etag == '\0');
 }
 
 const struct vfp VFP_etag = {

src/vmod_etag.vcc

-$Module etag 3 "Varnish etag Module"
+$Module etag 3 "Varnish ETag Module"
+$Synopsis manual
 
-DESCRIPTION
-===========
+SYNOPSIS
+========
 
-This VCC file was generated by VCDK, it is used to for both the VMOD
-interface and its manual using reStructuredText.
+::
+
+        import etag;
 
-XXX: document vmod-etag
+        sub vetag_backend_fetch {
+            if (bereq.http.If-None-Match ~ {"^(W/)?"vetag"}) {
+                unset bereq.http.If-None-Match;
+            }
+            if (bereq.http.If-Match ~ {"^(W/)?"vetag"}) {
+                unset bereq.http.If-Match;
+            }
+            if (bereq.http.If-Range ~ {"^(W/)?"vetag"}) {
+                unset bereq.http.If-Range;
+            }
+        }
 
-Example
-    ::
+        sub vetag_backend_response {
+            if (! beresp.http.ETag) {
+                set beresp.filters = beresp.filters + " etag";
+                # no Etag for cache miss with streaming
+                set beresp.do_stream = false;
+                # berep.do_* variables have no effect beyond this point
+            }
+        }
 
-        import etag;
+        sub vetag_deliver {
+            # safeguard for do_stream == true
+            if (resp.http.Etag ~ {"^(W/)?"vetag.*[-A-Z ]"}) {
+                unset resp.http.Etag;
+            }
+        }
+
+        sub vcl_backend_fetch {
+            # first
+            call vetag_backend_fetch;
+        }
+
+        sub vcl_backend_response {
+            # last - or after any change to beresp.do_*
+            call vetag_backend_response;
+        }
 
         sub vcl_deliver {
-	    set resp.http.Hello = etag.hello();
+            # first
+            call vetag_deliver;
         }
 
-XXX: define vmod-etag interface
+DESCRIPTION
+===========
 
-$Event event_function
+This vmod adds a `beresp.filter`_ (see aka Varnish Fetch Processor /
+VFP) to generate ETags on the way into the Varnish Cache.
+
+.. _`beresp.filter`: http://varnish-cache.org/docs/trunk/reference/vcl.html#beresp
+
+It is strongly recommended to use this vmod von VCL as shown in the `SYNOPSIS`_
+
+ETag generation is enabled by adding ``etag`` to ``beresp.filters`` as
+shown in the example above.
+
+The ETag format is the string ``"vetag`` followed by 64 hexadecimal
+characters representing a SHA256 hash over the response body and a
+closing quote ``"``. The ``"vetag`` prefix is used to identify
+incompletely generated ETags as explained below.
+
+Usage notes:
+
+* For *streaming*, an ETag can not be generated, because, by design,
+  the response headers are being sent before the backend response is
+  complete.
+
+  The VCL template in the `SYNOPSIS`_ thus contains ``set
+  beresp.do_stream = false`` to disable streaming when the ``etag``
+  filter is activated.
+
+  The ``vcl_deliver`` code from the `SYNOPSIS`_ nevertheless supports
+  streaming by removing any incomplete ETags generated by this vmod.
+
+* By design, ETags generated by this vmod can not be used for backend
+  conditional requests.
+
+  The VCL template in the `SYNOPSIS`_ thus contains code to remove any
+  conditional request headers based on ``vetag`` ETags.
+
+Implementation note:
+
+* Varnish core code does not officially support modifying headers of
+  cache objects after creation, which is exactly what is required
+  here. This vmod thus works by leaving a fixed length placeholder
+  ETag header with the cache object, which later gets overwritten by
+  the VFP. This may or may not work with custom storage engines.
 
 SEE ALSO
-========vcl\(7),varnishd\(1)
+========
+* :ref:`vcl(7)`
+* :ref:`varnishd(1)`
+
+$Event event_function

src/vtc/vmod_etag.vtc

 varnishtest "test vmod-etag"
 
+barrier b1 cond 2
+
 server s1 {
 	rxreq
-	txresp -bodylen 1048576
+	expect req.url == "/stream"
+	txresp -nolen -hdr "Transfer-Encoding: chunked"
+	chunkedlen 8192
+	delay 1
+	chunkedlen 8192
+	chunkedlen 0
+	barrier b1 sync
+
+	rxreq
+	expect req.url == "/nostream"
+	txresp -nolen -hdr "Transfer-Encoding: chunked"
+	chunkedlen 8192
+	delay 1
+	chunkedlen 8192
+	chunkedlen 0
+
+	rxreq
+	expect req.url == "/gzip"
+	txresp -gzipbody 0123456701234567012345670123456701234567012345670123456701234567012345670123456701234567012345670123456701234567012345670123456701234567012345670123456701234567012345670123456701234567012345670123456701234567012345670123456701234567012345670123456701234567
+
 } -start
 
 varnish v1 -vcl+backend {
@@ -10,21 +31,47 @@ varnish v1 -vcl+backend {
 
 	sub vcl_backend_response {
 		set beresp.filters = beresp.filters + " etag";
+		if (bereq.url != "/stream") {
+			set beresp.do_stream = false;
+		}
 	}
 
 	sub vcl_deliver {
-		# XXX racy
-		if (resp.http.Etag ~ "placeholder") {
+		# safeguard for do_stream == true
+		if (resp.http.Etag ~ {"^(W/)?"vetag.*[-A-Z ]"}) {
 			unset resp.http.Etag;
 		}
 	}
 } -start
 
+## chunkedlen test pattern is 01234567
+# $ perl -e 'print ("01234567" x 2048);'|sha256sum
+# 5e2cbc9e7c53f7a2c28497d42f9d7e6049591297869ec9b2f38e5b046c10a42a  -
+
 client c1 {
-	txreq
+	txreq -url "/stream"
+	rxresp
+	expect resp.status == 200
+	expect resp.http.ETag == <undef>
+
+	barrier b1 sync
+	txreq -url "/stream"
+	rxresp
+	expect resp.status == 200
+	expect resp.http.ETag == {"vetag5e2cbc9e7c53f7a2c28497d42f9d7e6049591297869ec9b2f38e5b046c10a42a"}
+
+	txreq -url "/nostream"
 	rxresp
 	expect resp.status == 200
-	txreq
+	expect resp.http.ETag == {"vetag5e2cbc9e7c53f7a2c28497d42f9d7e6049591297869ec9b2f38e5b046c10a42a"}
+
+	txreq -url "/nostream"
+	rxresp
+	expect resp.status == 200
+	expect resp.http.ETag == {"vetag5e2cbc9e7c53f7a2c28497d42f9d7e6049591297869ec9b2f38e5b046c10a42a"}
+
+	txreq -url "/gzip"
 	rxresp
 	expect resp.status == 200
+	expect resp.http.ETag == {W/"vetag7f2ed3c1f4db208d92d837bce4544807005e99df266d6f780a174b4a68be178b"}
 } -run