From: Stefan Eissing <stefan@eissing.org>
Date: Thu, 5 Dec 2024 11:37:38 +0000 (+0100)
Subject: mime: fix reader stall on small read lengths
X-Git-Tag: curl-8_11_1~12
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ce949ba1dce08cf0de141b5f82309958e0086367;p=thirdparty%2Fcurl.git

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
---

diff --git a/lib/mime.c b/lib/mime.c
index 70286069ce..21c40b06cb 100644
--- a/lib/mime.c
+++ b/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,
diff --git a/tests/http/test_07_upload.py b/tests/http/test_07_upload.py
index 2ce2707cc1..dcafe7c064 100644
--- a/tests/http/test_07_upload.py
+++ b/tests/http/test_07_upload.py
@@ -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")