[6.0] 7e51625d3 h2: Manage missing :scheme as a custom error

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Thu Apr 4 14:33:08 UTC 2024


commit 7e51625d384f4afdf5f7e258b6881fe5c47525e3
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Mon Dec 4 18:30:06 2023 +0100

    h2: Manage missing :scheme as a custom error
    
    There is room for further improvement in the dynamic between HPACK and
    the HTTP/2 session, but this will serve as the first step.
    
    Conflicts:
            include/tbl/h2_error.h

diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index 201875409..53339dbe5 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -221,14 +221,12 @@ vtr_deliver_f h2_deliver;
 vtr_minimal_response_f h2_minimal_response;
 #endif /* TRANSPORT_MAGIC */
 
-#define H2H_DECODE_FLAG_SCHEME_SEEN	0x1
-
 /* http2/cache_http2_hpack.c */
 struct h2h_decode {
 	unsigned			magic;
 #define H2H_DECODE_MAGIC		0xd092bde4
 
-	int				flags;
+	unsigned			has_scheme:1;
 	h2_error			error;
 	enum vhd_ret_e			vhd_ret;
 	char				*out;
diff --git a/bin/varnishd/http2/cache_http2_hpack.c b/bin/varnishd/http2/cache_http2_hpack.c
index e5fcac7af..e12562523 100644
--- a/bin/varnishd/http2/cache_http2_hpack.c
+++ b/bin/varnishd/http2/cache_http2_hpack.c
@@ -90,7 +90,8 @@ h2h_checkhdr(const struct http *hp, const char *b, size_t namelen, size_t len)
 }
 
 static h2_error
-h2h_addhdr(struct http *hp, char *b, size_t namelen, size_t len, int *flags)
+h2h_addhdr(struct h2h_decode *d, struct http *hp, char *b, size_t namelen,
+    size_t len)
 {
 	/* XXX: This might belong in cache/cache_http.c */
 	const char *b0;
@@ -142,7 +143,7 @@ h2h_addhdr(struct http *hp, char *b, size_t namelen, size_t len, int *flags)
 			/* XXX: What to do about this one? (typically
 			   "http" or "https"). For now set it as a normal
 			   header, stripping the first ':'. */
-			if (*flags & H2H_DECODE_FLAG_SCHEME_SEEN) {
+			if (d->has_scheme) {
 				VSLb(hp->vsl, SLT_BogoHeader,
 				    "Duplicate pseudo-header %.*s%.*s",
 				    (int)namelen, b0,
@@ -153,7 +154,7 @@ h2h_addhdr(struct http *hp, char *b, size_t namelen, size_t len, int *flags)
 			b++;
 			len-=1;
 			n = hp->nhd;
-			*flags |= H2H_DECODE_FLAG_SCHEME_SEEN;
+			d->has_scheme = 1;
 
 			for (p = b + namelen, u = 0; u < len-namelen;
 			    p++, u++) {
@@ -256,6 +257,9 @@ h2h_decode_fini(const struct h2_sess *h2)
 		VSLb(h2->new_req->http->vsl, SLT_BogoHeader,
 		    "HPACK compression error/fini (%s)", VHD_Error(d->vhd_ret));
 		ret = H2CE_COMPRESSION_ERROR;
+	} else if (d->error == NULL && !d->has_scheme) {
+		VSLb(h2->vsl, SLT_Debug, "Missing :scheme");
+		ret = H2SE_MISSING_SCHEME; //rfc7540,l,3087,3090
 	} else
 		ret = d->error;
 	d->magic = 0;
@@ -339,8 +343,8 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
 			    d->out_u);
 			if (d->error)
 				break;
-			d->error = h2h_addhdr(hp, d->out, d->namelen, d->out_u,
-			    &d->flags);
+			d->error = h2h_addhdr(d, hp, d->out,
+			    d->namelen, d->out_u);
 			if (d->error)
 				break;
 			d->out[d->out_u++] = '\0'; /* Zero guard */
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index cc0335144..4d2d7134b 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -619,13 +619,11 @@ static h2_error
 h2_end_headers(struct worker *wrk, struct h2_sess *h2,
     struct req *req, struct h2_req *r2)
 {
-	int scheme_seen;
 	h2_error h2e;
 	ssize_t cl;
 
 	ASSERT_RXTHR(h2);
 	assert(r2->state == H2_S_OPEN);
-	scheme_seen = h2->decode->flags & H2H_DECODE_FLAG_SCHEME_SEEN;
 	h2e = h2h_decode_fini(h2);
 	h2->new_req = NULL;
 	if (r2->req->req_body_status == REQ_BODY_NONE) {
@@ -687,11 +685,6 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2,
 		return (H2SE_PROTOCOL_ERROR); //rfc7540,l,3087,3090
 	}
 
-	if (!(scheme_seen)) {
-		VSLb(h2->vsl, SLT_Debug, "Missing :scheme");
-		return (H2SE_PROTOCOL_ERROR); //rfc7540,l,3087,3090
-	}
-
 	AN(req->http->hd[HTTP_HDR_PROTO].b);
 
 	req->req_step = R_STP_TRANSPORT;
diff --git a/include/tbl/h2_error.h b/include/tbl/h2_error.h
index 6c14a909b..49436f861 100644
--- a/include/tbl/h2_error.h
+++ b/include/tbl/h2_error.h
@@ -169,6 +169,15 @@ H2_ERROR(
 	/* descr */	"http/2 rapid reset detected"
 )
 
+H2_ERROR(
+	/* name */	MISSING_SCHEME,
+	/* val */	1, /* PROTOCOL_ERROR */
+	/* types */	2,
+	/* goaway */	1,
+	/* reason */	SC_NULL,
+	/* descr */	"Missing :scheme pseudo-header"
+)
+
 H2_ERROR(
 	/* name */	BROKE_WINDOW,
 	/* val */	8, /* CANCEL */


More information about the varnish-commit mailing list