Allow to selectively set metadata, improve test coverage

src/vmod_zipflow.c

@@ -34,6 +34,7 @@
 #include <cache/cache.h>
 #include <cache/cache_filter.h>
 #include <vcl.h>
+#include <vtim.h>
 
 #include "vcc_zipflow_if.h"
 
@@ -71,7 +72,7 @@ struct zipflow_request {
 	VCL_STRING			url;
 	VCL_STRING			name;
 	unsigned			mode;
-	uint32_t			atime, mtime;
+	VCL_TIME			atime, mtime;
 	VSTAILQ_ENTRY(zipflow_request)	list;
 	struct zipflow_top		*top;
 };
@@ -125,10 +126,10 @@ new_zipflow_request(VRT_CTX, struct zipflow_top *zft)
 
 	zfr->bundle = 1;
 	zfr->level = default_level;
-	zfr->name = "unnamed_file";
+	zfr->name = NULL;
 	zfr->mode = 0644;
-	zfr->atime = ctx->now;
-	zfr->mtime = ctx->now;
+	zfr->atime = -1;
+	zfr->mtime = -1;
 	zfr->top = zft;
 	return (zfr);
 }
@@ -232,30 +233,118 @@ vmod_set_level(VRT_CTX, VCL_INT level)
 }
 
 VCL_VOID
-vmod_meta(VRT_CTX, VCL_STRING name, VCL_STRING mode,
-    VCL_TIME atime, VCL_TIME mtime)
+vmod_meta(VRT_CTX, struct VARGS(meta) *args)
 {
-	unsigned long u;
 	struct zipflow_request *zfr = get_zipflow_request(ctx);
+	unsigned long u;
+	char *e;
 
 	if (zfr == NULL)
 		return;
 
-	u = strtoul(mode, NULL, 8);
-	if (u == ULONG_MAX) {
-		VRT_fail(ctx, ".meta() error converting mode: %d (%s)",
-		    errno, VAS_errtxt(errno));
-		return;
+	if (args->valid_name)
+		zfr->name = args->name;
+
+	if (args->valid_mode) {
+		e = NULL;
+		u = strtoul(args->mode, &e, 8);
+		if (u == ULONG_MAX || (e != NULL && *e != '\0')) {
+			VRT_fail(ctx, ".meta() error converting mode at %s: "
+			    "%d (%s)", e, errno, VAS_errtxt(errno));
+		} else
+			zfr->mode = u;
 	}
-	if (atime == 0)
-		atime = ctx->now;
-	if (mtime == 0)
-		mtime = ctx->now;
-
-	zfr->name = name;
-	zfr->mode = u;
-	zfr->atime = atime;
-	zfr->mtime = mtime;
+	if (args->valid_atime)
+		zfr->atime = args->atime;
+	if (args->valid_mtime)
+		zfr->mtime = args->mtime;
+}
+
+static const char *
+get_url(VRT_CTX)
+{
+	const char *url;
+	VCL_HTTP http;
+
+	if (ctx->http_bereq != NULL)
+		http = ctx->http_bereq;
+	else if (ctx->http_req != NULL)
+		http = ctx->http_req;
+	else
+		WRONG("No http headers");
+
+	AN(http);
+	url = http->hd[HTTP_HDR_URL].b;
+	AN(url);
+	AN(*url);
+	return (url);
+}
+
+static const char *
+default_name(VRT_CTX)
+{
+	const char *url = get_url(ctx);
+	size_t l;
+	char *p;
+
+	url = strrchr(url, '/');
+	AN(url);
+	url++;
+
+	if (*url == '\0' || *url == '?')
+		return ("unnamed_file");
+
+	p = strchr(url, '?');
+	if (p == NULL)
+		return (url);
+
+	assert(p > url);
+	l = p - url;
+	p = WS_Copy(ctx->ws, url, 1 + l);
+	if (p == NULL)
+		return ("unnamed_file");
+	p[l] = '\0';
+	return (p);
+}
+
+static VCL_TIME
+lm(VRT_CTX)
+{
+	const char *p;
+	VCL_TIME lm;
+	VCL_HTTP http;
+
+	if (ctx->http_beresp != NULL)
+		http = ctx->http_beresp;
+	else if (ctx->http_resp != NULL)
+		http = ctx->http_resp;
+	else
+		return (ctx->now);
+
+	if (!http_GetHdr(http, H_Last_Modified, &p))
+		return (ctx->now);
+
+	lm = VTIM_parse(p);
+	return (lm ? lm : ctx->now);
+}
+
+static void
+fill_meta(VRT_CTX, struct zipflow_request *zfr)
+{
+	const char *r;
+
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+	CHECK_OBJ_NOTNULL(zfr, ZIPFLOW_REQUEST_MAGIC);
+
+	if (zfr->name == NULL)
+		zfr->name = default_name(ctx);
+
+	if (zfr->atime < 0)
+		zfr->atime = ctx->now;
+	if (zfr->mtime < 0)
+		zfr->mtime = lm(ctx);
+	VSLb(ctx->vsl, SLT_Debug, "%s %o %f %f", zfr->name, zfr->mode,
+	    zfr->atime, zfr->mtime);
 }
 
 int
@@ -316,8 +405,6 @@ vdp_zipflow_init(VRT_CTX, struct vdp_ctx *vdc, void **priv, struct objcore *oc)
 	if (zfr == NULL)
 		return (1);
 
-	AN(zfr->name);
-
 	zft = zfr->top;
 	CHECK_OBJ_NOTNULL(zft, ZIPFLOW_TOP_MAGIC);
 
@@ -332,8 +419,9 @@ vdp_zipflow_init(VRT_CTX, struct vdp_ctx *vdc, void **priv, struct objcore *oc)
 	}
 	AZ(zip_log(zft->zip, vdc->vsl, vdp_zipflow_log));
 	if (zfr->bundle) {
+		fill_meta(ctx, zfr);
 		AZ(zip_meta(zft->zip, zfr->name, 3, zfr->mode,
-		    zfr->atime, zfr->mtime));
+			(uint32_t)zfr->atime, (uint32_t)zfr->mtime));
 	}
 
 	AN(priv);

