[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