Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send VFP_END with last data for v1f chunked & read ahead #3809

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Send VFP_END with last data for v1f chunked & read ahead #3809

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
870f16c
Avoid panic in VRT_CacheReqBody if called from vcl_init{}
nigoroll Jun 24, 2022
0c85ddc
V1F: Read end-of-chunk as part of the chunk
nigoroll May 30, 2022
1a3f9fc
V1F: pull chunk header parsing into an own function
nigoroll May 30, 2022
0262661
V1F: Send VFP_END with last data for v1f chunked
nigoroll May 30, 2022
12e0619
Stabilize partial write -sdeprecated_persistent test
nigoroll May 30, 2022
70fa9c9
V1F: Add state and readahead buffer for v1f chunked processing
nigoroll May 31, 2022
f564ab6
V1F: Refactor v1f_read() to use scatter/gather reads (readv())
nigoroll May 31, 2022
fdf390c
V1F: Readahead for v1f_chunked_*
nigoroll May 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 72 additions & 51 deletions bin/varnishd/http1/cache_http1_vfp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,77 +108,98 @@ v1f_chunk_end(struct vfp_ctx *vc, struct http_conn *htc)


/*--------------------------------------------------------------------
* Read a chunked HTTP object.
* Parse a chunk header and, for VFP_OK, return size in a pointer
*
* XXX: Reading one byte at a time is pretty pessimal.
*/

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
v1f_chunked_hdr(struct vfp_ctx *vc, struct http_conn *htc, ssize_t *szp)
{
static enum vfp_status vfps;
struct http_conn *htc;
char buf[20]; /* XXX: 20 is arbitrary */
char *q;
unsigned u;
uintmax_t cll;
ssize_t cl, l, lr;
ssize_t cl, lr;
char *q;

CHECK_OBJ_NOTNULL(vc, VFP_CTX_MAGIC);
CHECK_OBJ_NOTNULL(vfe, VFP_ENTRY_MAGIC);
CAST_OBJ_NOTNULL(htc, vfe->priv1, HTTP_CONN_MAGIC);
AN(ptr);
AN(lp);
CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
AN(szp);
assert(*szp == -1);

l = *lp;
*lp = 0;
if (vfe->priv2 == -1) {
/* Skip leading whitespace */
do {
lr = v1f_read(vc, htc, buf, 1);
if (lr <= 0)
return (VFP_Error(vc, "chunked read err"));
} while (vct_islws(buf[0]));

if (!vct_ishex(buf[0]))
return (VFP_Error(vc, "chunked header non-hex"));

/* Collect hex digits, skipping leading zeros */
for (u = 1; u < sizeof buf; u++) {
do {
lr = v1f_read(vc, htc, buf + u, 1);
if (lr <= 0)
return (VFP_Error(vc,
"chunked read err"));
} while (u == 1 && buf[0] == '0' && buf[u] == '0');
if (!vct_ishex(buf[u]))
break;
}
/* Skip leading whitespace */
do {
lr = v1f_read(vc, htc, buf, 1);
if (lr <= 0)
return (VFP_Error(vc, "chunked read err"));
} while (vct_islws(buf[0]));

if (u >= sizeof buf)
return (VFP_Error(vc, "chunked header too long"));
if (!vct_ishex(buf[0]))
return (VFP_Error(vc, "chunked header non-hex"));

/* Skip trailing white space */
while (vct_islws(buf[u]) && buf[u] != '\n') {
/* Collect hex digits, skipping leading zeros */
for (u = 1; u < sizeof buf; u++) {
do {
lr = v1f_read(vc, htc, buf + u, 1);
if (lr <= 0)
return (VFP_Error(vc, "chunked read err"));
}
} while (u == 1 && buf[0] == '0' && buf[u] == '0');
if (!vct_ishex(buf[u]))
break;
}

if (buf[u] != '\n')
return (VFP_Error(vc, "chunked header no NL"));
if (u >= sizeof buf)
return (VFP_Error(vc, "chunked header too long"));

/* Skip trailing white space */
while (vct_islws(buf[u]) && buf[u] != '\n') {
lr = v1f_read(vc, htc, buf + u, 1);
if (lr <= 0)
return (VFP_Error(vc, "chunked read err"));
}

buf[u] = '\0';
if (buf[u] != '\n')
return (VFP_Error(vc, "chunked header no NL"));

cll = strtoumax(buf, &q, 16);
if (q == NULL || *q != '\0')
return (VFP_Error(vc, "chunked header number syntax"));
cl = (ssize_t)cll;
if (cl < 0 || (uintmax_t)cl != cll)
return (VFP_Error(vc, "bogusly large chunk size"));
buf[u] = '\0';

vfe->priv2 = cl;
cll = strtoumax(buf, &q, 16);
if (q == NULL || *q != '\0')
return (VFP_Error(vc, "chunked header number syntax"));
cl = (ssize_t)cll;
if (cl < 0 || (uintmax_t)cl != cll)
return (VFP_Error(vc, "bogusly large chunk size"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VNUM_hex()? I'm aware you didn't really touch this part, I simply noticed.

Copy link
Member Author

@nigoroll nigoroll Jul 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no opinion. Why do we even have vnum_uint () and do not just use strtoumax()?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To optionally work with [b..e) ranges, not just null-terminated strings.


*szp = cl;
return (VFP_OK);
}


/*--------------------------------------------------------------------
* Read a chunked HTTP object.
*
*/

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;
ssize_t l, lr;

CHECK_OBJ_NOTNULL(vc, VFP_CTX_MAGIC);
CHECK_OBJ_NOTNULL(vfe, VFP_ENTRY_MAGIC);
CAST_OBJ_NOTNULL(htc, vfe->priv1, HTTP_CONN_MAGIC);
AN(ptr);
AN(lp);

l = *lp;
*lp = 0;
if (vfe->priv2 == -1) {
vfps = v1f_chunked_hdr(vc, htc, &vfe->priv2);
if (vfps != VFP_OK)
return (vfps);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should introduce a macro for this magic -1.

if (vfe->priv2 > 0) {
if (vfe->priv2 < l)
Expand Down