[master] ba59fd2d0 v1f: Read end-of-chunk as part of the chunk

Asad Sajjad Ahmed asadsa at varnish-software.com
Mon May 12 12:57:06 UTC 2025


commit ba59fd2d0dd88022d7390a5d943f458a45fc5e6d
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Mon May 30 13:09:11 2022 +0200

    v1f: Read end-of-chunk as part of the chunk
    
    Until now, we read the (CR)?LF at the end of a chunk as part of the
    next chunk header (see: /* Skip leading whitespace */).
    
    For a follow up commit, we are going to want to know if the next chunk
    header is available for read, so we now consume the chunk end as part
    of the chunk itself.
    
    This also fixes a corner case: We previously accepted chunks with a
    missing end-of-chunk (see fix of r01729.vtc).
    
    Ref: https://datatracker.ietf.org/doc/html/rfc7230#section-4.1

diff --git a/bin/varnishd/http1/cache_http1_vfp.c b/bin/varnishd/http1/cache_http1_vfp.c
index 20f349d1c..aceb5a628 100644
--- a/bin/varnishd/http1/cache_http1_vfp.c
+++ b/bin/varnishd/http1/cache_http1_vfp.c
@@ -89,6 +89,24 @@ v1f_read(const struct vfp_ctx *vc, struct http_conn *htc, void *d, ssize_t len)
 }
 
 
+/*--------------------------------------------------------------------
+ * read (CR)?LF at the end of a chunk
+ */
+static enum vfp_status
+v1f_chunk_end(struct vfp_ctx *vc, struct http_conn *htc)
+{
+	char c;
+
+	if (v1f_read(vc, htc, &c, 1) <= 0)
+		return (VFP_Error(vc, "chunked read err"));
+	if (c == '\r' && v1f_read(vc, htc, &c, 1) <= 0)
+		return (VFP_Error(vc, "chunked read err"));
+	if (c != '\n')
+		return (VFP_Error(vc, "chunked tail no NL"));
+	return (VFP_OK);
+}
+
+
 /*--------------------------------------------------------------------
  * Read a chunked HTTP object.
  *
@@ -99,6 +117,7 @@ static enum vfp_status v_matchproto_(vfp_pull_f)
 v1f_chunked_pull(struct vfp_ctx *vc, struct vfp_entry *vfe, void *ptr,
     ssize_t *lp)
 {
+	static enum vfp_status vfps;
 	struct http_conn *htc;
 	char buf[20];		/* XXX: 20 is arbitrary */
 	char *q;
@@ -168,18 +187,15 @@ v1f_chunked_pull(struct vfp_ctx *vc, struct vfp_entry *vfe, void *ptr,
 			return (VFP_Error(vc, "chunked insufficient bytes"));
 		*lp = lr;
 		vfe->priv2 -= lr;
-		if (vfe->priv2 == 0)
-			vfe->priv2 = -1;
-		return (VFP_OK);
+		if (vfe->priv2 != 0)
+			return (VFP_OK);
+
+		vfe->priv2 = -1;
+		return (v1f_chunk_end(vc, htc));
 	}
 	AZ(vfe->priv2);
-	if (v1f_read(vc, htc, buf, 1) <= 0)
-		return (VFP_Error(vc, "chunked read err"));
-	if (buf[0] == '\r' && v1f_read(vc, htc, buf, 1) <= 0)
-		return (VFP_Error(vc, "chunked read err"));
-	if (buf[0] != '\n')
-		return (VFP_Error(vc, "chunked tail no NL"));
-	return (VFP_END);
+	vfps = v1f_chunk_end(vc, htc);
+	return (vfps == VFP_OK ? VFP_END : vfps);
 }
 
 static const struct vfp v1f_chunked = {
diff --git a/bin/varnishtest/tests/r01184.vtc b/bin/varnishtest/tests/r01184.vtc
index 0988e65a3..94ecd3c23 100644
--- a/bin/varnishtest/tests/r01184.vtc
+++ b/bin/varnishtest/tests/r01184.vtc
@@ -62,6 +62,7 @@ server s1 {
 		sendhex " 10 45 f3 a9 83 b8 18 1c  7b c2 30 55 04 17 13 c4"
 		sendhex " 0f 07 5f 7a 38 f4 8e 50  b3 37 d4 3a 32 4a 34 07"
 		sendhex " FF FF FF FF FF FF FF FF  72 ea 06 5f b3 1c fa dd"
+	send "\n"
 	expect_close
 } -start
 
@@ -93,6 +94,7 @@ server s1 {
 		sendhex " 10 45 f3 a9 83 b8 18 1c  7b c2 30 55 04 17 13 c4"
 		sendhex " 0f 07 5f 7a 38 f4 8e 50  b3 37 d4 3a 32 4a 34 07"
 		sendhex " FF FF FF FF FF FF FF FF  72 ea 06 5f b3 1c fa dd"
+	send "\n"
 	expect_close
 } -start
 
diff --git a/bin/varnishtest/tests/r01506.vtc b/bin/varnishtest/tests/r01506.vtc
index 96b7b54c9..f7f89a716 100644
--- a/bin/varnishtest/tests/r01506.vtc
+++ b/bin/varnishtest/tests/r01506.vtc
@@ -7,15 +7,15 @@ server s0 {
 	txresp -nolen \
 	    -hdr "Transfer-Encoding: chunked" \
 	    -hdr "Connection: close"
-	send "11\r\n0_23456789abcdef\n"
-	send "11\r\n1_23456789abcdef\n"
-	send "11\r\n2_23456789abcdef\n"
-	send "11\r\n3_23456789abcdef\n"
+	send "11\r\n0_23456789abcdef\n\n"
+	send "11\r\n1_23456789abcdef\n\n"
+	send "11\r\n2_23456789abcdef\n\n"
+	send "11\r\n3_23456789abcdef\n\n"
 	barrier b1 sync
-	send "11\r\n4_23456789abcdef\n"
-	send "11\r\n5_23456789abcdef\n"
-	send "11\r\n6_23456789abcdef\n"
-	send "11\r\n7_23456789abcdef\n"
+	send "11\r\n4_23456789abcdef\n\n"
+	send "11\r\n5_23456789abcdef\n\n"
+	send "11\r\n6_23456789abcdef\n\n"
+	send "11\r\n7_23456789abcdef\n\n"
 	chunkedlen 0
 
 } -dispatch
diff --git a/bin/varnishtest/tests/r01729.vtc b/bin/varnishtest/tests/r01729.vtc
index 883a60cc6..f6a01e976 100644
--- a/bin/varnishtest/tests/r01729.vtc
+++ b/bin/varnishtest/tests/r01729.vtc
@@ -11,7 +11,7 @@ server s1 {
 	send "\r\n"
 	send "14\r\n"
 	send "0123456789"
-	send "0123456789"
+	send "0123456789\n"
 	send "0\r\n"
 	send "\r\n"
 
@@ -29,7 +29,7 @@ client c1 {
 	send "\r\n"
 	send "14\r\n"
 	send "0123456789"
-	send "0123456789"
+	send "0123456789\n"
 	send "0\r\n"
 	send "\r\n"
 
@@ -45,7 +45,7 @@ client c1 {
 	send "\r\n"
 	send "14\r\n"
 	send "0123456789"
-	send "0123456789"
+	send "0123456789\n"
 	send "0\r\n"
 	send "\r\n"
 


More information about the varnish-commit mailing list