[master] 5d788a8c0 vrt: Turn hdr_t into a structured type

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Apr 14 15:47:06 UTC 2025


commit 5d788a8c081e3d0f626807773588b640582fc786
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Wed Apr 2 13:49:27 2025 +0200

    vrt: Turn hdr_t into a structured type
    
    Without changing the memory layout of our pseudo-pascal-string for
    header names, this structured alternative clarifies the semantics
    and removes error-prone pointer arithmetics spread out everywhere.
    
    The new structure adds type safety to this specific use case.
    
    It comes with a new static analyzer with the HDR() macro, and a
    CAST_HDR() macro that includes a check with the CHECK_HDR() macro
    that was already present.
    
    Closes #3215

diff --git a/bin/varnishd/cache/cache_ban.c b/bin/varnishd/cache/cache_ban.c
index ce54c5c3d..04468a547 100644
--- a/bin/varnishd/cache/cache_ban.c
+++ b/bin/varnishd/cache/cache_ban.c
@@ -495,6 +495,7 @@ ban_evaluate(struct worker *wrk, const uint8_t *bsarg, struct objcore *oc,
 	const char *p;
 	const char *arg1;
 	double darg1, darg2;
+	hdr_t hdr;
 	int rv;
 
 	/*
@@ -521,11 +522,13 @@ ban_evaluate(struct worker *wrk, const uint8_t *bsarg, struct objcore *oc,
 			break;
 		case BANS_ARG_REQHTTP:
 			AN(reqhttp);
-			(void)http_GetHdr(reqhttp, bt.arg1_spec, &p);
+			CAST_HDR(hdr, bt.arg1_spec);
+			(void)http_GetHdr(reqhttp, hdr, &p);
 			arg1 = p;
 			break;
 		case BANS_ARG_OBJHTTP:
-			arg1 = HTTP_GetHdrPack(wrk, oc, bt.arg1_spec);
+			CAST_HDR(hdr, bt.arg1_spec);
+			arg1 = HTTP_GetHdrPack(wrk, oc, hdr);
 			break;
 		case BANS_ARG_OBJSTATUS:
 			arg1 = HTTP_GetHdrPack(wrk, oc, H__Status);
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 84db8e947..0521d0314 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -64,7 +64,7 @@ struct fetch_step {
 FETCH_STEPS
 #undef FETCH_STEP
 
-static hdr_t const H_X_Varnish = "\012X-Varnish:";
+static hdr_t const H_X_Varnish = HDR("X-Varnish");
 
 /*--------------------------------------------------------------------
  * Allocate an object, with fall-back to Transient.
diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c
index 0cc17d572..f8402458f 100644
--- a/bin/varnishd/cache/cache_http.c
+++ b/bin/varnishd/cache/cache_http.c
@@ -54,14 +54,12 @@
 #include "tbl/body_status.h"
 
 
-#define HTTPH(a, b, c) \
-static char _##b[] = "*" a ":"; \
-hdr_t b = _##b;
+#define HTTPH(a, b, c) hdr_t b = HDR(a);
 #include "tbl/http_headers.h"
 
-hdr_t H__Status	= "\010:status:";
-hdr_t H__Proto	= "\007:proto:";
-hdr_t H__Reason	= "\010:reason:";
+hdr_t H__Status	= HDR(":status");
+hdr_t H__Proto	= HDR(":proto");
+hdr_t H__Reason	= HDR(":reason");
 
 static char * via_hdr;
 
@@ -208,12 +206,11 @@ http_hdr_flags(const char *b, const char *e)
 /*--------------------------------------------------------------------*/
 
 static void
-http_init_hdr(char *hdr, int flg)
+http_init_hdr(hdr_t hdr, int flg)
 {
 	struct http_hdrflg *f;
 
-	hdr[0] = strlen(hdr + 1);
-	f = http_hdr_flags(hdr + 1, hdr + hdr[0]);
+	f = http_hdr_flags(hdr->str, hdr->str + hdr->len - 1);
 	AN(f);
 	assert(*f->hdr == hdr);
 	f->flag = flg;
@@ -224,7 +221,7 @@ HTTP_Init(void)
 {
 	struct vsb *vsb;
 
-#define HTTPH(a, b, c) http_init_hdr(TRUST_ME(b), c);
+#define HTTPH(a, b, c) http_init_hdr(b, c);
 #include "tbl/http_headers.h"
 
 	vsb = VSB_new_auto();
@@ -479,13 +476,10 @@ http_PutField(struct http *to, int field, const char *string)
 int
 http_IsHdr(const txt *hh, hdr_t hdr)
 {
-	unsigned l;
 
 	Tcheck(*hh);
 	CHECK_HDR(hdr);
-	l = hdr[0];
-	hdr++;
-	return (http_hdr_at(hdr, hh->b, l));
+	return (http_hdr_at(hdr->str, hh->b, hdr->len));
 }
 
 /*--------------------------------------------------------------------*/
@@ -549,7 +543,7 @@ http_CollectHdr(struct http *hp, hdr_t hdr)
 void
 http_CollectHdrSep(struct http *hp, hdr_t hdr, const char *sep)
 {
-	unsigned u, l, lsep, ml, f, x, d;
+	unsigned u, lsep, ml, f, x, d;
 	char *b = NULL, *e = NULL;
 	const char *v;
 
@@ -563,8 +557,7 @@ http_CollectHdrSep(struct http *hp, hdr_t hdr, const char *sep)
 		sep = ", ";
 	lsep = strlen(sep);
 
-	l = hdr[0];
-	f = http_findhdr(hp, l - 1, hdr + 1);
+	f = http_findhdr(hp, hdr->len - 1, hdr->str);
 	if (f == 0)
 		return;
 
@@ -587,7 +580,7 @@ http_CollectHdrSep(struct http *hp, hdr_t hdr, const char *sep)
 			if (b + x >= e) {
 				http_fail(hp);
 				VSLbs(hp->vsl, SLT_LostHeader,
-				    TOSTRAND(hdr + 1));
+				    TOSTRAND(hdr->str));
 				WS_Release(hp->ws, 0);
 				return;
 			}
@@ -599,9 +592,9 @@ http_CollectHdrSep(struct http *hp, hdr_t hdr, const char *sep)
 		AN(e);
 
 		/* Append the Nth header we found */
-		x = Tlen(hp->hd[u]) - l;
+		x = Tlen(hp->hd[u]) - hdr->len;
 
-		v = hp->hd[u].b + *hdr;
+		v = hp->hd[u].b + hdr->len;
 		while (vct_issp(*v)) {
 			v++;
 			x--;
@@ -609,7 +602,7 @@ http_CollectHdrSep(struct http *hp, hdr_t hdr, const char *sep)
 
 		if (b + lsep + x >= e) {
 			http_fail(hp);
-			VSLbs(hp->vsl, SLT_LostHeader, TOSTRAND(hdr + 1));
+			VSLbs(hp->vsl, SLT_LostHeader, TOSTRAND(hdr->str));
 			WS_Release(hp->ws, 0);
 			return;
 		}
@@ -633,20 +626,18 @@ http_CollectHdrSep(struct http *hp, hdr_t hdr, const char *sep)
 int
 http_GetHdr(const struct http *hp, hdr_t hdr, const char **ptr)
 {
-	unsigned u, l;
+	unsigned u;
 	const char *p;
 
 	CHECK_HDR(hdr);
-	l = hdr[0];
-	hdr++;
-	u = http_findhdr(hp, l - 1, hdr);
+	u = http_findhdr(hp, hdr->len - 1, hdr->str);
 	if (u == 0) {
 		if (ptr != NULL)
 			*ptr = NULL;
 		return (0);
 	}
 	if (ptr != NULL) {
-		p = hp->hd[u].b + l;
+		p = hp->hd[u].b + hdr->len;
 		while (vct_issp(*p))
 			p++;
 		*ptr = p;
@@ -1366,36 +1357,32 @@ const char *
 HTTP_GetHdrPack(struct worker *wrk, struct objcore *oc, hdr_t hdr)
 {
 	const char *ptr;
-	unsigned l;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
 	CHECK_HDR(hdr);
 
-	l = hdr[0];
-	hdr++;
-
-	if (hdr[0] == ':') {
+	if (hdr->str[0] == ':') {
 		/* Special cases */
 		ptr = ObjGetAttr(wrk, oc, OA_HEADERS, NULL);
 		AN(ptr);
 		ptr += 4;	/* Skip nhd and status */
 
 		/* XXX: should we also have h2_hdr_eq() ? */
-		if (!strcmp(hdr, ":proto:"))
+		if (!strcmp(hdr->str, ":proto:"))
 			return (ptr);
 		ptr = strchr(ptr, '\0') + 1;
-		if (!strcmp(hdr, ":status:"))
+		if (!strcmp(hdr->str, ":status:"))
 			return (ptr);
 		ptr = strchr(ptr, '\0') + 1;
-		if (!strcmp(hdr, ":reason:"))
+		if (!strcmp(hdr->str, ":reason:"))
 			return (ptr);
 		WRONG("Unknown magic packed header");
 	}
 
 	HTTP_FOREACH_PACK(wrk, oc, ptr) {
-		if (http_hdr_at(ptr, hdr, l)) {
-			ptr += l;
+		if (http_hdr_at(ptr, hdr->str, hdr->len)) {
+			ptr += hdr->len;
 			while (vct_islws(*ptr))
 				ptr++;
 			return (ptr);
@@ -1544,7 +1531,7 @@ http_ForceHeader(struct http *to, hdr_t hdr, const char *val)
 	if (http_HdrIs(to, hdr, val))
 		return;
 	http_Unset(to, hdr);
-	http_PrintfHeader(to, "%s %s", hdr + 1, val);
+	http_PrintfHeader(to, "%s %s", hdr->str, val);
 }
 
 void
@@ -1555,9 +1542,9 @@ http_AppendHeader(struct http *to, hdr_t hdr, const char *val)
 	http_CollectHdr(to, hdr);
 	if (http_GetHdr(to, hdr, &old)) {
 		http_Unset(to, hdr);
-		http_PrintfHeader(to, "%s %s, %s", hdr + 1, old, val);
+		http_PrintfHeader(to, "%s %s, %s", hdr->str, old, val);
 	} else {
-		http_PrintfHeader(to, "%s %s", hdr + 1, val);
+		http_PrintfHeader(to, "%s %s", hdr->str, val);
 	}
 }
 
diff --git a/bin/varnishd/cache/cache_vary.c b/bin/varnishd/cache/cache_vary.c
index fa9053966..c9c46231b 100644
--- a/bin/varnishd/cache/cache_vary.c
+++ b/bin/varnishd/cache/cache_vary.c
@@ -81,6 +81,7 @@ VRY_Create(struct busyobj *bo, struct vsb **psb)
 {
 	const char *v, *p, *q, *h, *e;
 	struct vsb *sb, *sbh;
+	hdr_t hdr;
 	unsigned l;
 	int error = 0;
 
@@ -122,8 +123,9 @@ VRY_Create(struct busyobj *bo, struct vsb **psb)
 		AZ(VSB_printf(sbh, "%c%.*s:%c",
 		    (char)(1 + (q - p)), (int)(q - p), p, 0));
 		AZ(VSB_finish(sbh));
+		CAST_HDR(hdr, VSB_data(sbh));
 
-		if (http_GetHdr(bo->bereq, VSB_data(sbh), &h)) {
+		if (http_GetHdr(bo->bereq, hdr, &h)) {
 			AZ(vct_issp(*h));
 			/* Trim trailing space */
 			e = strchr(h, '\0');
@@ -292,6 +294,7 @@ VRY_Match(const struct req *req, const uint8_t *vary)
 	const char *h, *e;
 	unsigned lh, ln;
 	int i, oflo = 0;
+	hdr_t hdr;
 
 	AN(vsp);
 	AN(vary);
@@ -310,8 +313,9 @@ VRY_Match(const struct req *req, const uint8_t *vary)
 			 * then compare again with that new entry.
 			 */
 
+			CAST_HDR(hdr, vary + 2);
 			ln = 2 + vary[2] + 2;
-			i = http_GetHdr(req->http, (const char*)(vary+2), &h);
+			i = http_GetHdr(req->http, hdr, &h);
 			if (i) {
 				/* Trim trailing space */
 				e = strchr(h, '\0');
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 39c8ef53c..fce89425d 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -604,11 +604,11 @@ VRT_SetHdr(VRT_CTX, VCL_HEADER hs, const char *pfx, VCL_STRANDS s)
 
 	u = WS_ReserveAll(hp->ws);
 	pl = (pfx == NULL) ? 0 : strlen(pfx);
-	l = hs->what[0] + 1 + pl;
+	l = hs->what->len + 1 + pl;
 	if (u <= l) {
 		WS_Release(hp->ws, 0);
 		WS_MarkOverflow(hp->ws);
-		VSLbs(ctx->vsl, SLT_LostHeader, TOSTRAND(hs->what + 1));
+		VSLbs(ctx->vsl, SLT_LostHeader, TOSTRAND(hs->what->str));
 		return;
 	}
 	b = WS_Reservation(hp->ws);
@@ -618,15 +618,15 @@ VRT_SetHdr(VRT_CTX, VCL_HEADER hs, const char *pfx, VCL_STRANDS s)
 			WS_Release(hp->ws, 0);
 			WS_MarkOverflow(hp->ws);
 			VSLbs(ctx->vsl, SLT_LostHeader,
-			    TOSTRAND(hs->what + 1));
+			    TOSTRAND(hs->what->str));
 			return;
 		}
 	} else {
 		b[l] = '\0';
 	}
 	p = b;
-	memcpy(p, hs->what + 1, hs->what[0]);
-	p += hs->what[0];
+	memcpy(p, hs->what->str, hs->what->len);
+	p += hs->what->len;
 	*p++ = ' ';
 	if (pfx != NULL)
 		memcpy(p, pfx, pl);
diff --git a/bin/varnishtest/tests/r01406.vtc b/bin/varnishtest/tests/r01406.vtc
index 1d090201b..76af91952 100644
--- a/bin/varnishtest/tests/r01406.vtc
+++ b/bin/varnishtest/tests/r01406.vtc
@@ -10,7 +10,7 @@ varnish v1 -arg "-p vcc_feature=+allow_inline_c" -vcl+backend {
 
 	C{
 		static const struct gethdr_s VGC_HDR_REQ_foo =
-		    { HDR_REQ, "\020X-CUSTOM-HEADER:"};
+		    { HDR_REQ, HDR("X-CUSTOM-HEADER")};
 	}C
 
 	sub vcl_recv {
diff --git a/include/vrt.h b/include/vrt.h
index 326611273..59d461087 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -57,6 +57,9 @@
  * Whenever something is deleted or changed in a way which is not
  * binary/load-time compatible, increment MAJOR version
  *
+ * XX.X (unreleased)
+ *	typedef hdr_t added
+ *	struct gethdr_s.what changed to hdr_t
  * 21.0 (2025-03-17)
  *	VRT_u_req_grace() added
  *	VRT_u_req_ttl() added
@@ -693,15 +696,29 @@ enum gethdr_e {
 	HDR_BERESP
 };
 
-typedef const char *hdr_t;
+typedef const struct {
+	unsigned char	len;
+	const char	str[];
+} *hdr_t;
+
+#define HDR(name)							\
+	((hdr_t)&(const struct {					\
+		unsigned char _l;					\
+		char _s[sizeof(name ":")];				\
+	}){ sizeof(name), name ":" })
+
+#define CHECK_HDR(hdr)							\
+	do {								\
+		AN(hdr);						\
+		assert((hdr)->len > 0);					\
+		assert((hdr)->len == strlen((hdr)->str));		\
+		assert((hdr)->str[(hdr)->len - 1] == ':');		\
+	} while (0)
 
-#define CHECK_HDR(hdr)					\
-	do {						\
-		AN(hdr);				\
-		unsigned _l = hdr[0];			\
-		assert(_l > 0);				\
-		assert(_l == strlen(hdr + 1));		\
-		assert(hdr[_l] == ':');			\
+#define CAST_HDR(hdr, str)						\
+	do {								\
+		hdr = (hdr_t)(str);					\
+		CHECK_HDR(hdr);						\
 	} while (0)
 
 struct gethdr_s {
diff --git a/lib/libvcc/vcc_var.c b/lib/libvcc/vcc_var.c
index 643af2f1e..8f0892e05 100644
--- a/lib/libvcc/vcc_var.c
+++ b/lib/libvcc/vcc_var.c
@@ -57,8 +57,7 @@ vcc_Header_Fh(const struct vcc *tl, const struct symbol *sym)
 
 	/* Create the static identifier */
 	Fh(tl, 0, "static const struct gethdr_s %s =\n", sym->rname + 1);
-	Fh(tl, 0, "    { %s, \"\\%03o%s:\"};\n",
-	    parent->rname, (unsigned int)strlen(sym->name) + 1, sym->name);
+	Fh(tl, 0, "    { %s, HDR(\"%s\")};\n", parent->rname, sym->name);
 }
 
 /*--------------------------------------------------------------------*/
diff --git a/vmod/vmod_debug.c b/vmod/vmod_debug.c
index 7b22d87df..73155871a 100644
--- a/vmod/vmod_debug.c
+++ b/vmod/vmod_debug.c
@@ -685,10 +685,10 @@ xyzzy_sethdr(VRT_CTX, VCL_HEADER hdr, VCL_STRANDS s)
 	if (s->n == 0) {
 		http_Unset(hp, hdr->what);
 	} else {
-		b = VRT_StrandsWS(hp->ws, hdr->what + 1, s);
+		b = VRT_StrandsWS(hp->ws, hdr->what->str, s);
 		if (b == NULL) {
 			VSLbs(ctx->vsl, SLT_LostHeader,
-			    TOSTRAND(hdr->what + 1));
+			    TOSTRAND(hdr->what->str));
 		} else {
 			if (*b != '\0')
 				AN(WS_Allocated(hp->ws, b, strlen(b) + 1));


More information about the varnish-commit mailing list