src/vmod_zipflow.vcc

@@ -84,19 +84,30 @@ default per VCL is not currently supported.
 When used elsewhere, sets the level for the current task. Calls from
 ``esi_level`` > 0 have no effect.
 
-$Function VOID meta(STRING name, STRING mode="0644", TIME atime=0, TIME mtime=0)
+$Function VOID meta([STRING name], [STRING mode], [TIME atime], [TIME mtime])
 
-Set zip metadata for the currently streamed object:
+Set zip metadata for the currently streamed object. Defaults apply if
+`zipflow.meta()`_ is not called, or arguments are not given.
 
 * `name`: Filename
 
-* `mode`: UNIX mode bits, default to 0644
+  Default: Last component of ``req.url`` or, if it is empty,
+  ``unnamed_file``.
 
-  Note: This argument is of string type and interpreted as *octal*.
+* `mode`: UNIX mode bits
 
-* `atime`: Access time, defaults to ``now``.
+  This argument is of string type and interpreted as *octal*.
 
-* `mtime`: Modification time, defaults to ``now``.
+  Default: ``0644``
+
+* `atime`: Access time
+
+  Default: ``now``.
+
+* `mtime`: Modification time
+
+  Default: ``Last-Modified`` header of the response or, if it is not
+  available, ``now``.
 
 SEE ALSO
 ========

src/vtc/vmod_zipflow.vtc

 varnishtest "test vmod-zipflow"
 
+feature cmd "type curl && type unzip && echo 'foo' grep -P '^foo'"
+
 server s1 {
 	rxreq
-	txresp -body "zip file content"
+	txresp -bodylen 1024
+
+	rxreq
+	expect req.url == "/path/to/lm?p"
+	txresp -hdr "Last-Modified: Mon, 21 Aug 2023 09:48:03 GMT" -bodylen 512
 } -start
 
 varnish v1 -vcl+backend {
 	import zipflow;
+	import std;
 
 	sub vcl_init {
 		zipflow.set_level(1);
 	}
 
 	sub vcl_deliver {
-		zipflow.set_level(-1);
-		zipflow.meta("filename", mode="444", atime=now, mtime=now - 1d);
+		zipflow.set_level(9);
+		if (req.http.filename) {
+			zipflow.meta(name=req.http.filename);
+		}
+		if (req.http.mode) {
+			zipflow.meta(mode=req.http.mode);
+		}
+		if (req.http.mtime) {
+			zipflow.meta(mtime=std.time(req.http.mtime));
+		}
 		set resp.filters += " zipflow";
 	}
 } -start
@@ -23,6 +38,27 @@ client c1 {
 	txreq
 	rxresp
 	expect resp.status == 200
+
+	txreq -url "/path/to/lm?p"
+	rxresp
+	expect resp.status == 200
+
+	txreq -hdr "mode: 900"
+	rxresp
+	expect resp.status == 503
 } -run
 
-shell "curl -svo t.zip -H 'Host: ${v1_addr}' http://${v1_addr}:${v1_port}/ && unzip -l t.zip"
+# all default
+shell "curl -so t.zip -H 'Host: ${v1_addr}' http://${v1_addr}:${v1_port}/ && unzip -Z t.zip | grep -P '^-rw-r--r--.*\\b1024\\b.*\\b..-...-.. ..:.. unnamed_file$'"
+
+# mtime = last-modified
+shell "curl -so t.zip -H 'Host: ${v1_addr}' http://${v1_addr}:${v1_port}/path/to/lm?p && unzip -Z t.zip | grep -P '^-rw-r--r--.*\\b512\\b.*\\b23-Aug-21 11:48 lm'"
+
+# filename
+shell "curl -so t.zip -H 'Host: ${v1_addr}' -H 'filename: file' http://${v1_addr}:${v1_port}/ && unzip -Z t.zip | grep -P '^-rw-r--r--.*\\b1024\\b.*\\b..-...-.. ..:.. file$'"
+
+# mode
+shell "curl -so t.zip -H 'Host: ${v1_addr}' -H 'mode: 0755' http://${v1_addr}:${v1_port}/ && unzip -Z t.zip | grep -P '^-rwxr-xr-x.*\\b1024\\b.*\\b..-...-.. ..:.. unnamed_file$'"
+
+# mtime
+shell "curl -so t.zip -H 'Host: ${v1_addr}' -H 'mtime: 157590000' http://${v1_addr}:${v1_port}/ && unzip -Z t.zip | grep -P '^-rw-r--r--.*\\b1024\\b.*\\b74-Dec-30 00:00 unnamed_file$'"