Skip to content

Commit

Permalink
V1F: Read end-of-chunk as part of the chunk
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nigoroll committed Jun 24, 2022
1 parent 870f16c commit 0c85ddc
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 13 deletions.
36 changes: 26 additions & 10 deletions bin/varnishd/http1/cache_http1_vfp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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;
Expand Down Expand Up @@ -169,18 +188,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 = {
Expand Down
2 changes: 2 additions & 0 deletions bin/varnishtest/tests/r01184.vtc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions bin/varnishtest/tests/r01729.vtc
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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"

Expand All @@ -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"

Expand Down

0 comments on commit 0c85ddc

Please sign in to comment.