Handle more Range requests (#790)

Also removed some effectively unused code.

diff --git a/src/HttpHdrRange.cc b/src/HttpHdrRange.cc
index 92b6660d17e0ff72dacdd895934d600954e69122..7da29765ce5e4928c536f05ba7ec9f6aa3a4d545 100644 (file)
--- a/src/HttpHdrRange.cc
+++ b/src/HttpHdrRange.cc
@@ -526,23 +526,6 @@ HttpHdrRange::offsetLimitExceeded(const int64_t limit) const
     return true;
 }
 
-bool
-HttpHdrRange::contains(const HttpHdrRangeSpec& r) const
-{
-    assert(r.length >= 0);
-    HttpHdrRangeSpec::HttpRange rrange(r.offset, r.offset + r.length);
-
-    for (const_iterator i = begin(); i != end(); ++i) {
-        HttpHdrRangeSpec::HttpRange irange((*i)->offset, (*i)->offset + (*i)->length);
-        HttpHdrRangeSpec::HttpRange intersection = rrange.intersection(irange);
-
-        if (intersection.start == irange.start && intersection.size() == irange.size())
-            return true;
-    }
-
-    return false;
-}
-
 const HttpHdrRangeSpec *
 HttpHdrRangeIter::currentSpec() const
 {

diff --git a/src/HttpHeaderRange.h b/src/HttpHeaderRange.h
index b4c6d7fd22b71e24d1db9b6725e07202ba8958b2..fb2956365e3ee09e53070659d9407f3da28d8d2f 100644 (file)
--- a/src/HttpHeaderRange.h
+++ b/src/HttpHeaderRange.h
@@ -78,7 +78,6 @@ public:
     int64_t firstOffset() const;
     int64_t lowestOffset(int64_t) const;
     bool offsetLimitExceeded(const int64_t limit) const;
-    bool contains(const HttpHdrRangeSpec& r) const;
     std::vector<HttpHdrRangeSpec *> specs;
 
 private:
@@ -100,9 +99,9 @@ public:
     void updateSpec();
     int64_t debt() const;
     void debt(int64_t);
-    int64_t debt_size;      /* bytes left to send from the current spec */
+    int64_t debt_size = 0;  /* bytes left to send from the current spec */
     String boundary;        /* boundary for multipart responses */
-    bool valid;
+    bool valid = false;
 };
 
 #endif /* SQUID_HTTPHEADERRANGE_H */

diff --git a/src/client_side.cc b/src/client_side.cc
index 946081465cb9ba9d4c82e218ca81d15202c7b4db..f57f3f7efaf831d40280851b00ae1931f4ae8a1e 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -728,8 +728,8 @@ clientPackRangeHdr(const HttpReply * rep, const HttpHdrRangeSpec * spec, String
  * warning: assumes that HTTP headers for individual ranges at the
  *          time of the actuall assembly will be exactly the same as
  *          the headers when clientMRangeCLen() is called */
-int
-ClientHttpRequest::mRangeCLen()
+int64_t
+ClientHttpRequest::mRangeCLen() const
 {
     int64_t clen = 0;
     MemBuf mb;

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 7d6e838f0060117d92dfd3ef337f34abad873ba9..ab08fd20edfd0a5a38581be21d6a46907623f3b5 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -1094,9 +1094,6 @@ clientInterpretRequestHeaders(ClientHttpRequest * http)
              * iter up at this point.
              */
             node->readBuffer.offset = request->range->lowestOffset(0);
-            http->range_iter.pos = request->range->begin();
-            http->range_iter.end = request->range->end();
-            http->range_iter.valid = true;
         }
     }
 
@@ -1954,6 +1951,30 @@ ClientHttpRequest::setErrorUri(const char *aUri)
 #include "client_side_request.cci"
 #endif
 
+// XXX: This should not be a _request_ method. Move range_iter elsewhere.
+int64_t
+ClientHttpRequest::prepPartialResponseGeneration()
+{
+    assert(request);
+    assert(request->range);
+
+    range_iter.pos = request->range->begin();
+    range_iter.end = request->range->end();
+    range_iter.debt_size = 0;
+    const auto multipart = request->range->specs.size() > 1;
+    if (multipart)
+        range_iter.boundary = rangeBoundaryStr();
+    range_iter.valid = true; // TODO: Remove.
+    range_iter.updateSpec(); // TODO: Refactor to initialize rather than update.
+
+    assert(range_iter.pos != range_iter.end);
+    const auto &firstRange = *range_iter.pos;
+    assert(firstRange);
+    out.offset = firstRange->offset;
+
+    return multipart ? mRangeCLen() : firstRange->length;
+}
+
 #if USE_ADAPTATION
 /// Initiate an asynchronous adaptation transaction which will call us back.
 void

