From 6c9c44d0e9cf7b72bb233360c5308aa063af3d69 Mon Sep 17 00:00:00 2001
From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Fri, 2 Apr 2021 07:46:20 +0000
Subject: [PATCH] Handle more partial responses (#791)

---
 src/HttpHdrContRange.cc | 14 ++++++++++++--
 src/HttpHeaderRange.h   |  7 +++++--
 src/clients/Client.cc   |  7 +++++--
 src/http/Stream.cc      |  9 +++++++--
 4 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/src/HttpHdrContRange.cc b/src/HttpHdrContRange.cc
index 8270e0f11aa..79a507fb84c 100644
--- a/src/HttpHdrContRange.cc
+++ b/src/HttpHdrContRange.cc
@@ -161,9 +161,13 @@ httpHdrContRangeParseInit(HttpHdrContRange * range, const char *str)

     ++p;

-    if (*p == '*')
+    if (*p == '*') {
+        if (!known_spec(range->spec.offset)) {
+            debugs(68, 2, "invalid (*/*) content-range-spec near: '" << str << "'");
+            return 0;
+        }
         range->elength = range_spec_unknown;
-    else if (!httpHeaderParseOffset(p, &range->elength))
+    } else if (!httpHeaderParseOffset(p, &range->elength))
         return 0;
     else if (range->elength <= 0) {
         /* Additional paranoidal check for BUG2155 - entity-length MUST be > 0 */
@@ -174,6 +178,12 @@ httpHdrContRangeParseInit(HttpHdrContRange * range, const char *str)
         return 0;
     }

+    // reject unsatisfied-range and such; we only use well-defined ranges today
+    if (!known_spec(range->spec.offset) || !known_spec(range->spec.length)) {
+        debugs(68, 2, "unwanted content-range-spec near: '" << str << "'");
+        return 0;
+    }
+
     debugs(68, 8, "parsed content-range field: " <<
            (long int) range->spec.offset << "-" <<
            (long int) range->spec.offset + range->spec.length - 1 << " / " <<
diff --git a/src/HttpHeaderRange.h b/src/HttpHeaderRange.h
index 6d93e72b2b8..bf54c8562ba 100644
--- a/src/HttpHeaderRange.h
+++ b/src/HttpHeaderRange.h
@@ -18,8 +18,11 @@
 class HttpReply;
 class Packable;

-/* http byte-range-spec */
-
+// TODO: Refactor to disambiguate and provide message-specific APIs.
+/// either byte-range-spec (in a request Range header)
+/// or suffix-byte-range-spec (in a request Range header)
+/// or byte-range part of byte-range-resp (in a response Content-Range header)
+/// or "*" part of unsatisfied-range (in a response Content-Range header)
 class HttpHdrRangeSpec
 {
     MEMPROXY_CLASS(HttpHdrRangeSpec);
diff --git a/src/clients/Client.cc b/src/clients/Client.cc
index dfb13d053c3..2a2f0e8f878 100644
--- a/src/clients/Client.cc
+++ b/src/clients/Client.cc
@@ -520,8 +520,11 @@ Client::haveParsedReplyHeaders()
     maybePurgeOthers();

     // adaptation may overwrite old offset computed using the virgin response
-    const bool partial = theFinalReply->contentRange();
-    currentOffset = partial ? theFinalReply->contentRange()->spec.offset : 0;
+    currentOffset = 0;
+    if (const auto cr = theFinalReply->contentRange()) {
+        if (cr->spec.offset != HttpHdrRangeSpec::UnknownPosition)
+            currentOffset = cr->spec.offset;
+    }
 }

 /// whether to prevent caching of an otherwise cachable response
diff --git a/src/http/Stream.cc b/src/http/Stream.cc
index 9e346b9d99d..d685a22306e 100644
--- a/src/http/Stream.cc
+++ b/src/http/Stream.cc
@@ -171,12 +171,13 @@ Http::Stream::getNextRangeOffset() const
             return start;
         }

-    } else if (reply && reply->contentRange()) {
+    } else if (const auto cr = reply ? reply->contentRange() : nullptr) {
         /* request does not have ranges, but reply does */
         /** \todo FIXME: should use range_iter_pos on reply, as soon as reply->content_range
          *        becomes HttpHdrRange rather than HttpHdrRangeSpec.
          */
-        return http->out.offset + reply->contentRange()->spec.offset;
+        if (cr->spec.offset != HttpHdrRangeSpec::UnknownPosition)
+            return http->out.offset + cr->spec.offset;
     }

     return http->out.offset;
@@ -240,6 +241,10 @@ Http::Stream::socketState()

             // did we get at least what we expected, based on range specs?

+            // this Content-Range does not tell us how many bytes to expect
+            if (bytesExpected == HttpHdrRangeSpec::UnknownPosition)
+                return STREAM_NONE;
+
             if (bytesSent == bytesExpected) // got everything
                 return STREAM_COMPLETE;