[master] 2b2a30c63 vcc: Add infrastructure for protected header fields

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Aug 28 13:02:07 UTC 2023


commit 2b2a30c6338b90e108636533f8e81ed91e85cc5c
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Mon May 30 06:29:41 2022 +0200

    vcc: Add infrastructure for protected header fields

diff --git a/bin/varnishtest/tests/r00694.vtc b/bin/varnishtest/tests/r00694.vtc
index a28f4f124..37d7b4dcd 100644
--- a/bin/varnishtest/tests/r00694.vtc
+++ b/bin/varnishtest/tests/r00694.vtc
@@ -2,22 +2,19 @@ varnishtest "Wrong calculation of last storage segment length"
 
 server s1 {
 	rxreq
-	send "HTTP/1.1 200 OK\r\n"
-	send "Transfer-encoding: chunked\r\n"
-	send "\r\n"
+	txresp -hdr "Transfer-encoding: chunked"
 	# This is chunksize (128k) + 2 to force to chunks to be allocated
 	chunkedlen 131074
 	chunkedlen 0
 } -start
 
-varnish v1 -vcl+backend {
-	sub vcl_deliver {
-		unset resp.http.content-length;
-	}
-} -start
+varnish v1 -vcl+backend "" -start
 
 client c1 {
 	txreq -proto HTTP/1.0
 	rxresp
+	# Chunked encoding streaming HTTP/1.0 client turns
+	# into EOF body delivery.
+	expect resp.http.connection == close
 	expect resp.bodylen == 131074
 } -run
diff --git a/bin/varnishtest/tests/r00801.vtc b/bin/varnishtest/tests/r00801.vtc
deleted file mode 100644
index ca1a56703..000000000
--- a/bin/varnishtest/tests/r00801.vtc
+++ /dev/null
@@ -1,24 +0,0 @@
-varnishtest "Regression test for duplicate content-length in pass"
-
-server s1 {
-	rxreq
-	txresp \
-		-hdr "Date: Mon, 25 Oct 2010 06:34:06 GMT" \
-		-hdr "Content-length: 000010" \
-		-nolen -bodylen 10
-} -start
-
-
-varnish v1 -vcl+backend {
-	sub vcl_recv { return (pass); }
-	sub vcl_backend_response {
-		set beresp.do_stream = false;
-		unset beresp.http.content-length;
-	}
-} -start
-
-client c1 {
-	txreq
-	rxresp
-	expect resp.http.content-length == "10"
-} -run
diff --git a/bin/varnishtest/tests/r00806.vtc b/bin/varnishtest/tests/r00806.vtc
index 25567b58d..f2d682cab 100644
--- a/bin/varnishtest/tests/r00806.vtc
+++ b/bin/varnishtest/tests/r00806.vtc
@@ -14,13 +14,13 @@ varnish v1 -vcl+backend {
 	sub vcl_recv { return(pass);}
 	sub vcl_deliver {
 		set resp.http.CL = resp.http.content-length;
-		unset resp.http.content-length;
 	}
 } -start
 
 client c1 {
 	txreq
-	rxresp -no_obj
+	rxresphdrs
 	expect resp.status == 304
 	expect resp.http.cl == 100
+	expect_close
 } -run
diff --git a/bin/varnishtest/tests/v00001.vtc b/bin/varnishtest/tests/v00001.vtc
index 1abed8076..8bcea8dbc 100644
--- a/bin/varnishtest/tests/v00001.vtc
+++ b/bin/varnishtest/tests/v00001.vtc
@@ -20,6 +20,16 @@ varnish v1 -errvcl "Variable is read only" {
 	sub vcl_backend_response { set beresp.proto = "HTTP/1.2"; }
 }
 
