Mailing List Archive

commit cdcae28a960be46ddf9cc77c9d70bc9ceae873c5
Author: Dridi Boukelmoune <dridi.boukelmoune@gmail.com>
Date: Thu Mar 28 14:13:36 2024 +0100

http2_hpack: Also rewrite h2h_checkhdr() for clarity

It does a first pass on header names and values, and only logs errors,
so the signature is updated accordingly and the call site is moved into
h2h_addhdr().

diff --git a/bin/varnishd/http2/cache_http2_hpack.c b/bin/varnishd/http2/cache_http2_hpack.c
index c2013c1f2..531c50307 100644
--- a/bin/varnishd/http2/cache_http2_hpack.c
+++ b/bin/varnishd/http2/cache_http2_hpack.c
@@ -53,9 +53,10 @@ h2h_assert_ready(struct h2h_decode *d)

// rfc9113,l,2493,2528
static h2_error
-h2h_checkhdr(const struct http *hp, const char *b, size_t namelen, size_t len)
+h2h_checkhdr(struct vsl_log *vsl, txt nm, txt val)
{
const char *p;
+ int l;
enum {
FLD_NAME_FIRST,
FLD_NAME,
@@ -63,23 +64,17 @@ h2h_checkhdr(const struct http *hp, const char *b, size_t namelen, size_t len)
FLD_VALUE
} state;

- CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
- AN(b);
- assert(namelen >= 2); /* 2 chars from the ': ' that we added */
- assert(namelen <= len);
- assert(b[namelen - 2] == ':');
- assert(b[namelen - 1] == ' ');
-
- if (namelen == 2) {
- VSLb(hp->vsl, SLT_BogoHeader, "Empty name");
+ if (Tlen(nm) == 0) {
+ VSLb(vsl, SLT_BogoHeader, "Empty name");
return (H2SE_PROTOCOL_ERROR);
}

- // VSLb(hp->vsl, SLT_Debug, "CHDR [%.*s] [%.*s]",
- // (int)namelen, b, (int)(len - namelen), b + namelen);
+ // VSLb(vsl, SLT_Debug, "CHDR [%.*s] [%.*s]",
+ // (int)Tlen(nm), nm.b, (int)Tlen(val), val.b);

+ l = vmin_t(int, Tlen(nm) + 2 + Tlen(val), 20);
state = FLD_NAME_FIRST;
- for (p = b; p < b + namelen - 2; p++) {
+ for (p = nm.b; p < nm.e; p++) {
switch(state) {
case FLD_NAME_FIRST:
state = FLD_NAME;
@@ -88,15 +83,15 @@ h2h_checkhdr(const struct http *hp, const char *b, size_t namelen, size_t len)
/* FALL_THROUGH */
case FLD_NAME:
if (isupper(*p)) {
- VSLb(hp->vsl, SLT_BogoHeader,
+ VSLb(vsl, SLT_BogoHeader,
"Illegal field header name (upper-case): %.*s",
- (int)(len > 20 ? 20 : len), b);
+ l, nm.b);
return (H2SE_PROTOCOL_ERROR);
}
if (!vct_istchar(*p) || *p == ':') {
- VSLb(hp->vsl, SLT_BogoHeader,
+ VSLb(vsl, SLT_BogoHeader,
"Illegal field header name (non-token): %.*s",
- (int)(len > 20 ? 20 : len), b);
+ l, nm.b);
return (H2SE_PROTOCOL_ERROR);
}
break;
@@ -106,22 +101,20 @@ h2h_checkhdr(const struct http *hp, const char *b, size_t namelen, size_t len)
}

state = FLD_VALUE_FIRST;
- for (p = b + namelen; p < b + len; p++) {
+ for (p = val.b; p < val.e; p++) {
switch(state) {
case FLD_VALUE_FIRST:
if (vct_issp(*p)) {
- VSLb(hp->vsl, SLT_BogoHeader,
- "Illegal field value start %.*s",
- (int)(len > 20 ? 20 : len), b);
+ VSLb(vsl, SLT_BogoHeader,
+ "Illegal field value start %.*s", l, nm.b);
return (H2SE_PROTOCOL_ERROR);
}
state = FLD_VALUE;
/* FALL_THROUGH */
case FLD_VALUE:
if (!vct_ishdrval(*p)) {
- VSLb(hp->vsl, SLT_BogoHeader,
- "Illegal field value %.*s",
- (int)(len > 20 ? 20 : len), b);
+ VSLb(vsl, SLT_BogoHeader,
+ "Illegal field value %.*s", l, nm.b);
return (H2SE_PROTOCOL_ERROR);
}
break;
@@ -129,10 +122,9 @@ h2h_checkhdr(const struct http *hp, const char *b, size_t namelen, size_t len)
WRONG("http2 field value validation state");
}
}
- if (state == FLD_VALUE && vct_issp(b[len - 1])) {
- VSLb(hp->vsl, SLT_BogoHeader,
- "Illegal field value (end) %.*s",
- (int)(len > 20 ? 20 : len), b);
+ if (state == FLD_VALUE && vct_issp(val.e[-1])) {
+ VSLb(vsl, SLT_BogoHeader,
+ "Illegal field value (end) %.*s", l, nm.b);
return (H2SE_PROTOCOL_ERROR);
}
return (0);
@@ -146,6 +138,7 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
int disallow_empty;
const char *p;
unsigned n, has_dup;
+ h2_error err;

CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
h2h_assert_ready(d);
@@ -161,6 +154,10 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
val.b = nm.e + 2;
val.e = hdr.e;

+ err = h2h_checkhdr(hp->vsl, nm, val);
+ if (err != NULL)
+ return (err);
+
disallow_empty = 0;
has_dup = 0;

@@ -393,10 +390,6 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
d->error = H2SE_ENHANCE_YOUR_CALM;
break;
}
- d->error = h2h_checkhdr(hp, d->out, d->namelen,
- d->out_u);
- if (d->error)
- break;
d->error = h2h_addhdr(hp, d);
if (d->error)
break;
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit