commit e7cf864f938f24eea8af0692c04d16790983c823 Author: Alex Rousskov Date: 2021-03-31 02:44:42 +0000 Handle more Range requests (#790) Also removed some effectively unused code. diff --git a/src/HttpHdrRange.cc b/src/HttpHdrRange.cc index 92b6660d1..7da29765c 100644 --- 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 b4c6d7fd2..fb2956365 100644 --- 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 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 946081465..f57f3f7ef 100644 --- 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 7d6e838f0..ab08fd20e 100644 --- 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 1258d5218..3ea46f5c7 100644 --- 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 e2594b624..338503b4a 100644 --- 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(); } }