+varnish v1 -errvcl "Variable is read only" {
+	backend proforma None;
+	sub vcl_recv { set req.http.content-length = "42"; }
+}
+
+varnish v1 -errvcl "Variable cannot be unset" {
+	backend proforma None;
+	sub vcl_recv { unset req.http.content-length; }
+}
+
 server s1 {
 	rxreq
 	txresp -hdr "Connection: close" -body "012345\n"
diff --git a/doc/sphinx/reference/vcl-var.rst b/doc/sphinx/reference/vcl-var.rst
index f2d832306..4d38f523f 100644
--- a/doc/sphinx/reference/vcl-var.rst
+++ b/doc/sphinx/reference/vcl-var.rst
@@ -35,6 +35,18 @@ and ``vcl_fini {}``.
 
 .. include:: vcl_var.rst
 
+.. _protected_headers:
+
+Protected header fields
+-----------------------
+
+The ``content-length`` and ``transfer-encoding`` headers are read-only. They
+must be preserved to ensure HTTP/1 framing remains consistent and maintain a
+proper request and response synchronization with both clients and backends.
+
+VMODs can still update these headers, when there is a reason to change the
+framing, such as a transformation of a request or response body.
+
 HTTP response status
 --------------------
 
diff --git a/doc/sphinx/reference/vcl_var.rst b/doc/sphinx/reference/vcl_var.rst
index b8a28c453..c207867e4 100644
--- a/doc/sphinx/reference/vcl_var.rst
+++ b/doc/sphinx/reference/vcl_var.rst
@@ -353,6 +353,27 @@ req.http.*
 	headers present in IANA registries need to be quoted, so the
 	quoted syntax is discouraged but available for interoperability.
 
+	Some headers that cannot be tampered with for proper HTTP fetch
+	or delivery are read-only.
+
+
+req.http.content-length
+
+	Type: HEADER
+
+	Readable from: client
+
+	The content-length header field is protected, see protected_headers_.
+
+
+req.http.transfer-encoding
+
+	Type: HEADER
+
+	Readable from: client
+
+	The transfer-encoding header field is protected, see protected_headers_.
+
 
 .. _req.is_hitmiss:
 
@@ -715,6 +736,24 @@ bereq.http.*
 	See ``req.http.*`` for general notes.
 
 
+bereq.http.content-length
+
+	Type: HEADER
+
+	Readable from: client
+
+	The content-length header field is protected, see protected_headers_.
+
+
+bereq.http.transfer-encoding
+
+	Type: HEADER
+
+	Readable from: client
+
+	The transfer-encoding header field is protected, see protected_headers_.
+
+
 .. _bereq.is_bgfetch:
 
 bereq.is_bgfetch
@@ -1119,6 +1158,24 @@ beresp.http.*
 	See ``req.http.*`` for general notes.
 
 
+beresp.http.content-length
+
+	Type: HEADER
+
+	Readable from: client
+
+	The content-length header field is protected, see protected_headers_.
+
+
+beresp.http.transfer-encoding
+
+	Type: HEADER
+
+	Readable from: client
+
+	The transfer-encoding header field is protected, see protected_headers_.
+
+
 .. _beresp.keep:
 
 beresp.keep
@@ -1551,6 +1608,24 @@ resp.http.*
 	See ``req.http.*`` for general notes.
 
 
+resp.http.content-length
+
+	Type: HEADER
+
+	Readable from: client
+
+	The content-length header field is protected, see protected_headers_.
+
+
+resp.http.transfer-encoding
+
+	Type: HEADER
+
+	Readable from: client
+
+	The transfer-encoding header field is protected, see protected_headers_.
+
+
 .. _resp.is_streaming:
 
 resp.is_streaming
diff --git a/lib/libvcc/generate.py b/lib/libvcc/generate.py
index a62e9427d..fe5bcc9b4 100755
--- a/lib/libvcc/generate.py
+++ b/lib/libvcc/generate.py
@@ -174,20 +174,23 @@ class vardef(object):
             fo.write(" SYM_VAR, %d, %d);\n" % (self.vlo, self.vhi))
         fo.write("\tAN(sym);\n")
         fo.write("\tsym->type = %s;\n" % self.typ)
-        fo.write("\tsym->eval = vcc_Eval_Var;\n")
+        if self.typ == "HEADER" and not var_is_wildcard(self.sym):
+            fo.write("\tsym->eval = vcc_Eval_ProtectedHeader;\n")
+        else:
+            fo.write("\tsym->eval = vcc_Eval_Var;\n")
 
-        if self.typ == "HEADER":
+        if var_is_wildcard(self.sym):
             fo.write('\tsym->rname = "HDR_')
             fo.write(self.nam.split(".")[0].upper())
             fo.write('";\n')
-        elif self.rd:
+        elif self.rd and self.typ != "HEADER":
             fo.write('\tsym->rname = "VRT_r_%s(ctx)";\n' % cnam)
             varproto("VCL_" + self.typ + " VRT_r_%s(VRT_CTX)" % cnam)
         fo.write("\tsym->r_methods =\n")
         restrict(fo, self.rd)
         fo.write(";\n")
 
-        if self.typ == "HEADER":
+        if var_is_wildcard(self.sym):
             fo.write('\tsym->lname = "HDR_')
             fo.write(self.nam.split(".")[0].upper())
             fo.write('";\n')
diff --git a/lib/libvcc/vcc_compile.h b/lib/libvcc/vcc_compile.h
index b99e40fdf..369c66daa 100644
--- a/lib/libvcc/vcc_compile.h
+++ b/lib/libvcc/vcc_compile.h
@@ -355,6 +355,7 @@ sym_act_f vcc_Act_Call;
 sym_act_f vcc_Act_Obj;
 void vcc_Expr_Init(struct vcc *tl);
 sym_expr_t vcc_Eval_Var;
+sym_expr_t vcc_Eval_ProtectedHeader;
 sym_expr_t vcc_Eval_Handle;
 sym_expr_t vcc_Eval_Sub;
 sym_expr_t vcc_Eval_SymFunc;
@@ -455,6 +456,7 @@ char *vcc_Dup_be(const char *b, const char *e);
 int vcc_Has_vcl_prefix(const char *b);
 
 /* vcc_var.c */
+void vcc_Header_Fh(struct vcc *, struct symbol *);
 sym_wildcard_t vcc_Var_Wildcard;
 
 /* vcc_vmod.c */
diff --git a/lib/libvcc/vcc_expr.c b/lib/libvcc/vcc_expr.c
index 63578df9d..1e94dbf76 100644
--- a/lib/libvcc/vcc_expr.c
+++ b/lib/libvcc/vcc_expr.c
@@ -379,6 +379,19 @@ vcc_Eval_Var(struct vcc *tl, struct expr **e, struct token *t,
 		(*e)->fmt = STRINGS;
 }
 
+void v_matchproto_(sym_expr_t)
+vcc_Eval_ProtectedHeader(struct vcc *tl, struct expr **e, struct token *t,
+    struct symbol *sym, vcc_type_t type)
+{
+
+	AN(sym);
+	AZ(sym->lorev);
+
+	vcc_Header_Fh(tl, sym);
+	sym->eval = vcc_Eval_Var;
+	vcc_Eval_Var(tl, e, t, sym, type);
+}
+
 /*--------------------------------------------------------------------
  */
 
diff --git a/lib/libvcc/vcc_symb.c b/lib/libvcc/vcc_symb.c
index 61d05e156..fc273f2de 100644
--- a/lib/libvcc/vcc_symb.c
+++ b/lib/libvcc/vcc_symb.c
@@ -438,6 +438,7 @@ VCC_MkSym(struct vcc *tl, const char *b, vcc_ns_t ns, vcc_kind_t kind,
 {
 	struct symtab *st;
 	struct symbol *sym;
+	const struct symbol *parent;
 
 	AN(tl);
 	AN(b);
@@ -450,6 +451,14 @@ VCC_MkSym(struct vcc *tl, const char *b, vcc_ns_t ns, vcc_kind_t kind,
 	st = vcc_symtab_str(tl->syms[ns->id], b, NULL, ID);
 	AN(st);
 	sym = vcc_sym_in_tab(tl, st, kind, vlo, vhi);
+	if (sym != NULL) {
+		assert(sym->kind == SYM_VAR);
+		parent = sym->eval_priv;
+		AN(parent);
+		AN(parent->wildcard);
+		assert(sym->type == parent->type);
+		return (sym);
+	}
 	AZ(sym);
 	sym = vcc_new_symbol(tl, st, kind, vlo, vhi);
 	AN(sym);
diff --git a/lib/libvcc/vcc_var.c b/lib/libvcc/vcc_var.c
index cfa3fec4c..7292924ae 100644
--- a/lib/libvcc/vcc_var.c
+++ b/lib/libvcc/vcc_var.c
@@ -40,7 +40,7 @@
 
 /*--------------------------------------------------------------------*/
 
-static void
+void
 vcc_Header_Fh(struct vcc *tl, struct symbol *sym)
 {
 	const struct symbol *parent;
@@ -123,5 +123,6 @@ vcc_Var_Wildcard(struct vcc *tl, struct symbol *parent, struct symbol *sym)
 	}
 	VSB_destroy(&vsb);
 
-	vcc_Header_Fh(tl, sym);
+	if (sym->lorev > 0)
+		vcc_Header_Fh(tl, sym);
 }


More information about the varnish-commit mailing list