mime: fix reader stall on small read lengths
The base64 mime encoder stalls when it cannot encode a full 3 byte input set into the read buffer. The workaround for this limitation was incomplete and could lead to stalled transfers when the last chunk to upload was smaller than 4 bytes. Use a tmp buffer on small reads to allow mime encoders more space to put their things. Add test case reproducing the issue and fix. Reported-by: Alexis Savin Fixes #15688 Closes #15691
This commit is contained in:
parent
4bba14c35d
commit
ce949ba1dc
62
lib/mime.c
62
lib/mime.c
@ -1926,6 +1926,7 @@ struct cr_mime_ctx {
|
||||
curl_off_t total_len;
|
||||
curl_off_t read_len;
|
||||
CURLcode error_result;
|
||||
struct bufq tmpbuf;
|
||||
BIT(seen_eos);
|
||||
BIT(errored);
|
||||
};
|
||||
@ -1937,9 +1938,18 @@ static CURLcode cr_mime_init(struct Curl_easy *data,
|
||||
(void)data;
|
||||
ctx->total_len = -1;
|
||||
ctx->read_len = 0;
|
||||
Curl_bufq_init2(&ctx->tmpbuf, 1024, 1, BUFQ_OPT_NO_SPARES);
|
||||
return CURLE_OK;
|
||||
}
|
||||
|
||||
static void cr_mime_close(struct Curl_easy *data,
|
||||
struct Curl_creader *reader)
|
||||
{
|
||||
struct cr_mime_ctx *ctx = reader->ctx;
|
||||
(void)data;
|
||||
Curl_bufq_free(&ctx->tmpbuf);
|
||||
}
|
||||
|
||||
/* Real client reader to installed client callbacks. */
|
||||
static CURLcode cr_mime_read(struct Curl_easy *data,
|
||||
struct Curl_creader *reader,
|
||||
@ -1948,6 +1958,7 @@ static CURLcode cr_mime_read(struct Curl_easy *data,
|
||||
{
|
||||
struct cr_mime_ctx *ctx = reader->ctx;
|
||||
size_t nread;
|
||||
char tmp[256];
|
||||
|
||||
|
||||
/* Once we have errored, we will return the same error forever */
|
||||
@ -1973,18 +1984,46 @@ static CURLcode cr_mime_read(struct Curl_easy *data,
|
||||
blen = (size_t)remain;
|
||||
}
|
||||
|
||||
if(blen <= 4) {
|
||||
/* TODO: Curl_mime_read() may go into an infinite loop when reading
|
||||
* such small lengths. Returning 0 bytes read is a fix that only works
|
||||
* as request upload buffers will get flushed eventually and larger
|
||||
* reads will happen again. */
|
||||
CURL_TRC_READ(data, "cr_mime_read(len=%zu), too small, return", blen);
|
||||
*pnread = 0;
|
||||
*peos = FALSE;
|
||||
goto out;
|
||||
if(!Curl_bufq_is_empty(&ctx->tmpbuf)) {
|
||||
CURLcode result = CURLE_OK;
|
||||
ssize_t n = Curl_bufq_read(&ctx->tmpbuf, (unsigned char *)buf, blen,
|
||||
&result);
|
||||
if(n < 0) {
|
||||
ctx->errored = TRUE;
|
||||
ctx->error_result = result;
|
||||
return result;
|
||||
}
|
||||
nread = (size_t)n;
|
||||
}
|
||||
else if(blen <= 4) {
|
||||
/* Curl_mime_read() may go into an infinite loop when reading
|
||||
* via a base64 encoder, as it stalls when the read buffer is too small
|
||||
* to contain a complete 3 byte encoding. Read into a larger buffer
|
||||
* and use that until empty. */
|
||||
CURL_TRC_READ(data, "cr_mime_read(len=%zu), small read, using tmp", blen);
|
||||
nread = Curl_mime_read(tmp, 1, sizeof(tmp), ctx->part);
|
||||
if(nread <= sizeof(tmp)) {
|
||||
CURLcode result = CURLE_OK;
|
||||
ssize_t n = Curl_bufq_write(&ctx->tmpbuf, (unsigned char *)tmp, nread,
|
||||
&result);
|
||||
if(n < 0) {
|
||||
ctx->errored = TRUE;
|
||||
ctx->error_result = result;
|
||||
return result;
|
||||
}
|
||||
/* stored it, read again */
|
||||
n = Curl_bufq_read(&ctx->tmpbuf, (unsigned char *)buf, blen, &result);
|
||||
if(n < 0) {
|
||||
ctx->errored = TRUE;
|
||||
ctx->error_result = result;
|
||||
return result;
|
||||
}
|
||||
nread = (size_t)n;
|
||||
}
|
||||
}
|
||||
else
|
||||
nread = Curl_mime_read(buf, 1, blen, ctx->part);
|
||||
|
||||
nread = Curl_mime_read(buf, 1, blen, ctx->part);
|
||||
CURL_TRC_READ(data, "cr_mime_read(len=%zu), mime_read() -> %zd",
|
||||
blen, nread);
|
||||
|
||||
@ -2044,7 +2083,6 @@ static CURLcode cr_mime_read(struct Curl_easy *data,
|
||||
break;
|
||||
}
|
||||
|
||||
out:
|
||||
CURL_TRC_READ(data, "cr_mime_read(len=%zu, total=%" FMT_OFF_T
|
||||
", read=%"FMT_OFF_T") -> %d, %zu, %d",
|
||||
blen, ctx->total_len, ctx->read_len, CURLE_OK, *pnread, *peos);
|
||||
@ -2140,7 +2178,7 @@ static const struct Curl_crtype cr_mime = {
|
||||
"cr-mime",
|
||||
cr_mime_init,
|
||||
cr_mime_read,
|
||||
Curl_creader_def_close,
|
||||
cr_mime_close,
|
||||
cr_mime_needs_rewind,
|
||||
cr_mime_total_length,
|
||||
cr_mime_resume_from,
|
||||
|
||||
@ -669,6 +669,25 @@ class TestUpload:
|
||||
])
|
||||
r.check_stats(count=1, http_status=200, exitcode=0)
|
||||
|
||||
# issue #15688 when posting a form and cr_mime_read() is called with
|
||||
# length < 4, we did not progress
|
||||
@pytest.mark.parametrize("proto", ['http/1.1'])
|
||||
def test_07_62_upload_issue_15688(self, env: Env, httpd, proto):
|
||||
# this length leads to (including multipart formatting) to a
|
||||
# client reader invocation with length 1.
|
||||
upload_len = 196169
|
||||
fname = f'data-{upload_len}'
|
||||
env.make_data_file(indir=env.gen_dir, fname=fname, fsize=upload_len)
|
||||
fdata = os.path.join(env.gen_dir, fname)
|
||||
curl = CurlClient(env=env)
|
||||
url = f'https://{env.authority_for(env.domain1, proto)}/curltest/echo?id=[0-0]'
|
||||
r = curl.http_form(urls=[url], form={
|
||||
'file': f'@{fdata}',
|
||||
}, alpn_proto=proto, extra_args=[
|
||||
'--max-time', '10'
|
||||
])
|
||||
r.check_stats(count=1, http_status=200, exitcode=0)
|
||||
|
||||
# nghttpx is the only server we have that supports TLS early data and
|
||||
# has a limit of 16k it announces
|
||||
@pytest.mark.skipif(condition=not Env.have_nghttpx(), reason="no nghttpx")
|
||||
|
||||
Loading…
Reference in New Issue
Block a user