Handle more Range requests (#790)

Also removed some effectively unused code.

diff --git a/src/HttpHdrRange.cc b/src/HttpHdrRange.cc
index 501cc4d99c66a80e7e882fc5ad070fe62e88bb2a..273f700370af82b2f7903c03cc4ff52c51a3788d 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 c919375ed344d42389a756a975dc50ee02305f5f..6d93e72b2b8276323bdb357721f2a6e20fba9e5a 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 f64e0041aabb85abd482bffbee744f9c33ec636f..dbf966e026df38a48c4582781cf1e53510f9cef8 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -778,8 +778,8 @@ clientPackRangeHdr(const HttpReplyPointer &rep, const HttpHdrRangeSpec * spec, S
  * 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 db24bba323d327818247fdc926ea66bb89d57c1e..e7faeaddad4bd9d566bbcaa0a8ea3d51e7a20ddf 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -1082,9 +1082,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;
         }
     }
 
@@ -1951,6 +1948,30 @@ ClientHttpRequest::setErrorUri(const char *aUri)
     al->setVirginUrlForMissingRequest(errorUri);
 }
 
+// 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 e52568f3a81680fb8aa3ff3b964053e863c50ce0..9016d4c6ec3c518ce6b5da1d171d09d44f8d0835 100644 (file)
--- a/src/client_side_request.h
+++ b/src/client_side_request.h
@@ -142,7 +142,7 @@ public:
 
     dlink_node active;
     dlink_list client_stream;
-    int mRangeCLen();
+    int64_t mRangeCLen() const;
 
     ClientRequestContext *calloutContext;
     void doCallouts();
@@ -159,6 +159,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 ErrorDetail::Pointer &errDetail);
 

diff --git a/src/http/Stream.cc b/src/http/Stream.cc
index 6aa04d41aada623a897c5d2dcd7af648d183d3c1..9e346b9d99d9ca12106bca36b32ec9defcbd19ee 100644 (file)
--- a/src/http/Stream.cc
+++ b/src/http/Stream.cc
@@ -470,59 +470,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 */
@@ -530,9 +498,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();
     }
 }