From: Stefan Eissing <stefan@eissing.org>
Date: Thu, 3 Oct 2024 08:15:07 +0000 (+0200)
Subject: bufq: unwrite fix
X-Git-Tag: curl-8_11_0~247
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2400a6c6b21a65aa759ac4301333fd71d87f47ee;p=thirdparty%2Fcurl.git

bufq: unwrite fix

`Curl_bufq_unwrite()` used the head instead of the tail chunk to shrink
the bufq's content. Fix this and add test case that checks correct
behaviour.

Amended test 2601 accordingly.

Reported-by: Chris Stubbs
Closes #15136
---

diff --git a/.github/scripts/spellcheck.words b/.github/scripts/spellcheck.words
index 9ae95ac1d2..4254ec9557 100644
--- a/.github/scripts/spellcheck.words
+++ b/.github/scripts/spellcheck.words
@@ -889,6 +889,7 @@ unsanitized
 Unshare
 unsharing
 untrusted
+unwrite
 UPN
 upstreaming
 URI
diff --git a/docs/internals/BUFQ.md b/docs/internals/BUFQ.md
index 60843819ef..bb57784521 100644
--- a/docs/internals/BUFQ.md
+++ b/docs/internals/BUFQ.md
@@ -76,6 +76,17 @@ void Curl_bufq_skip(struct bufq *q, size_t amount);
 
 This removes `amount` number of bytes from the `bufq`.
 
+## unwrite
+
+It is possible to undo writes by calling:
+
+```
+CURLcode Curl_bufq_unwrite(struct bufq *q, size_t len);
+```
+
+This will remove `len` bytes from the end of the bufq again. When removing
+more bytes than are present, CURLE_AGAIN is returned and the bufq will be
+empty.
 
 ## lifetime
 
diff --git a/lib/bufq.c b/lib/bufq.c
index 4e67c3a3e8..547d4d3762 100644
--- a/lib/bufq.c
+++ b/lib/bufq.c
@@ -491,7 +491,7 @@ CURLcode Curl_bufq_cwrite(struct bufq *q,
 CURLcode Curl_bufq_unwrite(struct bufq *q, size_t len)
 {
   while(len && q->tail) {
-    len -= chunk_unwrite(q->head, len);
+    len -= chunk_unwrite(q->tail, len);
     prune_tail(q);
   }
   return len ? CURLE_AGAIN : CURLE_OK;
diff --git a/tests/unit/unit2601.c b/tests/unit/unit2601.c
index 7e7a14554e..1744d9d315 100644
--- a/tests/unit/unit2601.c
+++ b/tests/unit/unit2601.c
@@ -209,6 +209,41 @@ static void check_bufq(size_t pool_spares,
   }
   fail_unless(nread == nwritten, "did not get the same out as put in");
 
+  /* CHECK bufq_unwrite: write a string repeatedly into the second chunk.
+   * bufq_unwrite() 1 byte. Read strings again and check for content.
+   * We had a bug that unwrite used the head chunk instead of tail, which
+   * did corrupt the read values. */
+  if(TRUE) {
+    const unsigned char buf[] = "0123456789--";
+    size_t roffset;
+    Curl_bufq_reset(&q);
+    while(Curl_bufq_len(&q) < chunk_size) {
+      n = Curl_bufq_write(&q, buf, sizeof(buf), &result);
+      fail_unless(n > 0 && (size_t)n == sizeof(buf), "write incomplete");
+      if(result)
+        break;
+    }
+    result = Curl_bufq_unwrite(&q, 1);
+    roffset = 0;
+    while(!Curl_bufq_is_empty(&q)) {
+      unsigned char rbuf[sizeof(buf)];
+      n = Curl_bufq_read(&q, rbuf, sizeof(rbuf), &result);
+      fail_unless(n > 0, "read should work");
+      if(result)
+        break;
+      if(n != sizeof(rbuf)) {
+        fail_unless(Curl_bufq_is_empty(&q), "should be last read");
+      }
+      if(memcmp(buf, rbuf, n)) {
+        fprintf(stderr, "at offset %zu expected '%.*s', got '%.*s'\n",
+                roffset, (int)n, buf, (int)n, rbuf);
+        fail("read buf content wrong");
+      }
+      roffset += n;
+    }
+    Curl_bufq_reset(&q);
+  }
+
   dump_bufq(&q, "at end of test");
   Curl_bufq_free(&q);
   if(pool_spares > 0)