commit 6bf66733c122804fada7f5839ef5f3b57e57591c 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 c0a6fb7f4..89ba9e5d5 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 67bb8aab2..e280b3ae0 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 3351b43f1..77a44598f 100644 --- 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 4c116349b..86b48420b 100644 --- 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 7bf39e238..d5a845383 100644 --- 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 417448305..ecf1495f6 100644 --- 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(); } }