diff --git a/src/client_side_request.h b/src/client_side_request.h
index 1258d5218c5269f5c6ea4d0846c69242cae31604..3ea46f5c7c4cfc7aa71dab9b9e7f034194450622 100644 (file)
--- a/src/client_side_request.h
+++ b/src/client_side_request.h
@@ -131,7 +131,7 @@ public:
 
     dlink_node active;
     dlink_list client_stream;
-    int mRangeCLen();
+    int64_t mRangeCLen() const;
 
     ClientRequestContext *calloutContext;
     void doCallouts();
@@ -148,6 +148,11 @@ public:
     /// neither the current request nor the parsed request URI are known
     void setErrorUri(const char *errorUri);
 
+    /// Prepares to satisfy a Range request with a generated HTTP 206 response.
+    /// Initializes range_iter state to allow raw range_iter access.
+    /// \returns Content-Length value for the future response; never negative
+    int64_t prepPartialResponseGeneration();
+
     /// Build an error reply. For use with the callouts.
     void calloutsError(const err_type error, const int errDetail);
 

diff --git a/src/http/Stream.cc b/src/http/Stream.cc
index e2594b624c4a45ea559842e24779346ad7527b8e..338503b4a417173e65070c6fa466018aa2766752 100644 (file)
--- a/src/http/Stream.cc
+++ b/src/http/Stream.cc
@@ -444,59 +444,27 @@ Http::Stream::buildRangeHeader(HttpReply *rep)
     } else {
         /* XXX: TODO: Review, this unconditional set may be wrong. */
         rep->sline.set(rep->sline.version, Http::scPartialContent);
-        // web server responded with a valid, but unexpected range.
-        // will (try-to) forward as-is.
-        //TODO: we should cope with multirange request/responses
-        // TODO: review, since rep->content_range is always nil here.
-        bool replyMatchRequest = contentRange != nullptr ?
-                                 request->range->contains(contentRange->spec) :
-                                 true;
+
+        // before range_iter accesses
+        const auto actual_clen = http->prepPartialResponseGeneration();
+
         const int spec_count = http->request->range->specs.size();
-        int64_t actual_clen = -1;
 
         debugs(33, 3, "range spec count: " << spec_count <<
                " virgin clen: " << rep->content_length);
         assert(spec_count > 0);
         /* append appropriate header(s) */
         if (spec_count == 1) {
-            if (!replyMatchRequest) {
-                hdr->putContRange(contentRange);
-                actual_clen = rep->content_length;
-                //http->range_iter.pos = rep->content_range->spec.begin();
-                (*http->range_iter.pos)->offset = contentRange->spec.offset;
-                (*http->range_iter.pos)->length = contentRange->spec.length;
-
-            } else {
-                HttpHdrRange::iterator pos = http->request->range->begin();
-                assert(*pos);
-                /* append Content-Range */
-
-                if (!contentRange) {
-                    /* No content range, so this was a full object we are
-                     * sending parts of.
-                     */
-                    httpHeaderAddContRange(hdr, **pos, rep->content_length);
-                }
-
-                /* set new Content-Length to the actual number of bytes
-                 * transmitted in the message-body */
-                actual_clen = (*pos)->length;
-            }
+            const auto singleSpec = *http->request->range->begin();
+            assert(singleSpec);
+            httpHeaderAddContRange(hdr, *singleSpec, rep->content_length);
         } else {
             /* multipart! */
-            /* generate boundary string */
-            http->range_iter.boundary = http->rangeBoundaryStr();
             /* delete old Content-Type, add ours */
             hdr->delById(Http::HdrType::CONTENT_TYPE);
             httpHeaderPutStrf(hdr, Http::HdrType::CONTENT_TYPE,
                               "multipart/byteranges; boundary=\"" SQUIDSTRINGPH "\"",
                               SQUIDSTRINGPRINT(http->range_iter.boundary));
-            /* Content-Length is not required in multipart responses
-             * but it is always nice to have one */
-            actual_clen = http->mRangeCLen();
-
-            /* http->out needs to start where we want data at */
-            http->out.offset = http->range_iter.currentSpec()->offset;
         }
 
         /* replace Content-Length header */
@@ -504,9 +472,6 @@ Http::Stream::buildRangeHeader(HttpReply *rep)
         hdr->delById(Http::HdrType::CONTENT_LENGTH);
         hdr->putInt64(Http::HdrType::CONTENT_LENGTH, actual_clen);
         debugs(33, 3, "actual content length: " << actual_clen);
-
-        /* And start the range iter off */
-        http->range_iter.updateSpec();
     }
 }