Handle more Range requests (#790)

Also removed some effectively unused code.

diff --git a/src/HttpHdrRange.cc b/src/HttpHdrRange.cc
index c0a6fb7f419e815d781cc654b516986c8c653f60..89ba9e5d5206f7f9db70cf675408e8f3071d64a3 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 67bb8aab2f015ba816f82e53acb874317f522c15..e280b3ae0b84151e303320843a55c13a10dea33f 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 3351b43f101b0455f081190ced05dc2b112bc812..77a44598f61824960d8b4ea15a4efe0986a2ac85 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 4c116349b1fc17c7dd79df8073929d6d4dad37d4..86b48420b5cebf7adb153b8cc036c50f5918e5bb 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -1088,9 +1088,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;
         }
     }
 
@@ -1957,6 +1954,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 7bf39e2381a86ac97bc01998f759c410990c7c0a..d5a8453833ed279e42497dcade8081370280ed70 100644 (file)
--- a/src/client_side_request.h
+++ b/src/client_side_request.h
@@ -143,7 +143,7 @@ public:
 
     dlink_node active;
     dlink_list client_stream;
-    int mRangeCLen();
+    int64_t mRangeCLen() const;
 
     ClientRequestContext *calloutContext;
     void doCallouts();
@@ -160,6 +160,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 417448305d4e94818acf934b33debb4f74887310..ecf1495f6be9a35dee51bff05b71904f6ac25d31 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();
     }
 }