diff --git a/backport-0001-CVE-2023-5824.patch b/backport-0001-CVE-2023-5824.patch deleted file mode 100644 index 8f937c2..0000000 --- a/backport-0001-CVE-2023-5824.patch +++ /dev/null @@ -1,122 +0,0 @@ -From 5921355e474ffbff2cb577c3622ce0e686e8996a Mon Sep 17 00:00:00 2001 -From: Alex Rousskov -Date: Sat, 11 Mar 2023 05:48:14 +0000 -Subject: [PATCH] Replaced clientReplyContext::tempBuffer with old_reqofs - (#1304) - -The tempBuffer data member was not actually used as a buffer. We only -used its offset field, and only for saving reqofs (which has a different -type than tempBuffer.offset!). Replaced the buffer with old_reqofs, -consistent with the rest of the "saved stale entry state" code. - -Also fixed old_reqsize type to match reqsize and grouped that member -with the other private "saved stale entry state" fields. - -Bad old types probably did not trigger runtime failures because the -associated saved numbers are saved at the very beginning of fetching the -entry, when all these accumulation-related counters are still small. - -The remaining reqofs and reqsize types are wrong for platforms where -size_t is not uint64_t, but fixing that deserves a dedicated change. For -now, we just made the types of "old_" and "current" members consistent. - -Reference:https://github.com/squid-cache/squid/commit/5921355e474ffbff2cb577c3622ce0e686e8996a -Conflict:NA ---- - src/client_side_reply.cc | 12 ++++++------ - src/client_side_reply.h | 6 +++--- - 2 files changed, 9 insertions(+), 9 deletions(-) - -diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc -index 0004137cbc9..606a3ecafff 100644 ---- a/src/client_side_reply.cc -+++ b/src/client_side_reply.cc -@@ -66,7 +66,6 @@ clientReplyContext::~clientReplyContext() - /* old_entry might still be set if we didn't yet get the reply - * code in HandleIMSReply() */ - removeStoreReference(&old_sc, &old_entry); -- safe_free(tempBuffer.data); - cbdataReferenceDone(http); - HTTPMSGUNLOCK(reply); - } -@@ -76,7 +75,6 @@ clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) : - http(cbdataReference(clientContext)), - headers_sz(0), - sc(nullptr), -- old_reqsize(0), - reqsize(0), - reqofs(0), - ourNode(nullptr), -@@ -84,6 +82,8 @@ clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) : - old_entry(nullptr), - old_sc(nullptr), - old_lastmod(-1), -+ old_reqofs(0), -+ old_reqsize(0), - deleting(false), - collapsedRevalidation(crNone) - { -@@ -202,7 +202,7 @@ clientReplyContext::saveState() - old_lastmod = http->request->lastmod; - old_etag = http->request->etag; - old_reqsize = reqsize; -- tempBuffer.offset = reqofs; -+ old_reqofs = reqofs; - /* Prevent accessing the now saved entries */ - http->storeEntry(nullptr); - sc = nullptr; -@@ -219,7 +219,7 @@ clientReplyContext::restoreState() - http->storeEntry(old_entry); - sc = old_sc; - reqsize = old_reqsize; -- reqofs = tempBuffer.offset; -+ reqofs = old_reqofs; - http->request->lastmod = old_lastmod; - http->request->etag = old_etag; - /* Prevent accessed the old saved entries */ -@@ -228,7 +228,7 @@ clientReplyContext::restoreState() - old_lastmod = -1; - old_etag.clean(); - old_reqsize = 0; -- tempBuffer.offset = 0; -+ old_reqofs = 0; - } - - void -@@ -377,7 +377,7 @@ clientReplyContext::sendClientUpstreamResponse() - http->storeEntry()->clearPublicKeyScope(); - - /* here the data to send is the data we just received */ -- tempBuffer.offset = 0; -+ old_reqofs = 0; - old_reqsize = 0; - /* sendMoreData tracks the offset as well. - * Force it back to zero */ -diff --git a/src/client_side_reply.h b/src/client_side_reply.h -index 68b45715b33..32a38bc95e1 100644 ---- a/src/client_side_reply.h -+++ b/src/client_side_reply.h -@@ -74,8 +74,6 @@ class clientReplyContext : public RefCountable, public StoreClient - /// Not to be confused with ClientHttpRequest::Out::headers_sz. - int headers_sz; - store_client *sc; /* The store_client we're using */ -- StoreIOBuffer tempBuffer; /* For use in validating requests via IMS */ -- int old_reqsize; /* ... again, for the buffer */ - size_t reqsize; - size_t reqofs; - char tempbuf[HTTP_REQBUF_SZ]; ///< a temporary buffer if we need working storage -@@ -135,11 +133,13 @@ class clientReplyContext : public RefCountable, public StoreClient - /// TODO: Exclude internal Store match bans from the "mismatch" category. - const char *firstStoreLookup_ = nullptr; - -+ /* (stale) cache hit information preserved during IMS revalidation */ - StoreEntry *old_entry; -- /* ... for entry to be validated */ - store_client *old_sc; - time_t old_lastmod; - String old_etag; -+ size_t old_reqofs; -+ size_t old_reqsize; - - bool deleting; - diff --git a/backport-0002-CVE-2023-5824.patch b/backport-0002-CVE-2023-5824.patch deleted file mode 100644 index 9b38067..0000000 --- a/backport-0002-CVE-2023-5824.patch +++ /dev/null @@ -1,3247 +0,0 @@ -From 122a6e3cb1a26940369b5af5323a6b6e7a4a5928 Mon Sep 17 00:00:00 2001 -From: Alex Rousskov -Date: Sat, 24 Jun 2023 08:18:55 +0000 -Subject: [PATCH] Remove serialized HTTP headers from storeClientCopy() (#1335) - -Do not send serialized HTTP response header bytes in storeClientCopy() -answers. Ignore serialized header size when calling storeClientCopy(). - -This complex change adjusts storeClientCopy() API to addresses several -related problems with storeClientCopy() and its callers. The sections -below summarize storeClientCopy() changes and then move on to callers. - - -### storeClientCopy() changes - -Squid incorrectly assumed that serialized HTTP response headers are read -from disk in a single storeRead() request. In reality, many situations -lead to store_client::readBody() receiving partial HTTP headers, -resulting in parseCharBuf() failure and a level-0 cache.log message: - - Could not parse headers from on disk object - -Inadequate handling of this failure resulted in a variety of problems. -Squid now accumulates storeRead() results to parse larger headers and -also handles parsing failures better, but we could not just stop there. - -With the storeRead() accumulation in place, it is no longer possible to -send parsed serialized HTTP headers to storeClientCopy() callers because -those callers do not provide enough buffer space to fit larger headers. -Increasing caller buffer capacity does not work well because the actual -size of the serialized header is unknown in advance and may be quite -large. Always allocating large buffers "just in case" is bad for -performance. Finally, larger buffers may jeopardize hard-to-find code -that uses hard-coded 4KB buffers without using HTTP_REQBUF_SZ macro. - -Fortunately, storeClientCopy() callers either do not care about -serialized HTTP response headers or should not care about them! The API -forced callers to deal with serialized headers, but callers could (and -some did) just use the parsed headers available in the corresponding -MemObject. With this API change, storeClientCopy() callers no longer -receive serialized headers and do not need to parse or skip them. -Consequently, callers also do not need to account for response headers -size when computing offsets for subsequent storeClientCopy() requests. - -Restricting storeClientCopy() API to HTTP _body_ bytes removed a lot of -problematic caller code. Caller changes are summarized further below. - - -A similar HTTP response header parsing problem existed in shared memory -cache code. That code was actually aware that headers may span multiple -cache slices but incorrectly assumed that httpMsgParseStep() accumulates -input as needed (to make another parsing "step"). It does not. Large -response headers cached in shared memory triggered a level-1 message: - - Corrupted mem-cached headers: e:... - -Fixed MemStore code now accumulates serialized HTTP response headers as -needed to parse them, sharing high-level parsing code with store_client. - - -### Primary code path (clientReplyContext::SendMoreData, CacheHit, etc.) - -Old clientReplyContext methods worked hard to skip received serialized -HTTP headers. The code contained dangerous and often complex/unreadable -manipulation of various raw offsets and buffer pointers, aggravated by -the perceived need to save/restore those offsets across asynchronous -checks (see below). That header skipping code is gone now. Several stale -and misleading comments related to Store buffers management were also -removed or updated. - -We replaced reqofs/reqsize with simpler/safer lastStreamBufferedBytes, -while becoming more consistent with that "cached" info invalidation. We -still need this info to resume HTTP body processing after asynchronous -http_reply_access checks and cache hit validations, but we no longer -save/restore this info for hit validation: No need to save/restore -information about the buffer that hit validation does not use and must -never touch! - -The API change also moved from-Store StoreIOBuffer usage closer to -StoreIOBuffers manipulated by Clients Streams code. Buffers in both -categories now contain just the body bytes, and both now treat zero -length as EOF only _after_ processing the response headers. - -These changes improve overall code quality, but this code path and these -changes still suffer from utterly unsafe legacy interfaces like -StoreIOBuffer and clientStreamNode. We cannot rely on the compiler to -check our work. The risk of these changes exposing/causing bugs is high. - - -### AS number WHOIS lookup - -asHandleReply() expected WHOIS response body bytes where serialized HTTP -headers were! The code also had multiple problems typical for manually -written C parsers dealing with raw input buffers. Now replaced with a -Tokenizer-based code. - - -### Cache Digests - -To skip received HTTP response headers, peerDigestHandleReply() helper -functions called headersEnd() on the received buffer. Twice. We have now -merged those two parsing helper functions into one (that just checks the -already parsed headers). This merger preserved "304s must come with -fetch->pd->cd" logic that was hidden/spread across those two functions. - - -### URN resolver - -urnHandleReply() re-parsed received HTTP response headers. We left its -HTTP body parsing code unchanged except for polishing NUL-termination. - - -### NetDB exchange - -netdbExchangeHandleReply() re-parsed received HTTP response headers to -find where they end (via headersEnd()). We improved handing of corner -cases and replaced some "tricky bits" code, reusing the new -Store::ParsingBuffer class. The net_db record parsing code is unchanged. - - -### SMP Cache Manager - -Mgr::StoreToCommWriter::noteStoreCopied() is a very special case. It -actually worked OK because, unlike all other storeClientCopy() callers, -this code does not get serialized HTTP headers from Store: The code -adding bytes to the corresponding StoreEntry does not write serialized -HTTP headers at all. StoreToCommWriter is used to deliver kid-specific -pieces of an HTTP body of an SMP cache manager response. The HTTP -headers of that response are handled elsewhere. We left this code -unchanged, but the existence of the special no-headers case does -complicate storeClientCopy() API documentation, implementation, and -understanding. - -Co-authored-by: Eduard Bagdasaryan - -Reference:https://github.com/squid-cache/squid/commit/122a6e3cb1a26940369b5af5323a6b6e7a4a5928 -Conflict:NA ---- - src/HttpReply.cc | 34 +++ - src/HttpReply.h | 7 + - src/MemObject.cc | 6 + - src/MemObject.h | 9 + - src/MemStore.cc | 75 ++++--- - src/MemStore.h | 2 +- - src/StoreClient.h | 65 +++++- - src/StoreIOBuffer.h | 3 + - src/acl/Asn.cc | 167 +++++--------- - src/clientStream.cc | 3 +- - src/client_side_reply.cc | 304 +++++++++++-------------- - src/client_side_reply.h | 40 ++-- - src/client_side_request.h | 1 - - src/enums.h | 1 - - src/icmp/net_db.cc | 144 ++++-------- - src/peer_digest.cc | 95 ++------ - src/store.cc | 11 + - src/store/Makefile.am | 2 + - src/store/ParsingBuffer.cc | 198 +++++++++++++++++ - src/store/ParsingBuffer.h | 128 +++++++++++ - src/store/forward.h | 1 + - src/store_client.cc | 430 ++++++++++++++++++++++++------------ - src/tests/stub_HttpReply.cc | 1 + - src/urn.cc | 90 +++----- - 24 files changed, 1090 insertions(+), 727 deletions(-) - create mode 100644 src/store/ParsingBuffer.cc - create mode 100644 src/store/ParsingBuffer.h - -diff --git a/src/HttpReply.cc b/src/HttpReply.cc -index d0de373ff69..dbf9ceb1ff4 100644 ---- a/src/HttpReply.cc -+++ b/src/HttpReply.cc -@@ -21,7 +21,9 @@ - #include "HttpReply.h" - #include "HttpRequest.h" - #include "MemBuf.h" -+#include "sbuf/Stream.h" - #include "SquidConfig.h" -+#include "SquidMath.h" - #include "Store.h" - #include "StrList.h" - -@@ -456,6 +458,38 @@ HttpReply::parseFirstLine(const char *blk_start, const char *blk_end) - return sline.parse(protoPrefix, blk_start, blk_end); - } - -+size_t -+HttpReply::parseTerminatedPrefix(const char * const terminatedBuf, const size_t bufSize) -+{ -+ auto error = Http::scNone; -+ const bool eof = false; // TODO: Remove after removing atEnd from HttpHeader::parse() -+ if (parse(terminatedBuf, bufSize, eof, &error)) { -+ debugs(58, 7, "success after accumulating " << bufSize << " bytes and parsing " << hdr_sz); -+ Assure(pstate == Http::Message::psParsed); -+ Assure(hdr_sz > 0); -+ Assure(!Less(bufSize, hdr_sz)); // cannot parse more bytes than we have -+ return hdr_sz; // success -+ } -+ -+ Assure(pstate != Http::Message::psParsed); -+ hdr_sz = 0; -+ -+ if (error) { -+ throw TextException(ToSBuf("failed to parse HTTP headers", -+ Debug::Extra, "parser error code: ", error, -+ Debug::Extra, "accumulated unparsed bytes: ", bufSize, -+ Debug::Extra, "reply_header_max_size: ", Config.maxReplyHeaderSize), -+ Here()); -+ } -+ -+ debugs(58, 3, "need more bytes after accumulating " << bufSize << " out of " << Config.maxReplyHeaderSize); -+ -+ // the parse() call above enforces Config.maxReplyHeaderSize limit -+ // XXX: Make this a strict comparison after fixing Http::Message::parse() enforcement -+ Assure(bufSize <= Config.maxReplyHeaderSize); -+ return 0; // parsed nothing, need more data -+} -+ - void - HttpReply::configureContentLengthInterpreter(Http::ContentLengthInterpreter &interpreter) - { -diff --git a/src/HttpReply.h b/src/HttpReply.h -index 291f60a5d3e..f19614efa8e 100644 ---- a/src/HttpReply.h -+++ b/src/HttpReply.h -@@ -125,6 +125,13 @@ class HttpReply: public Http::Message - /// parses reply header using Parser - bool parseHeader(Http1::Parser &hp); - -+ /// Parses response status line and headers at the start of the given -+ /// NUL-terminated buffer of the given size. Respects reply_header_max_size. -+ /// Assures pstate becomes Http::Message::psParsed on (and only on) success. -+ /// \returns the number of bytes in a successfully parsed prefix (or zero) -+ /// \retval 0 implies that more data is needed to parse the response prefix -+ size_t parseTerminatedPrefix(const char *, size_t); -+ - private: - /** initialize */ - void init(); -diff --git a/src/MemObject.cc b/src/MemObject.cc -index bb1292108f2..effbb2c4cd8 100644 ---- a/src/MemObject.cc -+++ b/src/MemObject.cc -@@ -353,6 +353,12 @@ MemObject::policyLowestOffsetToKeep(bool swap) const - */ - int64_t lowest_offset = lowestMemReaderOffset(); - -+ // XXX: Remove the last (Config.onoff.memory_cache_first-based) condition -+ // and update keepForLocalMemoryCache() accordingly. The caller wants to -+ // remove all local memory that is safe to remove. Honoring caching -+ // preferences is its responsibility. Our responsibility is safety. The -+ // situation was different when ff4b33f added that condition -- there was no -+ // keepInLocalMemory/keepForLocalMemoryCache() call guard back then. - if (endOffset() < lowest_offset || - endOffset() - inmem_lo > (int64_t)Config.Store.maxInMemObjSize || - (swap && !Config.onoff.memory_cache_first)) -diff --git a/src/MemObject.h b/src/MemObject.h -index 0a7be720465..68007662405 100644 ---- a/src/MemObject.h -+++ b/src/MemObject.h -@@ -89,6 +89,15 @@ class MemObject - bool appliedUpdates = false; - - void stat (MemBuf * mb) const; -+ -+ /// The offset of the last memory-stored HTTP response byte plus one. -+ /// * HTTP response headers (if any) are stored at offset zero. -+ /// * HTTP response body byte[n] usually has offset (hdr_sz + n), where -+ /// hdr_sz is the size of stored HTTP response headers (zero if none); and -+ /// n is the corresponding byte offset in the whole resource body. -+ /// However, some 206 (Partial Content) response bodies are stored (and -+ /// retrieved) as regular 200 response bodies, disregarding offsets of -+ /// their body parts. \sa HttpStateData::decideIfWeDoRanges(). - int64_t endOffset () const; - - /// sets baseReply().hdr_sz (i.e. written reply headers size) to endOffset() -diff --git a/src/MemStore.cc b/src/MemStore.cc -index 44dc0dcb983..009c12a64c6 100644 ---- a/src/MemStore.cc -+++ b/src/MemStore.cc -@@ -17,6 +17,8 @@ - #include "MemObject.h" - #include "MemStore.h" - #include "mime_header.h" -+#include "sbuf/SBuf.h" -+#include "sbuf/Stream.h" - #include "SquidConfig.h" - #include "SquidMath.h" - #include "StoreStats.h" -@@ -311,19 +313,25 @@ MemStore::get(const cache_key *key) - // create a brand new store entry and initialize it with stored info - StoreEntry *e = new StoreEntry(); - -- // XXX: We do not know the URLs yet, only the key, but we need to parse and -- // store the response for the Root().find() callers to be happy because they -- // expect IN_MEMORY entries to already have the response headers and body. -- e->createMemObject(); -- -- anchorEntry(*e, index, *slot); -- -- const bool copied = copyFromShm(*e, index, *slot); -- -- if (copied) -- return e; -+ try { -+ // XXX: We do not know the URLs yet, only the key, but we need to parse and -+ // store the response for the Root().find() callers to be happy because they -+ // expect IN_MEMORY entries to already have the response headers and body. -+ e->createMemObject(); -+ -+ anchorEntry(*e, index, *slot); -+ -+ // TODO: make copyFromShm() throw on all failures, simplifying this code -+ if (copyFromShm(*e, index, *slot)) -+ return e; -+ debugs(20, 3, "failed for " << *e); -+ } catch (...) { -+ // see store_client::parseHttpHeadersFromDisk() for problems this may log -+ debugs(20, DBG_IMPORTANT, "ERROR: Cannot load a cache hit from shared memory" << -+ Debug::Extra << "exception: " << CurrentException << -+ Debug::Extra << "cache_mem entry: " << *e); -+ } - -- debugs(20, 3, "failed for " << *e); - map->freeEntry(index); // do not let others into the same trap - destroyStoreEntry(static_cast(e)); - return nullptr; -@@ -469,6 +477,8 @@ MemStore::copyFromShm(StoreEntry &e, const sfileno index, const Ipc::StoreMapAnc - Ipc::StoreMapSliceId sid = anchor.start; // optimize: remember the last sid - bool wasEof = anchor.complete() && sid < 0; - int64_t sliceOffset = 0; -+ -+ SBuf httpHeaderParsingBuffer; - while (sid >= 0) { - const Ipc::StoreMapSlice &slice = map->readableSlice(index, sid); - // slice state may change during copying; take snapshots now -@@ -491,10 +501,18 @@ MemStore::copyFromShm(StoreEntry &e, const sfileno index, const Ipc::StoreMapAnc - const StoreIOBuffer sliceBuf(wasSize - prefixSize, - e.mem_obj->endOffset(), - page + prefixSize); -- if (!copyFromShmSlice(e, sliceBuf, wasEof)) -- return false; -+ -+ copyFromShmSlice(e, sliceBuf); - debugs(20, 8, "entry " << index << " copied slice " << sid << - " from " << extra.page << '+' << prefixSize); -+ -+ // parse headers if needed; they might span multiple slices! -+ auto &reply = e.mem().adjustableBaseReply(); -+ if (reply.pstate != Http::Message::psParsed) { -+ httpHeaderParsingBuffer.append(sliceBuf.data, sliceBuf.length); -+ if (reply.parseTerminatedPrefix(httpHeaderParsingBuffer.c_str(), httpHeaderParsingBuffer.length())) -+ httpHeaderParsingBuffer = SBuf(); // we do not need these bytes anymore -+ } - } - // else skip a [possibly incomplete] slice that we copied earlier - -@@ -526,6 +544,9 @@ MemStore::copyFromShm(StoreEntry &e, const sfileno index, const Ipc::StoreMapAnc - debugs(20, 5, "mem-loaded all " << e.mem_obj->endOffset() << '/' << - anchor.basics.swap_file_sz << " bytes of " << e); - -+ if (e.mem().adjustableBaseReply().pstate != Http::Message::psParsed) -+ throw TextException(ToSBuf("truncated mem-cached headers; accumulated: ", httpHeaderParsingBuffer.length()), Here()); -+ - // from StoreEntry::complete() - e.mem_obj->object_sz = e.mem_obj->endOffset(); - e.store_status = STORE_OK; -@@ -541,32 +562,11 @@ MemStore::copyFromShm(StoreEntry &e, const sfileno index, const Ipc::StoreMapAnc - } - - /// imports one shared memory slice into local memory --bool --MemStore::copyFromShmSlice(StoreEntry &e, const StoreIOBuffer &buf, bool eof) -+void -+MemStore::copyFromShmSlice(StoreEntry &e, const StoreIOBuffer &buf) - { - debugs(20, 7, "buf: " << buf.offset << " + " << buf.length); - -- // from store_client::readBody() -- // parse headers if needed; they might span multiple slices! -- const auto rep = &e.mem().adjustableBaseReply(); -- if (rep->pstate < Http::Message::psParsed) { -- // XXX: have to copy because httpMsgParseStep() requires 0-termination -- MemBuf mb; -- mb.init(buf.length+1, buf.length+1); -- mb.append(buf.data, buf.length); -- mb.terminate(); -- const int result = rep->httpMsgParseStep(mb.buf, buf.length, eof); -- if (result > 0) { -- assert(rep->pstate == Http::Message::psParsed); -- } else if (result < 0) { -- debugs(20, DBG_IMPORTANT, "Corrupted mem-cached headers: " << e); -- return false; -- } else { // more slices are needed -- assert(!eof); -- } -- } -- debugs(20, 7, "rep pstate: " << rep->pstate); -- - // local memory stores both headers and body so copy regardless of pstate - const int64_t offBefore = e.mem_obj->endOffset(); - assert(e.mem_obj->data_hdr.write(buf)); // from MemObject::write() -@@ -574,7 +574,6 @@ MemStore::copyFromShmSlice(StoreEntry &e, const StoreIOBuffer &buf, bool eof) - // expect to write the entire buf because StoreEntry::write() never fails - assert(offAfter >= 0 && offBefore <= offAfter && - static_cast(offAfter - offBefore) == buf.length); -- return true; - } - - /// whether we should cache the entry -diff --git a/src/MemStore.h b/src/MemStore.h -index bb71a0e7402..b795254b94d 100644 ---- a/src/MemStore.h -+++ b/src/MemStore.h -@@ -80,7 +80,7 @@ class MemStore: public Store::Controlled, public Ipc::StoreMapCleaner - void copyToShm(StoreEntry &e); - void copyToShmSlice(StoreEntry &e, Ipc::StoreMapAnchor &anchor, Ipc::StoreMap::Slice &slice); - bool copyFromShm(StoreEntry &e, const sfileno index, const Ipc::StoreMapAnchor &anchor); -- bool copyFromShmSlice(StoreEntry &e, const StoreIOBuffer &buf, bool eof); -+ void copyFromShmSlice(StoreEntry &, const StoreIOBuffer &); - - void updateHeadersOrThrow(Ipc::StoreMapUpdate &update); - -diff --git a/src/StoreClient.h b/src/StoreClient.h -index 08026e75bed..29147433a34 100644 ---- a/src/StoreClient.h -+++ b/src/StoreClient.h -@@ -13,10 +13,23 @@ - #include "base/AsyncCall.h" - #include "base/forward.h" - #include "dlink.h" -+#include "store/ParsingBuffer.h" - #include "StoreIOBuffer.h" - #include "StoreIOState.h" - --typedef void STCB(void *, StoreIOBuffer); /* store callback */ -+/// A storeClientCopy() callback function. -+/// -+/// Upon storeClientCopy() success, StoreIOBuffer::flags.error is zero, and -+/// * HTTP response headers (if any) are available via MemObject::freshestReply(); -+/// * HTTP response body bytes (if any) are available via StoreIOBuffer. -+/// -+/// STCB callbacks may use response semantics to detect certain EOF conditions. -+/// Callbacks that expect HTTP headers may call store_client::atEof(). Similar -+/// to clientStreamCallback() callbacks, callbacks dedicated to receiving HTTP -+/// bodies may use zero StoreIOBuffer::length as an EOF condition. -+/// -+/// Errors are indicated by setting StoreIOBuffer flags.error. -+using STCB = void (void *, StoreIOBuffer); - - class StoreEntry; - class ACLFilledChecklist; -@@ -88,7 +101,13 @@ class store_client - - void dumpStats(MemBuf * output, int clientNumber) const; - -- int64_t cmp_offset; -+ // TODO: When STCB gets a dedicated Answer type, move this info there. -+ /// Whether the last successful storeClientCopy() answer was known to -+ /// contain the last body bytes of the HTTP response -+ /// \retval true requesting bytes at higher offsets is futile -+ /// \sa STCB -+ bool atEof() const { return atEof_; } -+ - #if STORE_CLIENT_LIST_DEBUG - - void *owner; -@@ -123,18 +142,27 @@ class store_client - dlink_node node; - - private: -- bool moreToSend() const; -+ bool moreToRead() const; -+ bool canReadFromMemory() const; -+ bool answeredOnce() const { return answers >= 1; } -+ bool sendingHttpHeaders() const; -+ int64_t nextHttpReadOffset() const; - - void fileRead(); - void scheduleDiskRead(); -- void scheduleMemRead(); -+ void readFromMemory(); - void scheduleRead(); - bool startSwapin(); -+ void handleBodyFromDisk(); -+ void maybeWriteFromDiskToMemory(const StoreIOBuffer &); -+ -+ bool parseHttpHeadersFromDisk(); -+ bool tryParsingHttpHeaders(); -+ void skipHttpHeadersFromDisk(); - - void fail(); - void callback(ssize_t); - void noteCopiedBytes(size_t); -- void noteEof(); - void noteNews(); - void finishCallback(); - static void FinishCallback(store_client *); -@@ -142,13 +170,23 @@ class store_client - int type; - bool object_ok; - -+ /// \copydoc atEof() -+ bool atEof_; -+ - /// Storage and metadata associated with the current copy() request. Ought - /// to be ignored when not answering a copy() request. - StoreIOBuffer copyInto; - -- /// The number of bytes loaded from Store into copyInto while answering the -- /// current copy() request. Ought to be ignored when not answering. -- size_t copiedSize; -+ /// the total number of finishCallback() calls -+ uint64_t answers; -+ -+ /// Accumulates raw bytes read from Store while answering the current copy() -+ /// request. Buffer contents depends on the source and parsing stage; it may -+ /// hold (parts of) swap metadata, HTTP response headers, and/or HTTP -+ /// response body bytes. -+ std::optional parsingBuffer; -+ -+ StoreIOBuffer lastDiskRead; ///< buffer used for the last storeRead() call - - /* Until we finish stuffing code into store_client */ - -@@ -173,7 +211,18 @@ class store_client - } _callback; - }; - -+/// Asynchronously read HTTP response headers and/or body bytes from Store. -+/// -+/// The requested zero-based HTTP body offset is specified via the -+/// StoreIOBuffer::offset field. The first call (for a given store_client -+/// object) must specify zero offset. -+/// -+/// The requested HTTP body portion size is specified via the -+/// StoreIOBuffer::length field. The function may return fewer body bytes. -+/// -+/// See STCB for result delivery details. - void storeClientCopy(store_client *, StoreEntry *, StoreIOBuffer, STCB *, void *); -+ - store_client* storeClientListAdd(StoreEntry * e, void *data); - int storeUnregister(store_client * sc, StoreEntry * e, void *data); - int storePendingNClients(const StoreEntry * e); -diff --git a/src/StoreIOBuffer.h b/src/StoreIOBuffer.h -index b38c40b33a4..d473ac58c7a 100644 ---- a/src/StoreIOBuffer.h -+++ b/src/StoreIOBuffer.h -@@ -43,6 +43,9 @@ class StoreIOBuffer - return Range(offset, offset + length); - } - -+ /// convenience method for changing the offset of a being-configured buffer -+ StoreIOBuffer &positionAt(const int64_t newOffset) { offset = newOffset; return *this; } -+ - void dump() const { - if (fwrite(data, length, 1, stderr)) {} - if (fwrite("\n", 1, 1, stderr)) {} -diff --git a/src/acl/Asn.cc b/src/acl/Asn.cc -index f41a084efe6..f3d20138ee1 100644 ---- a/src/acl/Asn.cc -+++ b/src/acl/Asn.cc -@@ -16,22 +16,21 @@ - #include "acl/DestinationIp.h" - #include "acl/SourceAsn.h" - #include "acl/Strategised.h" -+#include "base/CharacterSet.h" - #include "FwdState.h" - #include "HttpReply.h" - #include "HttpRequest.h" - #include "ipcache.h" - #include "MasterXaction.h" - #include "mgr/Registration.h" -+#include "parser/Tokenizer.h" - #include "radix.h" - #include "RequestFlags.h" -+#include "sbuf/SBuf.h" - #include "SquidConfig.h" - #include "Store.h" - #include "StoreClient.h" - --#ifndef AS_REQBUF_SZ --#define AS_REQBUF_SZ 4096 --#endif -- - /* BEGIN of definitions for radix tree entries */ - - /* 32/128 bits address in memory with length */ -@@ -71,9 +70,8 @@ class ASState - CBDATA_CLASS(ASState); - - public: -- ASState() { -- memset(reqbuf, 0, sizeof(reqbuf)); -- } -+ ASState() = default; -+ - ~ASState() { - if (entry) { - debugs(53, 3, entry->url()); -@@ -87,10 +85,9 @@ class ASState - store_client *sc = nullptr; - HttpRequest::Pointer request; - int as_number = 0; -- int64_t offset = 0; -- int reqofs = 0; -- char reqbuf[AS_REQBUF_SZ]; -- bool dataRead = false; -+ -+ /// for receiving a WHOIS reply body from Store and interpreting it -+ Store::ParsingBuffer parsingBuffer; - }; - - CBDATA_CLASS_INIT(ASState); -@@ -103,7 +100,7 @@ struct rtentry_t { - m_ADDR e_mask; - }; - --static int asnAddNet(char *, int); -+static int asnAddNet(const SBuf &, int); - - static void asnCacheStart(int as); - -@@ -258,8 +255,7 @@ asnCacheStart(int as) - xfree(asres); - - asState->entry = e; -- StoreIOBuffer readBuffer (AS_REQBUF_SZ, asState->offset, asState->reqbuf); -- storeClientCopy(asState->sc, e, readBuffer, asHandleReply, asState); -+ storeClientCopy(asState->sc, e, asState->parsingBuffer.makeInitialSpace(), asHandleReply, asState); - } - - static void -@@ -267,13 +263,8 @@ asHandleReply(void *data, StoreIOBuffer result) - { - ASState *asState = (ASState *)data; - StoreEntry *e = asState->entry; -- char *s; -- char *t; -- char *buf = asState->reqbuf; -- int leftoversz = -1; - -- debugs(53, 3, "asHandleReply: Called with size=" << (unsigned int)result.length); -- debugs(53, 3, "asHandleReply: buffer='" << buf << "'"); -+ debugs(53, 3, result << " for " << asState->as_number << " with " << *e); - - /* First figure out whether we should abort the request */ - -@@ -282,11 +273,7 @@ asHandleReply(void *data, StoreIOBuffer result) - return; - } - -- if (result.length == 0 && asState->dataRead) { -- debugs(53, 3, "asHandleReply: Done: " << e->url()); -- delete asState; -- return; -- } else if (result.flags.error) { -+ if (result.flags.error) { - debugs(53, DBG_IMPORTANT, "ERROR: asHandleReply: Called with Error set and size=" << (unsigned int) result.length); - delete asState; - return; -@@ -296,117 +283,77 @@ asHandleReply(void *data, StoreIOBuffer result) - return; - } - -- /* -- * Next, attempt to parse our request -- * Remembering that the actual buffer size is retsize + reqofs! -- */ -- s = buf; -- -- while ((size_t)(s - buf) < result.length + asState->reqofs && *s != '\0') { -- while (*s && xisspace(*s)) -- ++s; -+ asState->parsingBuffer.appended(result.data, result.length); -+ Parser::Tokenizer tok(SBuf(asState->parsingBuffer.content().data, asState->parsingBuffer.contentSize())); -+ SBuf address; -+ // Word delimiters in WHOIS ASN replies. RFC 3912 mentions SP, CR, and LF. -+ // Others are added to mimic an earlier isspace()-based implementation. -+ static const auto WhoisSpaces = CharacterSet("ASCII_spaces", " \f\r\n\t\v"); -+ while (tok.token(address, WhoisSpaces)) { -+ (void)asnAddNet(address, asState->as_number); -+ } -+ asState->parsingBuffer.consume(tok.parsedSize()); -+ const auto leftoverBytes = asState->parsingBuffer.contentSize(); - -- for (t = s; *t; ++t) { -- if (xisspace(*t)) -- break; -- } -+ if (asState->sc->atEof()) { -+ if (leftoverBytes) -+ debugs(53, 2, "WHOIS: Discarding the last " << leftoverBytes << " received bytes of a truncated AS response"); -+ delete asState; -+ return; -+ } - -- if (*t == '\0') { -- /* oof, word should continue on next block */ -- break; -- } -+ const auto remainingSpace = asState->parsingBuffer.space().positionAt(result.offset + result.length); - -- *t = '\0'; -- debugs(53, 3, "asHandleReply: AS# " << s << " (" << asState->as_number << ")"); -- asnAddNet(s, asState->as_number); -- s = t + 1; -- asState->dataRead = true; -+ if (!remainingSpace.length) { -+ Assure(leftoverBytes); -+ debugs(53, DBG_IMPORTANT, "WARNING: Ignoring the tail of a WHOIS AS response" << -+ " with an unparsable section of " << leftoverBytes << -+ " bytes ending at offset " << remainingSpace.offset); -+ delete asState; -+ return; - } - -- /* -- * Next, grab the end of the 'valid data' in the buffer, and figure -- * out how much data is left in our buffer, which we need to keep -- * around for the next request -- */ -- leftoversz = (asState->reqofs + result.length) - (s - buf); -- -- assert(leftoversz >= 0); -- -- /* -- * Next, copy the left over data, from s to s + leftoversz to the -- * beginning of the buffer -- */ -- memmove(buf, s, leftoversz); -- -- /* -- * Next, update our offset and reqofs, and kick off a copy if required -- */ -- asState->offset += result.length; -- -- asState->reqofs = leftoversz; -- -- debugs(53, 3, "asState->offset = " << asState->offset); -- -- if (e->store_status == STORE_PENDING) { -- debugs(53, 3, "asHandleReply: store_status == STORE_PENDING: " << e->url() ); -- StoreIOBuffer tempBuffer (AS_REQBUF_SZ - asState->reqofs, -- asState->offset, -- asState->reqbuf + asState->reqofs); -- storeClientCopy(asState->sc, -- e, -- tempBuffer, -- asHandleReply, -- asState); -- } else { -- StoreIOBuffer tempBuffer; -- debugs(53, 3, "asHandleReply: store complete, but data received " << e->url() ); -- tempBuffer.offset = asState->offset; -- tempBuffer.length = AS_REQBUF_SZ - asState->reqofs; -- tempBuffer.data = asState->reqbuf + asState->reqofs; -- storeClientCopy(asState->sc, -- e, -- tempBuffer, -- asHandleReply, -- asState); -+ const decltype(StoreIOBuffer::offset) stillReasonableOffset = 100000; // an arbitrary limit in bytes -+ if (remainingSpace.offset > stillReasonableOffset) { -+ // stop suspicious accumulation of parsed addresses and/or work -+ debugs(53, DBG_IMPORTANT, "WARNING: Ignoring the tail of a suspiciously large WHOIS AS response" << -+ " exceeding " << stillReasonableOffset << " bytes"); -+ delete asState; -+ return; - } -+ -+ storeClientCopy(asState->sc, e, remainingSpace, asHandleReply, asState); - } - - /** - * add a network (addr, mask) to the radix tree, with matching AS number - */ - static int --asnAddNet(char *as_string, int as_number) -+asnAddNet(const SBuf &addressAndMask, const int as_number) - { - struct squid_radix_node *rn; - CbDataList **Tail = nullptr; - CbDataList *q = nullptr; - as_info *asinfo = nullptr; - -- Ip::Address mask; -- Ip::Address addr; -- char *t; -- int bitl; -- -- t = strchr(as_string, '/'); -- -- if (t == nullptr) { -+ static const CharacterSet NonSlashSet = CharacterSet("slash", "/").complement("non-slash"); -+ Parser::Tokenizer tok(addressAndMask); -+ SBuf addressToken; -+ if (!(tok.prefix(addressToken, NonSlashSet) && tok.skip('/'))) { - debugs(53, 3, "asnAddNet: failed, invalid response from whois server."); - return 0; - } -- -- *t = '\0'; -- addr = as_string; -- bitl = atoi(t + 1); -- -- if (bitl < 0) -- bitl = 0; -+ const Ip::Address addr = addressToken.c_str(); - - // INET6 TODO : find a better way of identifying the base IPA family for mask than this. -- t = strchr(as_string, '.'); -+ const auto addrFamily = (addressToken.find('.') != SBuf::npos) ? AF_INET : AF_INET6; - - // generate Netbits Format Mask -+ Ip::Address mask; - mask.setNoAddr(); -- mask.applyMask(bitl, (t!=nullptr?AF_INET:AF_INET6) ); -+ int64_t bitl = 0; -+ if (tok.int64(bitl, 10, false)) -+ mask.applyMask(bitl, addrFamily); - - debugs(53, 3, "asnAddNet: called for " << addr << "/" << mask ); - -diff --git a/src/clientStream.cc b/src/clientStream.cc -index 31b85ca02f1..95da5eee670 100644 ---- a/src/clientStream.cc -+++ b/src/clientStream.cc -@@ -154,8 +154,7 @@ clientStreamCallback(clientStreamNode * thisObject, ClientHttpRequest * http, - assert(thisObject && http && thisObject->node.next); - next = thisObject->next(); - -- debugs(87, 3, "clientStreamCallback: Calling " << next->callback << " with cbdata " << -- next->data.getRaw() << " from node " << thisObject); -+ debugs(87, 3, thisObject << " gives " << next->data << ' ' << replyBuffer); - next->callback(next, http, rep, replyBuffer); - } - -diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc -index 53e64adb710..b8718c03799 100644 ---- a/src/client_side_reply.cc -+++ b/src/client_side_reply.cc -@@ -33,6 +33,7 @@ - #include "refresh.h" - #include "RequestFlags.h" - #include "SquidConfig.h" -+#include "SquidMath.h" - #include "Store.h" - #include "StrList.h" - #include "tools.h" -@@ -73,17 +74,12 @@ clientReplyContext::~clientReplyContext() - clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) : - purgeStatus(Http::scNone), - http(cbdataReference(clientContext)), -- headers_sz(0), - sc(nullptr), -- reqsize(0), -- reqofs(0), - ourNode(nullptr), - reply(nullptr), - old_entry(nullptr), - old_sc(nullptr), - old_lastmod(-1), -- old_reqofs(0), -- old_reqsize(0), - deleting(false), - collapsedRevalidation(crNone) - { -@@ -161,8 +157,6 @@ void clientReplyContext::setReplyToStoreEntry(StoreEntry *entry, const char *rea - #if USE_DELAY_POOLS - sc->setDelayId(DelayId::DelayClient(http)); - #endif -- reqofs = 0; -- reqsize = 0; - if (http->request) - http->request->ignoreRange(reason); - flags.storelogiccomplete = 1; -@@ -201,13 +195,9 @@ clientReplyContext::saveState() - old_sc = sc; - old_lastmod = http->request->lastmod; - old_etag = http->request->etag; -- old_reqsize = reqsize; -- old_reqofs = reqofs; - /* Prevent accessing the now saved entries */ - http->storeEntry(nullptr); - sc = nullptr; -- reqsize = 0; -- reqofs = 0; - } - - void -@@ -218,8 +208,6 @@ clientReplyContext::restoreState() - removeClientStoreReference(&sc, http); - http->storeEntry(old_entry); - sc = old_sc; -- reqsize = old_reqsize; -- reqofs = old_reqofs; - http->request->lastmod = old_lastmod; - http->request->etag = old_etag; - /* Prevent accessed the old saved entries */ -@@ -227,8 +215,6 @@ clientReplyContext::restoreState() - old_sc = nullptr; - old_lastmod = -1; - old_etag.clean(); -- old_reqsize = 0; -- old_reqofs = 0; - } - - void -@@ -245,18 +231,27 @@ clientReplyContext::getNextNode() const - return (clientStreamNode *)ourNode->node.next->data; - } - --/* This function is wrong - the client parameters don't include the -- * header offset -- */ -+/// Request HTTP response headers from Store, to be sent to the given recipient. -+/// That recipient also gets zero, some, or all HTTP response body bytes (into -+/// next()->readBuffer). - void --clientReplyContext::triggerInitialStoreRead() -+clientReplyContext::triggerInitialStoreRead(STCB recipient) - { -- /* when confident, 0 becomes reqofs, and then this factors into -- * startSendProcess -- */ -- assert(reqofs == 0); -+ Assure(recipient != HandleIMSReply); -+ lastStreamBufferedBytes = StoreIOBuffer(); // storeClientCopy(next()->readBuffer) invalidates - StoreIOBuffer localTempBuffer (next()->readBuffer.length, 0, next()->readBuffer.data); -- storeClientCopy(sc, http->storeEntry(), localTempBuffer, SendMoreData, this); -+ ::storeClientCopy(sc, http->storeEntry(), localTempBuffer, recipient, this); -+} -+ -+/// Request HTTP response body bytes from Store into next()->readBuffer. This -+/// method requests body bytes at readerBuffer.offset and, hence, it should only -+/// be called after we triggerInitialStoreRead() and get the requested HTTP -+/// response headers (using zero offset). -+void -+clientReplyContext::requestMoreBodyFromStore() -+{ -+ lastStreamBufferedBytes = StoreIOBuffer(); // storeClientCopy(next()->readBuffer) invalidates -+ ::storeClientCopy(sc, http->storeEntry(), next()->readBuffer, SendMoreData, this); - } - - /* there is an expired entry in the store. -@@ -363,30 +358,22 @@ clientReplyContext::processExpired() - { - /* start counting the length from 0 */ - StoreIOBuffer localTempBuffer(HTTP_REQBUF_SZ, 0, tempbuf); -- storeClientCopy(sc, entry, localTempBuffer, HandleIMSReply, this); -+ // keep lastStreamBufferedBytes: tempbuf is not a Client Stream buffer -+ ::storeClientCopy(sc, entry, localTempBuffer, HandleIMSReply, this); - } - } - - void --clientReplyContext::sendClientUpstreamResponse() -+clientReplyContext::sendClientUpstreamResponse(const StoreIOBuffer &upstreamResponse) - { -- StoreIOBuffer tempresult; - removeStoreReference(&old_sc, &old_entry); - - if (collapsedRevalidation) - http->storeEntry()->clearPublicKeyScope(); - - /* here the data to send is the data we just received */ -- old_reqofs = 0; -- old_reqsize = 0; -- /* sendMoreData tracks the offset as well. -- * Force it back to zero */ -- reqofs = 0; - assert(!EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED)); -- /* TODO: provide sendMoreData with the ready parsed reply */ -- tempresult.length = reqsize; -- tempresult.data = tempbuf; -- sendMoreData(tempresult); -+ sendMoreData(upstreamResponse); - } - - void -@@ -403,11 +390,9 @@ clientReplyContext::sendClientOldEntry() - restoreState(); - /* here the data to send is in the next nodes buffers already */ - assert(!EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED)); -- /* sendMoreData tracks the offset as well. -- * Force it back to zero */ -- reqofs = 0; -- StoreIOBuffer tempresult (reqsize, reqofs, next()->readBuffer.data); -- sendMoreData(tempresult); -+ Assure(matchesStreamBodyBuffer(lastStreamBufferedBytes)); -+ Assure(!lastStreamBufferedBytes.offset); -+ sendMoreData(lastStreamBufferedBytes); - } - - /* This is the workhorse of the HandleIMSReply callback. -@@ -416,16 +401,16 @@ clientReplyContext::sendClientOldEntry() - * IMS request to revalidate a stale entry. - */ - void --clientReplyContext::handleIMSReply(StoreIOBuffer result) -+clientReplyContext::handleIMSReply(const StoreIOBuffer result) - { - if (deleting) - return; - -- debugs(88, 3, http->storeEntry()->url() << ", " << (long unsigned) result.length << " bytes"); -- - if (http->storeEntry() == nullptr) - return; - -+ debugs(88, 3, http->storeEntry()->url() << " got " << result); -+ - if (result.flags.error && !EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED)) - return; - -@@ -438,9 +423,6 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result) - return; - } - -- /* update size of the request */ -- reqsize = result.length + reqofs; -- - // request to origin was aborted - if (EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED)) { - debugs(88, 3, "request to origin aborted '" << http->storeEntry()->url() << "', sending old entry to client"); -@@ -469,7 +451,7 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result) - if (http->request->flags.ims && !old_entry->modifiedSince(http->request->ims, http->request->imslen)) { - // forward the 304 from origin - debugs(88, 3, "origin replied 304, revalidated existing entry and forwarding 304 to client"); -- sendClientUpstreamResponse(); -+ sendClientUpstreamResponse(result); - return; - } - -@@ -494,7 +476,7 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result) - - http->updateLoggingTags(LOG_TCP_REFRESH_MODIFIED); - debugs(88, 3, "origin replied " << status << ", forwarding to client"); -- sendClientUpstreamResponse(); -+ sendClientUpstreamResponse(result); - return; - } - -@@ -502,7 +484,7 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result) - if (http->request->flags.failOnValidationError) { - http->updateLoggingTags(LOG_TCP_REFRESH_FAIL_ERR); - debugs(88, 3, "origin replied with error " << status << ", forwarding to client due to fail_on_validation_err"); -- sendClientUpstreamResponse(); -+ sendClientUpstreamResponse(result); - return; - } - -@@ -515,13 +497,7 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result) - SQUIDCEXTERN CSR clientGetMoreData; - SQUIDCEXTERN CSD clientReplyDetach; - --/** -- * clientReplyContext::cacheHit Should only be called until the HTTP reply headers -- * have been parsed. Normally this should be a single call, but -- * it might take more than one. As soon as we have the headers, -- * we hand off to clientSendMoreData, processExpired, or -- * processMiss. -- */ -+/// \copydoc clientReplyContext::cacheHit() - void - clientReplyContext::CacheHit(void *data, StoreIOBuffer result) - { -@@ -529,11 +505,11 @@ clientReplyContext::CacheHit(void *data, StoreIOBuffer result) - context->cacheHit(result); - } - --/** -- * Process a possible cache HIT. -- */ -+/// Processes HTTP response headers received from Store on a suspected cache hit -+/// path. May be called several times (e.g., a Vary marker object hit followed -+/// by the corresponding variant hit). - void --clientReplyContext::cacheHit(StoreIOBuffer result) -+clientReplyContext::cacheHit(const StoreIOBuffer result) - { - /** Ignore if the HIT object is being deleted. */ - if (deleting) { -@@ -545,7 +521,7 @@ clientReplyContext::cacheHit(StoreIOBuffer result) - - HttpRequest *r = http->request; - -- debugs(88, 3, "clientCacheHit: " << http->uri << ", " << result.length << " bytes"); -+ debugs(88, 3, http->uri << " got " << result); - - if (http->storeEntry() == nullptr) { - debugs(88, 3, "clientCacheHit: request aborted"); -@@ -569,20 +545,7 @@ clientReplyContext::cacheHit(StoreIOBuffer result) - return; - } - -- if (result.length == 0) { -- debugs(88, 5, "store IO buffer has no content. MISS"); -- /* the store couldn't get enough data from the file for us to id the -- * object -- */ -- /* treat as a miss */ -- http->updateLoggingTags(LOG_TCP_MISS); -- processMiss(); -- return; -- } -- - assert(!EBIT_TEST(e->flags, ENTRY_ABORTED)); -- /* update size of the request */ -- reqsize = result.length + reqofs; - - /* - * Got the headers, now grok them -@@ -596,6 +559,8 @@ clientReplyContext::cacheHit(StoreIOBuffer result) - return; - } - -+ noteStreamBufferredBytes(result); -+ - switch (varyEvaluateMatch(e, r)) { - - case VARY_NONE: -@@ -1018,16 +983,10 @@ clientReplyContext::purgeEntry(StoreEntry &entry, const Http::MethodType methodT - } - - void --clientReplyContext::traceReply(clientStreamNode * node) -+clientReplyContext::traceReply() - { -- clientStreamNode *nextNode = (clientStreamNode *)node->node.next->data; -- StoreIOBuffer localTempBuffer; - createStoreEntry(http->request->method, RequestFlags()); -- localTempBuffer.offset = nextNode->readBuffer.offset + headers_sz; -- localTempBuffer.length = nextNode->readBuffer.length; -- localTempBuffer.data = nextNode->readBuffer.data; -- storeClientCopy(sc, http->storeEntry(), -- localTempBuffer, SendMoreData, this); -+ triggerInitialStoreRead(); - http->storeEntry()->releaseRequest(); - http->storeEntry()->buffer(); - const HttpReplyPointer rep(new HttpReply); -@@ -1076,16 +1035,15 @@ int - clientReplyContext::storeOKTransferDone() const - { - assert(http->storeEntry()->objectLen() >= 0); -+ const auto headers_sz = http->storeEntry()->mem().baseReply().hdr_sz; - assert(http->storeEntry()->objectLen() >= headers_sz); -- if (http->out.offset >= http->storeEntry()->objectLen() - headers_sz) { -- debugs(88,3, "storeOKTransferDone " << -- " out.offset=" << http->out.offset << -- " objectLen()=" << http->storeEntry()->objectLen() << -- " headers_sz=" << headers_sz); -- return 1; -- } -- -- return 0; -+ const auto done = http->out.offset >= http->storeEntry()->objectLen() - headers_sz; -+ const auto debugLevel = done ? 3 : 5; -+ debugs(88, debugLevel, done << -+ " out.offset=" << http->out.offset << -+ " objectLen()=" << http->storeEntry()->objectLen() << -+ " headers_sz=" << headers_sz); -+ return done ? 1 : 0; - } - - int -@@ -1098,8 +1056,7 @@ clientReplyContext::storeNotOKTransferDone() const - assert(mem != nullptr); - assert(http->request != nullptr); - -- /* mem->reply was wrong because it uses the UPSTREAM header length!!! */ -- if (headers_sz == 0) -+ if (mem->baseReply().pstate != Http::Message::psParsed) - /* haven't found end of headers yet */ - return 0; - -@@ -1120,16 +1077,12 @@ clientReplyContext::storeNotOKTransferDone() const - if (expectedBodySize < 0) - return 0; - -- const uint64_t expectedLength = expectedBodySize + http->out.headers_sz; -- -- if (http->out.size < expectedLength) -- return 0; -- else { -- debugs(88,3, "storeNotOKTransferDone " << -- " out.size=" << http->out.size << -- " expectedLength=" << expectedLength); -- return 1; -- } -+ const auto done = http->out.offset >= expectedBodySize; -+ const auto debugLevel = done ? 3 : 5; -+ debugs(88, debugLevel, done << -+ " out.offset=" << http->out.offset << -+ " expectedBodySize=" << expectedBodySize); -+ return done ? 1 : 0; - } - - /* Preconditions: -@@ -1651,20 +1604,12 @@ clientGetMoreData(clientStreamNode * aNode, ClientHttpRequest * http) - assert (context); - assert(context->http == http); - -- clientStreamNode *next = ( clientStreamNode *)aNode->node.next->data; -- - if (!context->ourNode) - context->ourNode = aNode; - - /* no cbdatareference, this is only used once, and safely */ - if (context->flags.storelogiccomplete) { -- StoreIOBuffer tempBuffer; -- tempBuffer.offset = next->readBuffer.offset + context->headers_sz; -- tempBuffer.length = next->readBuffer.length; -- tempBuffer.data = next->readBuffer.data; -- -- storeClientCopy(context->sc, http->storeEntry(), -- tempBuffer, clientReplyContext::SendMoreData, context); -+ context->requestMoreBodyFromStore(); - return; - } - -@@ -1677,7 +1622,7 @@ clientGetMoreData(clientStreamNode * aNode, ClientHttpRequest * http) - - if (context->http->request->method == Http::METHOD_TRACE) { - if (context->http->request->header.getInt64(Http::HdrType::MAX_FORWARDS) == 0) { -- context->traceReply(aNode); -+ context->traceReply(); - return; - } - -@@ -1707,7 +1652,6 @@ clientReplyContext::doGetMoreData() - #endif - - assert(http->loggingTags().oldType == LOG_TCP_HIT); -- reqofs = 0; - /* guarantee nothing has been sent yet! */ - assert(http->out.size == 0); - assert(http->out.offset == 0); -@@ -1722,10 +1666,7 @@ clientReplyContext::doGetMoreData() - } - } - -- localTempBuffer.offset = reqofs; -- localTempBuffer.length = getNextNode()->readBuffer.length; -- localTempBuffer.data = getNextNode()->readBuffer.data; -- storeClientCopy(sc, http->storeEntry(), localTempBuffer, CacheHit, this); -+ triggerInitialStoreRead(CacheHit); - } else { - /* MISS CASE, http->loggingTags() are already set! */ - processMiss(); -@@ -1760,12 +1701,11 @@ clientReplyContext::makeThisHead() - } - - bool --clientReplyContext::errorInStream(StoreIOBuffer const &result, size_t const &sizeToProcess)const -+clientReplyContext::errorInStream(const StoreIOBuffer &result) const - { - return /* aborted request */ - (http->storeEntry() && EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED)) || -- /* Upstream read error */ (result.flags.error) || -- /* Upstream EOF */ (sizeToProcess == 0); -+ /* Upstream read error */ (result.flags.error); - } - - void -@@ -1786,24 +1726,16 @@ clientReplyContext::sendStreamError(StoreIOBuffer const &result) - } - - void --clientReplyContext::pushStreamData(StoreIOBuffer const &result, char *source) -+clientReplyContext::pushStreamData(const StoreIOBuffer &result) - { -- StoreIOBuffer localTempBuffer; -- - if (result.length == 0) { - debugs(88, 5, "clientReplyContext::pushStreamData: marking request as complete due to 0 length store result"); - flags.complete = 1; - } - -- assert(result.offset - headers_sz == next()->readBuffer.offset); -- localTempBuffer.offset = result.offset - headers_sz; -- localTempBuffer.length = result.length; -- -- if (localTempBuffer.length) -- localTempBuffer.data = source; -- -+ assert(!result.length || result.offset == next()->readBuffer.offset); - clientStreamCallback((clientStreamNode*)http->client_stream.head->data, http, nullptr, -- localTempBuffer); -+ result); - } - - clientStreamNode * -@@ -1890,7 +1822,6 @@ clientReplyContext::processReplyAccess () - if (http->loggingTags().oldType == LOG_TCP_DENIED || - http->loggingTags().oldType == LOG_TCP_DENIED_REPLY || - alwaysAllowResponse(reply->sline.status())) { -- headers_sz = reply->hdr_sz; - processReplyAccessResult(ACCESS_ALLOWED); - return; - } -@@ -1901,8 +1832,6 @@ clientReplyContext::processReplyAccess () - return; - } - -- headers_sz = reply->hdr_sz; -- - /** check for absent access controls (permit by default) */ - if (!Config.accessList.reply) { - processReplyAccessResult(ACCESS_ALLOWED); -@@ -1956,11 +1885,9 @@ clientReplyContext::processReplyAccessResult(const Acl::Answer &accessAllowed) - /* Ok, the reply is allowed, */ - http->loggingEntry(http->storeEntry()); - -- ssize_t body_size = reqofs - reply->hdr_sz; -- if (body_size < 0) { -- reqofs = reply->hdr_sz; -- body_size = 0; -- } -+ Assure(matchesStreamBodyBuffer(lastStreamBufferedBytes)); -+ Assure(!lastStreamBufferedBytes.offset); -+ auto body_size = lastStreamBufferedBytes.length; // may be zero - - debugs(88, 3, "clientReplyContext::sendMoreData: Appending " << - (int) body_size << " bytes after " << reply->hdr_sz << -@@ -1988,19 +1915,27 @@ clientReplyContext::processReplyAccessResult(const Acl::Answer &accessAllowed) - assert (!flags.headersSent); - flags.headersSent = true; - -+ // next()->readBuffer.offset may be positive for Range requests, but our -+ // localTempBuffer initialization code assumes that next()->readBuffer.data -+ // points to the response body at offset 0 because the first -+ // storeClientCopy() request always has offset 0 (i.e. our first Store -+ // request ignores next()->readBuffer.offset). -+ // -+ // XXX: We cannot fully check that assumption: readBuffer.offset field is -+ // often out of sync with the buffer content, and if some buggy code updates -+ // the buffer while we were waiting for the processReplyAccessResult() -+ // callback, we may not notice. -+ - StoreIOBuffer localTempBuffer; -- char *buf = next()->readBuffer.data; -- char *body_buf = buf + reply->hdr_sz; -+ const auto body_buf = next()->readBuffer.data; - - //Server side may disable ranges under some circumstances. - - if ((!http->request->range)) - next()->readBuffer.offset = 0; - -- body_buf -= next()->readBuffer.offset; -- -- if (next()->readBuffer.offset != 0) { -- if (next()->readBuffer.offset > body_size) { -+ if (next()->readBuffer.offset > 0) { -+ if (Less(body_size, next()->readBuffer.offset)) { - /* Can't use any of the body we received. send nothing */ - localTempBuffer.length = 0; - localTempBuffer.data = nullptr; -@@ -2013,7 +1948,6 @@ clientReplyContext::processReplyAccessResult(const Acl::Answer &accessAllowed) - localTempBuffer.data = body_buf; - } - -- /* TODO??: move the data in the buffer back by the request header size */ - clientStreamCallback((clientStreamNode *)http->client_stream.head->data, - http, reply, localTempBuffer); - -@@ -2026,6 +1960,8 @@ clientReplyContext::sendMoreData (StoreIOBuffer result) - if (deleting) - return; - -+ debugs(88, 5, http->uri << " got " << result); -+ - StoreEntry *entry = http->storeEntry(); - - if (ConnStateData * conn = http->getConn()) { -@@ -2038,7 +1974,9 @@ clientReplyContext::sendMoreData (StoreIOBuffer result) - return; - } - -- if (reqofs==0 && !http->loggingTags().isTcpHit()) { -+ if (!flags.headersSent && !http->loggingTags().isTcpHit()) { -+ // We get here twice if processReplyAccessResult() calls startError(). -+ // TODO: Revise when we check/change QoS markings to reduce syscalls. - if (Ip::Qos::TheConfig.isHitTosActive()) { - Ip::Qos::doTosLocalMiss(conn->clientConnection, http->request->hier.code); - } -@@ -2052,21 +1990,9 @@ clientReplyContext::sendMoreData (StoreIOBuffer result) - " out.offset=" << http->out.offset); - } - -- char *buf = next()->readBuffer.data; -- -- if (buf != result.data) { -- /* we've got to copy some data */ -- assert(result.length <= next()->readBuffer.length); -- memcpy(buf, result.data, result.length); -- } -- - /* We've got the final data to start pushing... */ - flags.storelogiccomplete = 1; - -- reqofs += result.length; -- -- assert(reqofs <= HTTP_REQBUF_SZ || flags.headersSent); -- - assert(http->request != nullptr); - - /* ESI TODO: remove this assert once everything is stable */ -@@ -2075,20 +2001,25 @@ clientReplyContext::sendMoreData (StoreIOBuffer result) - - makeThisHead(); - -- debugs(88, 5, "clientReplyContext::sendMoreData: " << http->uri << ", " << -- reqofs << " bytes (" << result.length << -- " new bytes)"); -- -- /* update size of the request */ -- reqsize = reqofs; -- -- if (errorInStream(result, reqofs)) { -+ if (errorInStream(result)) { - sendStreamError(result); - return; - } - -+ if (!matchesStreamBodyBuffer(result)) { -+ // Subsequent processing expects response body bytes to be at the start -+ // of our Client Stream buffer. When given something else (e.g., bytes -+ // in our tempbuf), we copy and adjust to meet those expectations. -+ const auto &ourClientStreamsBuffer = next()->readBuffer; -+ assert(result.length <= ourClientStreamsBuffer.length); -+ memcpy(ourClientStreamsBuffer.data, result.data, result.length); -+ result.data = ourClientStreamsBuffer.data; -+ } -+ -+ noteStreamBufferredBytes(result); -+ - if (flags.headersSent) { -- pushStreamData (result, buf); -+ pushStreamData(result); - return; - } - -@@ -2103,6 +2034,32 @@ clientReplyContext::sendMoreData (StoreIOBuffer result) - return; - } - -+/// Whether the given body area describes the start of our Client Stream buffer. -+/// An empty area does. -+bool -+clientReplyContext::matchesStreamBodyBuffer(const StoreIOBuffer &their) const -+{ -+ // the answer is undefined for errors; they are not really "body buffers" -+ Assure(!their.flags.error); -+ -+ if (!their.length) -+ return true; // an empty body area always matches our body area -+ -+ if (their.data != next()->readBuffer.data) { -+ debugs(88, 7, "no: " << their << " vs. " << next()->readBuffer); -+ return false; -+ } -+ -+ return true; -+} -+ -+void -+clientReplyContext::noteStreamBufferredBytes(const StoreIOBuffer &result) -+{ -+ Assure(matchesStreamBodyBuffer(result)); -+ lastStreamBufferedBytes = result; // may be unchanged and/or zero-length -+} -+ - void - clientReplyContext::fillChecklist(ACLFilledChecklist &checklist) const - { -@@ -2149,13 +2106,6 @@ clientReplyContext::createStoreEntry(const HttpRequestMethod& m, RequestFlags re - sc->setDelayId(DelayId::DelayClient(http)); - #endif - -- reqofs = 0; -- -- reqsize = 0; -- -- /* I don't think this is actually needed! -- adrian */ -- /* http->reqbuf = http->norm_reqbuf; */ -- // assert(http->reqbuf == http->norm_reqbuf); - /* The next line is illegal because we don't know if the client stream - * buffers have been set up - */ -diff --git a/src/client_side_reply.h b/src/client_side_reply.h -index 32a38bc95e1..488dc2a7fbd 100644 ---- a/src/client_side_reply.h -+++ b/src/client_side_reply.h -@@ -34,7 +34,6 @@ class clientReplyContext : public RefCountable, public StoreClient - void saveState(); - void restoreState(); - void purgeRequest (); -- void sendClientUpstreamResponse(); - void doGetMoreData(); - void identifyStoreObject(); - void identifyFoundObject(StoreEntry *entry, const char *detail); -@@ -60,7 +59,7 @@ class clientReplyContext : public RefCountable, public StoreClient - void processExpired(); - clientStream_status_t replyStatus(); - void processMiss(); -- void traceReply(clientStreamNode * node); -+ void traceReply(); - const char *storeId() const { return (http->store_id.size() > 0 ? http->store_id.termedBuf() : http->uri); } - - Http::StatusCode purgeStatus; -@@ -69,14 +68,13 @@ class clientReplyContext : public RefCountable, public StoreClient - LogTags *loggingTags() const override; - - ClientHttpRequest *http; -- /// Base reply header bytes received from Store. -- /// Compatible with ClientHttpRequest::Out::offset. -- /// Not to be confused with ClientHttpRequest::Out::headers_sz. -- int headers_sz; - store_client *sc; /* The store_client we're using */ -- size_t reqsize; -- size_t reqofs; -- char tempbuf[HTTP_REQBUF_SZ]; ///< a temporary buffer if we need working storage -+ -+ /// Buffer dedicated to receiving storeClientCopy() responses to generated -+ /// revalidation requests. These requests cannot use next()->readBuffer -+ /// because the latter keeps the contents of the stale HTTP response during -+ /// revalidation. sendClientOldEntry() uses that contents. -+ char tempbuf[HTTP_REQBUF_SZ]; - - struct Flags { - Flags() : storelogiccomplete(0), complete(0), headersSent(false) {} -@@ -93,9 +91,10 @@ class clientReplyContext : public RefCountable, public StoreClient - - clientStreamNode *getNextNode() const; - void makeThisHead(); -- bool errorInStream(StoreIOBuffer const &result, size_t const &sizeToProcess)const ; -+ bool errorInStream(const StoreIOBuffer &result) const; -+ bool matchesStreamBodyBuffer(const StoreIOBuffer &) const; - void sendStreamError(StoreIOBuffer const &result); -- void pushStreamData(StoreIOBuffer const &result, char *source); -+ void pushStreamData(const StoreIOBuffer &); - clientStreamNode * next() const; - HttpReply *reply; - void processReplyAccess(); -@@ -107,10 +106,12 @@ class clientReplyContext : public RefCountable, public StoreClient - int checkTransferDone(); - void processOnlyIfCachedMiss(); - bool processConditional(); -+ void noteStreamBufferredBytes(const StoreIOBuffer &); - void cacheHit(StoreIOBuffer result); - void handleIMSReply(StoreIOBuffer result); - void sendMoreData(StoreIOBuffer result); -- void triggerInitialStoreRead(); -+ void triggerInitialStoreRead(STCB = SendMoreData); -+ void requestMoreBodyFromStore(); - void sendClientOldEntry(); - void purgeAllCached(); - /// attempts to release the cached entry -@@ -127,6 +128,13 @@ class clientReplyContext : public RefCountable, public StoreClient - void sendPreconditionFailedError(); - void sendNotModified(); - void sendNotModifiedOrPreconditionFailedError(); -+ void sendClientUpstreamResponse(const StoreIOBuffer &upstreamResponse); -+ -+ /// Reduces a chance of an accidental direct storeClientCopy() call that -+ /// (should but) forgets to invalidate our lastStreamBufferedBytes. This -+ /// function is not defined; decltype() syntax prohibits "= delete", but -+ /// function usage will trigger deprecation warnings and linking errors. -+ static decltype(::storeClientCopy) storeClientCopy [[deprecated]]; - - /// Classification of the initial Store lookup. - /// This very first lookup happens without the Vary-driven key augmentation. -@@ -138,8 +146,6 @@ class clientReplyContext : public RefCountable, public StoreClient - store_client *old_sc; - time_t old_lastmod; - String old_etag; -- size_t old_reqofs; -- size_t old_reqsize; - - bool deleting; - -@@ -150,6 +156,12 @@ class clientReplyContext : public RefCountable, public StoreClient - } CollapsedRevalidation; - - CollapsedRevalidation collapsedRevalidation; -+ -+ /// HTTP response body bytes stored in our Client Stream buffer (if any) -+ StoreIOBuffer lastStreamBufferedBytes; -+ -+ // TODO: Remove after moving the meat of this function into a method. -+ friend CSR clientGetMoreData; - }; - - // TODO: move to SideAgent parent, when we have one -diff --git a/src/client_side_request.h b/src/client_side_request.h -index e610a4bcb4b..7db65d3721a 100644 ---- a/src/client_side_request.h -+++ b/src/client_side_request.h -@@ -147,7 +147,6 @@ class ClientHttpRequest - /// Response header and body bytes written to the client connection. - uint64_t size = 0; - /// Response header bytes written to the client connection. -- /// Not to be confused with clientReplyContext::headers_sz. - size_t headers_sz = 0; - } out; - -diff --git a/src/enums.h b/src/enums.h -index 1b2865413e0..4ad4f0c9434 100644 ---- a/src/enums.h -+++ b/src/enums.h -@@ -202,7 +202,6 @@ enum { - typedef enum { - DIGEST_READ_NONE, - DIGEST_READ_REPLY, -- DIGEST_READ_HEADERS, - DIGEST_READ_CBLOCK, - DIGEST_READ_MASK, - DIGEST_READ_DONE -diff --git a/src/icmp/net_db.cc b/src/icmp/net_db.cc -index 30865aaf34d..702918b9808 100644 ---- a/src/icmp/net_db.cc -+++ b/src/icmp/net_db.cc -@@ -33,6 +33,7 @@ - #include "mime_header.h" - #include "neighbors.h" - #include "PeerSelectState.h" -+#include "sbuf/SBuf.h" - #include "SquidConfig.h" - #include "Store.h" - #include "StoreClient.h" -@@ -48,8 +49,6 @@ - #include "ipcache.h" - #include "StoreClient.h" - --#define NETDB_REQBUF_SZ 4096 -- - typedef enum { - STATE_NONE, - STATE_HEADER, -@@ -65,7 +64,6 @@ class netdbExchangeState - p(aPeer), - r(theReq) - { -- *buf = 0; - assert(r); - // TODO: check if we actually need to do this. should be implicit - r->http_ver = Http::ProtocolVersion(); -@@ -81,10 +79,10 @@ class netdbExchangeState - StoreEntry *e = nullptr; - store_client *sc = nullptr; - HttpRequestPointer r; -- int64_t used = 0; -- size_t buf_sz = NETDB_REQBUF_SZ; -- char buf[NETDB_REQBUF_SZ]; -- int buf_ofs = 0; -+ -+ /// for receiving a NetDB reply body from Store and interpreting it -+ Store::ParsingBuffer parsingBuffer; -+ - netdb_conn_state_t connstate = STATE_HEADER; - }; - -@@ -667,23 +665,19 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData) - Ip::Address addr; - - netdbExchangeState *ex = (netdbExchangeState *)data; -- int rec_sz = 0; -- int o; - - struct in_addr line_addr; - double rtt; - double hops; -- char *p; - int j; -- size_t hdr_sz; - int nused = 0; -- int size; -- int oldbufofs = ex->buf_ofs; - -- rec_sz = 0; -+ size_t rec_sz = 0; // received record size (TODO: make const) - rec_sz += 1 + sizeof(struct in_addr); - rec_sz += 1 + sizeof(int); - rec_sz += 1 + sizeof(int); -+ // to make progress without growing buffer space, we must parse at least one record per call -+ Assure(rec_sz <= ex->parsingBuffer.capacity()); - debugs(38, 3, "netdbExchangeHandleReply: " << receivedData.length << " read bytes"); - - if (!ex->p.valid()) { -@@ -694,64 +688,28 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData) - - debugs(38, 3, "for " << *ex->p); - -- if (receivedData.length == 0 && !receivedData.flags.error) { -- debugs(38, 3, "netdbExchangeHandleReply: Done"); -+ if (receivedData.flags.error) { - delete ex; - return; - } - -- p = ex->buf; -- -- /* Get the size of the buffer now */ -- size = ex->buf_ofs + receivedData.length; -- debugs(38, 3, "netdbExchangeHandleReply: " << size << " bytes buf"); -- -- /* Check if we're still doing headers */ -- - if (ex->connstate == STATE_HEADER) { -- -- ex->buf_ofs += receivedData.length; -- -- /* skip reply headers */ -- -- if ((hdr_sz = headersEnd(p, ex->buf_ofs))) { -- debugs(38, 5, "netdbExchangeHandleReply: hdr_sz = " << hdr_sz); -- const auto scode = ex->e->mem().baseReply().sline.status(); -- assert(scode != Http::scNone); -- debugs(38, 3, "netdbExchangeHandleReply: reply status " << scode); -- -- if (scode != Http::scOkay) { -- delete ex; -- return; -- } -- -- assert((size_t)ex->buf_ofs >= hdr_sz); -- -- /* -- * Now, point p to the part of the buffer where the data -- * starts, and update the size accordingly -- */ -- assert(ex->used == 0); -- ex->used = hdr_sz; -- size = ex->buf_ofs - hdr_sz; -- p += hdr_sz; -- -- /* Finally, set the conn state mode to STATE_BODY */ -- ex->connstate = STATE_BODY; -- } else { -- StoreIOBuffer tempBuffer; -- tempBuffer.offset = ex->buf_ofs; -- tempBuffer.length = ex->buf_sz - ex->buf_ofs; -- tempBuffer.data = ex->buf + ex->buf_ofs; -- /* Have more headers .. */ -- storeClientCopy(ex->sc, ex->e, tempBuffer, -- netdbExchangeHandleReply, ex); -+ const auto scode = ex->e->mem().baseReply().sline.status(); -+ assert(scode != Http::scNone); -+ debugs(38, 3, "reply status " << scode); -+ if (scode != Http::scOkay) { -+ delete ex; - return; - } -+ ex->connstate = STATE_BODY; - } - - assert(ex->connstate == STATE_BODY); - -+ ex->parsingBuffer.appended(receivedData.data, receivedData.length); -+ auto p = ex->parsingBuffer.c_str(); // current parsing position -+ auto size = ex->parsingBuffer.contentSize(); // bytes we still need to parse -+ - /* If we get here, we have some body to parse .. */ - debugs(38, 5, "netdbExchangeHandleReply: start parsing loop, size = " << size); - -@@ -760,6 +718,7 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData) - addr.setAnyAddr(); - hops = rtt = 0.0; - -+ size_t o; // current record parsing offset - for (o = 0; o < rec_sz;) { - switch ((int) *(p + o)) { - -@@ -797,8 +756,6 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData) - - assert(o == rec_sz); - -- ex->used += rec_sz; -- - size -= rec_sz; - - p += rec_sz; -@@ -806,32 +763,8 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData) - ++nused; - } - -- /* -- * Copy anything that is left over to the beginning of the buffer, -- * and adjust buf_ofs accordingly -- */ -- -- /* -- * Evilly, size refers to the buf size left now, -- * ex->buf_ofs is the original buffer size, so just copy that -- * much data over -- */ -- memmove(ex->buf, ex->buf + (ex->buf_ofs - size), size); -- -- ex->buf_ofs = size; -- -- /* -- * And don't re-copy the remaining data .. -- */ -- ex->used += size; -- -- /* -- * Now the tricky bit - size _included_ the leftover bit from the _last_ -- * storeClientCopy. We don't want to include that, or our offset will be wrong. -- * So, don't count the size of the leftover buffer we began with. -- * This can _disappear_ when we're not tracking offsets .. -- */ -- ex->used -= oldbufofs; -+ const auto parsedSize = ex->parsingBuffer.contentSize() - size; -+ ex->parsingBuffer.consume(parsedSize); - - debugs(38, 3, "netdbExchangeHandleReply: size left over in this buffer: " << size << " bytes"); - -@@ -839,20 +772,26 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData) - " entries, (x " << rec_sz << " bytes) == " << nused * rec_sz << - " bytes total"); - -- debugs(38, 3, "netdbExchangeHandleReply: used " << ex->used); -- - if (EBIT_TEST(ex->e->flags, ENTRY_ABORTED)) { - debugs(38, 3, "netdbExchangeHandleReply: ENTRY_ABORTED"); - delete ex; -- } else if (ex->e->store_status == STORE_PENDING) { -- StoreIOBuffer tempBuffer; -- tempBuffer.offset = ex->used; -- tempBuffer.length = ex->buf_sz - ex->buf_ofs; -- tempBuffer.data = ex->buf + ex->buf_ofs; -- debugs(38, 3, "netdbExchangeHandleReply: EOF not received"); -- storeClientCopy(ex->sc, ex->e, tempBuffer, -- netdbExchangeHandleReply, ex); -+ return; - } -+ -+ if (ex->sc->atEof()) { -+ if (const auto leftoverBytes = ex->parsingBuffer.contentSize()) -+ debugs(38, 2, "discarding a partially received record due to Store EOF: " << leftoverBytes); -+ delete ex; -+ return; -+ } -+ -+ // TODO: To protect us from a broken peer sending an "infinite" stream of -+ // new addresses, limit the cumulative number of received bytes or records? -+ -+ const auto remainingSpace = ex->parsingBuffer.space().positionAt(receivedData.offset + receivedData.length); -+ // rec_sz is at most buffer capacity, and we consume all fully loaded records -+ Assure(remainingSpace.length); -+ storeClientCopy(ex->sc, ex->e, remainingSpace, netdbExchangeHandleReply, ex); - } - - #endif /* USE_ICMP */ -@@ -1273,14 +1212,9 @@ netdbExchangeStart(void *data) - ex->e = storeCreateEntry(uri, uri, RequestFlags(), Http::METHOD_GET); - assert(nullptr != ex->e); - -- StoreIOBuffer tempBuffer; -- tempBuffer.length = ex->buf_sz; -- tempBuffer.data = ex->buf; -- - ex->sc = storeClientListAdd(ex->e, ex); -+ storeClientCopy(ex->sc, ex->e, ex->parsingBuffer.makeInitialSpace(), netdbExchangeHandleReply, ex); - -- storeClientCopy(ex->sc, ex->e, tempBuffer, -- netdbExchangeHandleReply, ex); - ex->r->flags.loopDetected = true; /* cheat! -- force direct */ - - // XXX: send as Proxy-Authenticate instead -diff --git a/src/peer_digest.cc b/src/peer_digest.cc -index 2788fad7765..836e6b1d726 100644 ---- a/src/peer_digest.cc -+++ b/src/peer_digest.cc -@@ -39,7 +39,6 @@ static EVH peerDigestCheck; - static void peerDigestRequest(PeerDigest * pd); - static STCB peerDigestHandleReply; - static int peerDigestFetchReply(void *, char *, ssize_t); --int peerDigestSwapInHeaders(void *, char *, ssize_t); - int peerDigestSwapInCBlock(void *, char *, ssize_t); - int peerDigestSwapInMask(void *, char *, ssize_t); - static int peerDigestFetchedEnough(DigestFetchState * fetch, char *buf, ssize_t size, const char *step_name); -@@ -308,6 +307,9 @@ peerDigestRequest(PeerDigest * pd) - fetch->sc = storeClientListAdd(e, fetch); - /* set lastmod to trigger IMS request if possible */ - -+ // TODO: Also check for fetch->pd->cd presence as a precondition for sending -+ // IMS requests because peerDigestFetchReply() does not accept 304 responses -+ // without an in-memory cache digest. - if (old_e) - e->lastModified(old_e->lastModified()); - -@@ -342,6 +344,11 @@ peerDigestHandleReply(void *data, StoreIOBuffer receivedData) - digest_read_state_t prevstate; - int newsize; - -+ if (receivedData.flags.error) { -+ peerDigestFetchAbort(fetch, fetch->buf, "failure loading digest reply from Store"); -+ return; -+ } -+ - assert(fetch->pd && receivedData.data); - /* The existing code assumes that the received pointer is - * where we asked the data to be put -@@ -378,10 +385,6 @@ peerDigestHandleReply(void *data, StoreIOBuffer receivedData) - retsize = peerDigestFetchReply(fetch, fetch->buf, fetch->bufofs); - break; - -- case DIGEST_READ_HEADERS: -- retsize = peerDigestSwapInHeaders(fetch, fetch->buf, fetch->bufofs); -- break; -- - case DIGEST_READ_CBLOCK: - retsize = peerDigestSwapInCBlock(fetch, fetch->buf, fetch->bufofs); - break; -@@ -421,7 +424,7 @@ peerDigestHandleReply(void *data, StoreIOBuffer receivedData) - // checking at the beginning of this function. However, in this case, we would have to require - // that the parser does not regard EOF as a special condition (it is true now but may change - // in the future). -- if (!receivedData.length) { // EOF -+ if (fetch->sc->atEof()) { - peerDigestFetchAbort(fetch, fetch->buf, "premature end of digest reply"); - return; - } -@@ -440,19 +443,12 @@ peerDigestHandleReply(void *data, StoreIOBuffer receivedData) - } - } - --/* wait for full http headers to be received then parse them */ --/* -- * This routine handles parsing the reply line. -- * If the reply line indicates an OK, the same data is thrown -- * to SwapInHeaders(). If the reply line is a NOT_MODIFIED, -- * we simply stop parsing. -- */ -+/// handle HTTP response headers in the initial storeClientCopy() response - static int - peerDigestFetchReply(void *data, char *buf, ssize_t size) - { - DigestFetchState *fetch = (DigestFetchState *)data; - PeerDigest *pd = fetch->pd; -- size_t hdr_size; - assert(pd && buf); - assert(!fetch->offset); - -@@ -461,7 +457,7 @@ peerDigestFetchReply(void *data, char *buf, ssize_t size) - if (peerDigestFetchedEnough(fetch, buf, size, "peerDigestFetchReply")) - return -1; - -- if ((hdr_size = headersEnd(buf, size))) { -+ { - const auto &reply = fetch->entry->mem().freshestReply(); - const auto status = reply.sline.status(); - assert(status != Http::scNone); -@@ -494,6 +490,15 @@ peerDigestFetchReply(void *data, char *buf, ssize_t size) - /* preserve request -- we need its size to update counters */ - /* requestUnlink(r); */ - /* fetch->entry->mem_obj->request = NULL; */ -+ -+ if (!fetch->pd->cd) { -+ peerDigestFetchAbort(fetch, buf, "304 without the old in-memory digest"); -+ return -1; -+ } -+ -+ // stay with the old in-memory digest -+ peerDigestFetchStop(fetch, buf, "Not modified"); -+ fetch->state = DIGEST_READ_DONE; - } else if (status == Http::scOkay) { - /* get rid of old entry if any */ - -@@ -504,70 +509,16 @@ peerDigestFetchReply(void *data, char *buf, ssize_t size) - fetch->old_entry->unlock("peerDigestFetchReply 200"); - fetch->old_entry = nullptr; - } -+ -+ fetch->state = DIGEST_READ_CBLOCK; - } else { - /* some kind of a bug */ - peerDigestFetchAbort(fetch, buf, reply.sline.reason()); - return -1; /* XXX -1 will abort stuff in ReadReply! */ - } -- -- /* must have a ready-to-use store entry if we got here */ -- /* can we stay with the old in-memory digest? */ -- if (status == Http::scNotModified && fetch->pd->cd) { -- peerDigestFetchStop(fetch, buf, "Not modified"); -- fetch->state = DIGEST_READ_DONE; -- } else { -- fetch->state = DIGEST_READ_HEADERS; -- } -- } else { -- /* need more data, do we have space? */ -- -- if (size >= SM_PAGE_SIZE) -- peerDigestFetchAbort(fetch, buf, "reply header too big"); -- } -- -- /* We don't want to actually ack that we've handled anything, -- * otherwise SwapInHeaders() won't get the reply line .. */ -- return 0; --} -- --/* fetch headers from disk, pass on to SwapInCBlock */ --int --peerDigestSwapInHeaders(void *data, char *buf, ssize_t size) --{ -- DigestFetchState *fetch = (DigestFetchState *)data; -- size_t hdr_size; -- -- assert(fetch->state == DIGEST_READ_HEADERS); -- -- if (peerDigestFetchedEnough(fetch, buf, size, "peerDigestSwapInHeaders")) -- return -1; -- -- assert(!fetch->offset); -- -- if ((hdr_size = headersEnd(buf, size))) { -- const auto &reply = fetch->entry->mem().freshestReply(); -- const auto status = reply.sline.status(); -- assert(status != Http::scNone); -- -- if (status != Http::scOkay) { -- debugs(72, DBG_IMPORTANT, "peerDigestSwapInHeaders: " << fetch->pd->host << -- " status " << status << " got cached!"); -- -- peerDigestFetchAbort(fetch, buf, "internal status error"); -- return -1; -- } -- -- fetch->state = DIGEST_READ_CBLOCK; -- return hdr_size; /* Say how much data we read */ -- } -- -- /* need more data, do we have space? */ -- if (size >= SM_PAGE_SIZE) { -- peerDigestFetchAbort(fetch, buf, "stored header too big"); -- return -1; - } - -- return 0; /* We need to read more to parse .. */ -+ return 0; // we consumed/used no buffered bytes - } - - int -diff --git a/src/store.cc b/src/store.cc -index 00d7aed1d13..889512520d7 100644 ---- a/src/store.cc -+++ b/src/store.cc -@@ -256,6 +256,8 @@ StoreEntry::storeClientType() const - - assert(mem_obj); - -+ debugs(20, 7, *this << " inmem_lo=" << mem_obj->inmem_lo); -+ - if (mem_obj->inmem_lo) - return STORE_DISK_CLIENT; - -@@ -283,6 +285,7 @@ StoreEntry::storeClientType() const - return STORE_MEM_CLIENT; - } - } -+ debugs(20, 7, "STORE_OK STORE_DISK_CLIENT"); - return STORE_DISK_CLIENT; - } - -@@ -302,10 +305,18 @@ StoreEntry::storeClientType() const - if (swap_status == SWAPOUT_NONE) - return STORE_MEM_CLIENT; - -+ // TODO: The above "must make this a mem client" logic contradicts "Slight -+ // weirdness" logic in store_client::doCopy() that converts hits to misses -+ // on startSwapin() failures. We should probably attempt to open a swapin -+ // file _here_ instead (and avoid STORE_DISK_CLIENT designation for clients -+ // that fail to do so). That would also address a similar problem with Rock -+ // store that does not yet support swapin during SWAPOUT_WRITING. -+ - /* - * otherwise, make subsequent clients read from disk so they - * can not delay the first, and vice-versa. - */ -+ debugs(20, 7, "STORE_PENDING STORE_DISK_CLIENT"); - return STORE_DISK_CLIENT; - } - -diff --git a/src/store/Makefile.am b/src/store/Makefile.am -index 508705b0168..1830f42eaee 100644 ---- a/src/store/Makefile.am -+++ b/src/store/Makefile.am -@@ -22,6 +22,8 @@ libstore_la_SOURCES = \ - Disks.h \ - LocalSearch.cc \ - LocalSearch.h \ -+ ParsingBuffer.cc \ -+ ParsingBuffer.h \ - Storage.h \ - SwapMeta.cc \ - SwapMeta.h \ -diff --git a/src/store/ParsingBuffer.cc b/src/store/ParsingBuffer.cc -new file mode 100644 -index 00000000000..77b57408595 ---- /dev/null -+++ b/src/store/ParsingBuffer.cc -@@ -0,0 +1,198 @@ -+/* -+ * Copyright (C) 1996-2023 The Squid Software Foundation and contributors -+ * -+ * Squid software is distributed under GPLv2+ license and includes -+ * contributions from numerous individuals and organizations. -+ * Please see the COPYING and CONTRIBUTORS files for details. -+ */ -+ -+#include "squid.h" -+#include "sbuf/Stream.h" -+#include "SquidMath.h" -+#include "store/ParsingBuffer.h" -+ -+#include -+ -+// Several Store::ParsingBuffer() methods use assert() because the corresponding -+// failure means there is a good chance that somebody have already read from (or -+// written to) the wrong memory location. Since this buffer is used for storing -+// HTTP response bytes, such failures may corrupt traffic. No Assure() handling -+// code can safely recover from such failures. -+ -+Store::ParsingBuffer::ParsingBuffer(StoreIOBuffer &initialSpace): -+ readerSuppliedMemory_(initialSpace) -+{ -+} -+ -+/// a read-only content start (or nil for some zero-size buffers) -+const char * -+Store::ParsingBuffer::memory() const -+{ -+ return extraMemory_ ? extraMemory_->rawContent() : readerSuppliedMemory_.data; -+} -+ -+size_t -+Store::ParsingBuffer::capacity() const -+{ -+ return extraMemory_ ? (extraMemory_->length() + extraMemory_->spaceSize()) : readerSuppliedMemory_.length; -+} -+ -+size_t -+Store::ParsingBuffer::contentSize() const -+{ -+ return extraMemory_ ? extraMemory_->length() : readerSuppliedMemoryContentSize_; -+} -+ -+void -+Store::ParsingBuffer::appended(const char * const newBytes, const size_t newByteCount) -+{ -+ // a positive newByteCount guarantees that, after the first assertion below -+ // succeeds, the second assertion will not increment a nil memory() pointer -+ if (!newByteCount) -+ return; -+ -+ // these checks order guarantees that memory() is not nil in the second assertion -+ assert(newByteCount <= spaceSize()); // the new bytes end in our space -+ assert(memory() + contentSize() == newBytes); // the new bytes start in our space -+ // and now we know that newBytes is not nil either -+ -+ if (extraMemory_) -+ extraMemory_->rawAppendFinish(newBytes, newByteCount); -+ else -+ readerSuppliedMemoryContentSize_ = *IncreaseSum(readerSuppliedMemoryContentSize_, newByteCount); -+ -+ assert(contentSize() <= capacity()); // paranoid -+} -+ -+void -+Store::ParsingBuffer::consume(const size_t parsedBytes) -+{ -+ Assure(contentSize() >= parsedBytes); // more conservative than extraMemory_->consume() -+ if (extraMemory_) { -+ extraMemory_->consume(parsedBytes); -+ } else { -+ readerSuppliedMemoryContentSize_ -= parsedBytes; -+ if (parsedBytes && readerSuppliedMemoryContentSize_) -+ memmove(readerSuppliedMemory_.data, memory() + parsedBytes, readerSuppliedMemoryContentSize_); -+ } -+} -+ -+StoreIOBuffer -+Store::ParsingBuffer::space() -+{ -+ const auto size = spaceSize(); -+ const auto start = extraMemory_ ? -+ extraMemory_->rawAppendStart(size) : -+ (readerSuppliedMemory_.data + readerSuppliedMemoryContentSize_); -+ return StoreIOBuffer(spaceSize(), 0, start); -+} -+ -+StoreIOBuffer -+Store::ParsingBuffer::makeSpace(const size_t pageSize) -+{ -+ growSpace(pageSize); -+ auto result = space(); -+ Assure(result.length >= pageSize); -+ result.length = pageSize; -+ return result; -+} -+ -+StoreIOBuffer -+Store::ParsingBuffer::content() const -+{ -+ // This const_cast is a StoreIOBuffer API limitation: That class does not -+ // support a "constant content view", even though it is used as such a view. -+ return StoreIOBuffer(contentSize(), 0, const_cast(memory())); -+} -+ -+/// makes sure we have the requested number of bytes, allocates enough memory if needed -+void -+Store::ParsingBuffer::growSpace(const size_t minimumSpaceSize) -+{ -+ const auto capacityIncreaseAttempt = IncreaseSum(contentSize(), minimumSpaceSize); -+ if (!capacityIncreaseAttempt) -+ throw TextException(ToSBuf("no support for a single memory block of ", contentSize(), '+', minimumSpaceSize, " bytes"), Here()); -+ const auto newCapacity = *capacityIncreaseAttempt; -+ -+ if (newCapacity <= capacity()) -+ return; // already have enough space; no reallocation is needed -+ -+ debugs(90, 7, "growing to provide " << minimumSpaceSize << " in " << *this); -+ -+ if (extraMemory_) { -+ extraMemory_->reserveCapacity(newCapacity); -+ } else { -+ SBuf newStorage; -+ newStorage.reserveCapacity(newCapacity); -+ newStorage.append(readerSuppliedMemory_.data, readerSuppliedMemoryContentSize_); -+ extraMemory_ = std::move(newStorage); -+ } -+ Assure(spaceSize() >= minimumSpaceSize); -+} -+ -+SBuf -+Store::ParsingBuffer::toSBuf() const -+{ -+ return extraMemory_ ? *extraMemory_ : SBuf(content().data, content().length); -+} -+ -+size_t -+Store::ParsingBuffer::spaceSize() const -+{ -+ if (extraMemory_) -+ return extraMemory_->spaceSize(); -+ -+ assert(readerSuppliedMemoryContentSize_ <= readerSuppliedMemory_.length); -+ return readerSuppliedMemory_.length - readerSuppliedMemoryContentSize_; -+} -+ -+/// 0-terminates stored byte sequence, allocating more memory if needed, but -+/// without increasing the number of stored content bytes -+void -+Store::ParsingBuffer::terminate() -+{ -+ *makeSpace(1).data = 0; -+} -+ -+StoreIOBuffer -+Store::ParsingBuffer::packBack() -+{ -+ const auto bytesToPack = contentSize(); -+ // until our callers do not have to work around legacy code expectations -+ Assure(bytesToPack); -+ -+ // if we accumulated more bytes at some point, any extra metadata should -+ // have been consume()d by now, allowing readerSuppliedMemory_.data reuse -+ Assure(bytesToPack <= readerSuppliedMemory_.length); -+ -+ auto result = readerSuppliedMemory_; -+ result.length = bytesToPack; -+ Assure(result.data); -+ -+ if (!extraMemory_) { -+ // no accumulated bytes copying because they are in readerSuppliedMemory_ -+ debugs(90, 7, "quickly exporting " << result.length << " bytes via " << readerSuppliedMemory_); -+ } else { -+ debugs(90, 7, "slowly exporting " << result.length << " bytes from " << extraMemory_->id << " back into " << readerSuppliedMemory_); -+ memmove(result.data, extraMemory_->rawContent(), result.length); -+ } -+ -+ return result; -+} -+ -+void -+Store::ParsingBuffer::print(std::ostream &os) const -+{ -+ os << "size=" << contentSize(); -+ -+ if (extraMemory_) { -+ os << " capacity=" << capacity(); -+ os << " extra=" << extraMemory_->id; -+ } -+ -+ // report readerSuppliedMemory_ (if any) even if we are no longer using it -+ // for content storage; it affects packBack() and related parsing logic -+ if (readerSuppliedMemory_.length) -+ os << ' ' << readerSuppliedMemory_; -+} -+ -diff --git a/src/store/ParsingBuffer.h b/src/store/ParsingBuffer.h -new file mode 100644 -index 00000000000..b8aa9573144 ---- /dev/null -+++ b/src/store/ParsingBuffer.h -@@ -0,0 +1,128 @@ -+/* -+ * Copyright (C) 1996-2023 The Squid Software Foundation and contributors -+ * -+ * Squid software is distributed under GPLv2+ license and includes -+ * contributions from numerous individuals and organizations. -+ * Please see the COPYING and CONTRIBUTORS files for details. -+ */ -+ -+#ifndef SQUID_SRC_STORE_PARSINGBUFFER_H -+#define SQUID_SRC_STORE_PARSINGBUFFER_H -+ -+#include "sbuf/SBuf.h" -+#include "StoreIOBuffer.h" -+ -+#include -+ -+namespace Store -+{ -+ -+/// A continuous buffer for efficient accumulation and NUL-termination of -+/// Store-read bytes. The buffer accumulates two kinds of Store readers: -+/// -+/// * Readers that do not have any external buffer to worry about but need to -+/// accumulate, terminate, and/or consume buffered content read by Store. -+/// These readers use the default constructor and then allocate the initial -+/// buffer space for their first read (if any). -+/// -+/// * Readers that supply their StoreIOBuffer at construction time. That buffer -+/// is enough to handle the majority of use cases. However, the supplied -+/// StoreIOBuffer capacity may be exceeded when parsing requires accumulating -+/// multiple Store read results and/or NUL-termination of a full buffer. -+/// -+/// This buffer seamlessly grows as needed, reducing memory over-allocation and, -+/// in case of StoreIOBuffer-seeded construction, memory copies. -+class ParsingBuffer -+{ -+public: -+ /// creates buffer without any space or content -+ ParsingBuffer() = default; -+ -+ /// seeds this buffer with the caller-supplied buffer space -+ explicit ParsingBuffer(StoreIOBuffer &); -+ -+ /// a NUL-terminated version of content(); same lifetime as content() -+ const char *c_str() { terminate(); return memory(); } -+ -+ /// export content() into SBuf, avoiding content copying when possible -+ SBuf toSBuf() const; -+ -+ /// the total number of append()ed bytes that were not consume()d -+ size_t contentSize() const; -+ -+ /// the number of bytes in the space() buffer -+ size_t spaceSize() const; -+ -+ /// the maximum number of bytes we can store without allocating more space -+ size_t capacity() const; -+ -+ /// Stored append()ed bytes that have not been consume()d. The returned -+ /// buffer offset is set to zero; the caller is responsible for adjusting -+ /// the offset if needed (TODO: Add/return a no-offset Mem::View instead). -+ /// The returned buffer is invalidated by calling a non-constant method or -+ /// by changing the StoreIOBuffer contents given to our constructor. -+ StoreIOBuffer content() const; -+ -+ /// A (possibly empty) buffer for reading the next byte(s). The returned -+ /// buffer offset is set to zero; the caller is responsible for adjusting -+ /// the offset if needed (TODO: Add/return a no-offset Mem::Area instead). -+ /// The returned buffer is invalidated by calling a non-constant method or -+ /// by changing the StoreIOBuffer contents given to our constructor. -+ StoreIOBuffer space(); -+ -+ /// A buffer for reading the exact number of next byte(s). The method may -+ /// allocate new memory and copy previously appended() bytes as needed. -+ /// \param pageSize the exact number of bytes the caller wants to read -+ /// \returns space() after any necessary allocations -+ StoreIOBuffer makeSpace(size_t pageSize); -+ -+ /// A buffer suitable for the first storeClientCopy() call. The method may -+ /// allocate new memory and copy previously appended() bytes as needed. -+ /// \returns space() after any necessary allocations -+ /// \deprecated New clients should call makeSpace() with client-specific -+ /// pageSize instead of this one-size-fits-all legacy method. -+ StoreIOBuffer makeInitialSpace() { return makeSpace(4096); } -+ -+ /// remember the new bytes received into the previously provided space() -+ void appended(const char *, size_t); -+ -+ /// get rid of previously appended() prefix of a given size -+ void consume(size_t); -+ -+ /// Returns stored content, reusing the StoreIOBuffer given at the -+ /// construction time. Copying is avoided if we did not allocate extra -+ /// memory since construction. Not meant for default-constructed buffers. -+ /// \prec positive contentSize() (\sa store_client::finishCallback()) -+ StoreIOBuffer packBack(); -+ -+ /// summarizes object state (for debugging) -+ void print(std::ostream &) const; -+ -+private: -+ const char *memory() const; -+ void terminate(); -+ void growSpace(size_t); -+ -+private: -+ /// externally allocated buffer we were seeded with (or a zero-size one) -+ StoreIOBuffer readerSuppliedMemory_; -+ -+ /// append()ed to readerSuppliedMemory_ bytes that were not consume()d -+ size_t readerSuppliedMemoryContentSize_ = 0; -+ -+ /// our internal buffer that takes over readerSuppliedMemory_ when the -+ /// latter becomes full and more memory is needed -+ std::optional extraMemory_; -+}; -+ -+inline std::ostream & -+operator <<(std::ostream &os, const ParsingBuffer &b) -+{ -+ b.print(os); -+ return os; -+} -+ -+} // namespace Store -+ -+#endif /* SQUID_SRC_STORE_PARSINGBUFFER_H */ -+ -diff --git a/src/store/forward.h b/src/store/forward.h -index 210388ee865..a89689b66c1 100644 ---- a/src/store/forward.h -+++ b/src/store/forward.h -@@ -46,6 +46,7 @@ class Disks; - class Disk; - class DiskConfig; - class EntryGuard; -+class ParsingBuffer; - - typedef ::StoreEntry Entry; - typedef ::MemStore Memory; -diff --git a/src/store_client.cc b/src/store_client.cc -index 4f59d8c1cf7..26402b93ec7 100644 ---- a/src/store_client.cc -+++ b/src/store_client.cc -@@ -19,7 +19,9 @@ - #include "MemBuf.h" - #include "MemObject.h" - #include "mime_header.h" -+#include "sbuf/Stream.h" - #include "SquidConfig.h" -+#include "SquidMath.h" - #include "StatCounters.h" - #include "Store.h" - #include "store/SwapMetaIn.h" -@@ -139,20 +141,6 @@ storeClientListAdd(StoreEntry * e, void *data) - return sc; - } - --/// schedules asynchronous STCB call to relay disk or memory read results --/// \param outcome an error signal (if negative), an EOF signal (if zero), or the number of bytes read --void --store_client::callback(const ssize_t outcome) --{ -- if (outcome > 0) -- return noteCopiedBytes(outcome); -- -- if (outcome < 0) -- return fail(); -- -- noteEof(); --} -- - /// finishCallback() wrapper; TODO: Add NullaryMemFunT for non-jobs. - void - store_client::FinishCallback(store_client * const sc) -@@ -167,14 +155,20 @@ store_client::finishCallback() - Assure(_callback.callback_handler); - Assure(_callback.notifier); - -- // callers are not ready to handle a content+error combination -- Assure(object_ok || !copiedSize); -- -- StoreIOBuffer result(copiedSize, copyInto.offset, copyInto.data); -+ // XXX: Some legacy code relies on zero-length buffers having nil data -+ // pointers. Some other legacy code expects "correct" result.offset even -+ // when there is no body to return. Accommodate all those expectations. -+ auto result = StoreIOBuffer(0, copyInto.offset, nullptr); -+ if (object_ok && parsingBuffer && parsingBuffer->contentSize()) -+ result = parsingBuffer->packBack(); - result.flags.error = object_ok ? 0 : 1; -- copiedSize = 0; - -- cmp_offset = result.offset + result.length; -+ // no HTTP headers and no body bytes (but not because there was no space) -+ atEof_ = !sendingHttpHeaders() && !result.length && copyInto.length; -+ -+ parsingBuffer.reset(); -+ ++answers; -+ - STCB *temphandler = _callback.callback_handler; - void *cbdata = _callback.callback_data; - _callback = Callback(nullptr, nullptr); -@@ -186,35 +180,15 @@ store_client::finishCallback() - cbdataReferenceDone(cbdata); - } - --/// schedules asynchronous STCB call to relay a successful disk or memory read --/// \param bytesCopied the number of response bytes copied into copyInto --void --store_client::noteCopiedBytes(const size_t bytesCopied) --{ -- debugs(90, 5, bytesCopied); -- Assure(bytesCopied > 0); -- Assure(!copiedSize); -- copiedSize = bytesCopied; -- noteNews(); --} -- --void --store_client::noteEof() --{ -- debugs(90, 5, copiedSize); -- Assure(!copiedSize); -- noteNews(); --} -- - store_client::store_client(StoreEntry *e) : -- cmp_offset(0), - #if STORE_CLIENT_LIST_DEBUG - owner(cbdataReference(data)), - #endif - entry(e), - type(e->storeClientType()), - object_ok(true), -- copiedSize(0) -+ atEof_(false), -+ answers(0) - { - Assure(entry); - entry->lock("store_client"); -@@ -269,16 +243,29 @@ store_client::copy(StoreEntry * anEntry, - #endif - - assert(!_callback.pending()); --#if ONLYCONTIGUOUSREQUESTS -- -- assert(cmp_offset == copyRequest.offset); --#endif -- /* range requests will skip into the body */ -- cmp_offset = copyRequest.offset; - _callback = Callback (callback_fn, cbdataReference(data)); - copyInto.data = copyRequest.data; - copyInto.length = copyRequest.length; - copyInto.offset = copyRequest.offset; -+ Assure(copyInto.offset >= 0); -+ -+ if (!copyInto.length) { -+ // During the first storeClientCopy() call, a zero-size buffer means -+ // that we will have to drop any HTTP response body bytes we read (with -+ // the HTTP headers from disk). After that, it means we cannot return -+ // anything to the caller at all. -+ debugs(90, 2, "WARNING: zero-size storeClientCopy() buffer: " << copyInto); -+ // keep going; moreToRead() should prevent any from-Store reading -+ } -+ -+ // Our nextHttpReadOffset() expects the first copy() call to have zero -+ // offset. More complex code could handle a positive first offset, but it -+ // would only be useful when reading responses from memory: We would not -+ // _delay_ the response (to read the requested HTTP body bytes from disk) -+ // when we already can respond with HTTP headers. -+ Assure(!copyInto.offset || answeredOnce()); -+ -+ parsingBuffer.emplace(copyInto); - - static bool copying (false); - assert (!copying); -@@ -304,33 +291,30 @@ store_client::copy(StoreEntry * anEntry, - // Add no code here. This object may no longer exist. - } - --/// Whether there is (or will be) more entry data for us. -+/// Whether Store has (or possibly will have) more entry data for us. - bool --store_client::moreToSend() const -+store_client::moreToRead() const - { -+ if (!copyInto.length) -+ return false; // the client supplied a zero-size buffer -+ - if (entry->store_status == STORE_PENDING) - return true; // there may be more coming - - /* STORE_OK, including aborted entries: no more data is coming */ - -- const int64_t len = entry->objectLen(); -+ if (canReadFromMemory()) -+ return true; // memory has the first byte wanted by the client - -- // If we do not know the entry length, then we have to open the swap file. -- const bool canSwapIn = entry->hasDisk(); -- if (len < 0) -- return canSwapIn; -- -- if (copyInto.offset >= len) -- return false; // sent everything there is -+ if (!entry->hasDisk()) -+ return false; // cannot read anything from disk either - -- if (canSwapIn) -- return true; // if we lack prefix, we can swap it in -+ if (entry->objectLen() >= 0 && copyInto.offset >= entry->contentLen()) -+ return false; // the disk cannot have byte(s) wanted by the client - -- // If we cannot swap in, make sure we have what we want in RAM. Otherwise, -- // scheduleRead calls scheduleDiskRead which asserts without a swap file. -- const MemObject *mem = entry->mem_obj; -- return mem && -- mem->inmem_lo <= copyInto.offset && copyInto.offset < mem->endOffset(); -+ // we cannot be sure until we swap in metadata and learn contentLen(), -+ // but the disk may have the byte(s) wanted by the client -+ return true; - } - - static void -@@ -357,6 +341,14 @@ storeClientCopy2(StoreEntry * e, store_client * sc) - sc->doCopy(e); - } - -+/// Whether our answer, if sent right now, will announce the availability of -+/// HTTP response headers (to the STCB callback) for the first time. -+bool -+store_client::sendingHttpHeaders() const -+{ -+ return !answeredOnce() && entry->mem().baseReply().hdr_sz > 0; -+} -+ - void - store_client::doCopy(StoreEntry *anEntry) - { -@@ -368,20 +360,22 @@ store_client::doCopy(StoreEntry *anEntry) - flags.store_copying = true; - MemObject *mem = entry->mem_obj; - -- debugs(33, 5, "store_client::doCopy: co: " << -- copyInto.offset << ", hi: " << -- mem->endOffset()); -+ debugs(33, 5, this << " into " << copyInto << -+ " hi: " << mem->endOffset() << -+ " objectLen: " << entry->objectLen() << -+ " past_answers: " << answers); - -- if (!moreToSend()) { -+ const auto sendHttpHeaders = sendingHttpHeaders(); -+ -+ if (!sendHttpHeaders && !moreToRead()) { - /* There is no more to send! */ - debugs(33, 3, "There is no more to send!"); -- noteEof(); -+ noteNews(); - flags.store_copying = false; - return; - } - -- /* Check that we actually have data */ -- if (anEntry->store_status == STORE_PENDING && copyInto.offset >= mem->endOffset()) { -+ if (!sendHttpHeaders && anEntry->store_status == STORE_PENDING && nextHttpReadOffset() >= mem->endOffset()) { - debugs(90, 3, "store_client::doCopy: Waiting for more"); - flags.store_copying = false; - return; -@@ -403,7 +397,24 @@ store_client::doCopy(StoreEntry *anEntry) - if (!startSwapin()) - return; // failure - } -- scheduleRead(); -+ -+ // send any immediately available body bytes even if we also sendHttpHeaders -+ if (canReadFromMemory()) { -+ readFromMemory(); -+ noteNews(); // will sendHttpHeaders (if needed) as well -+ flags.store_copying = false; -+ return; -+ } -+ -+ if (sendHttpHeaders) { -+ debugs(33, 5, "just send HTTP headers: " << mem->baseReply().hdr_sz); -+ noteNews(); -+ flags.store_copying = false; -+ return; -+ } -+ -+ // no information that the client needs is available immediately -+ scheduleDiskRead(); - } - - /// opens the swapin "file" if possible; otherwise, fail()s and returns false -@@ -443,18 +454,7 @@ store_client::noteSwapInDone(const bool error) - if (error) - fail(); - else -- noteEof(); --} -- --void --store_client::scheduleRead() --{ -- MemObject *mem = entry->mem_obj; -- -- if (copyInto.offset >= mem->inmem_lo && copyInto.offset < mem->endOffset()) -- scheduleMemRead(); -- else -- scheduleDiskRead(); -+ noteNews(); - } - - void -@@ -479,15 +479,44 @@ store_client::scheduleDiskRead() - flags.store_copying = false; - } - -+/// whether at least one byte wanted by the client is in memory -+bool -+store_client::canReadFromMemory() const -+{ -+ const auto &mem = entry->mem(); -+ const auto memReadOffset = nextHttpReadOffset(); -+ return mem.inmem_lo <= memReadOffset && memReadOffset < mem.endOffset() && -+ parsingBuffer->spaceSize(); -+} -+ -+/// The offset of the next stored HTTP response byte wanted by the client. -+int64_t -+store_client::nextHttpReadOffset() const -+{ -+ Assure(parsingBuffer); -+ const auto &mem = entry->mem(); -+ const auto hdr_sz = mem.baseReply().hdr_sz; -+ // Certain SMP cache manager transactions do not store HTTP headers in -+ // mem_hdr; they store just a kid-specific piece of the future report body. -+ // In such cases, hdr_sz ought to be zero. In all other (known) cases, -+ // mem_hdr contains HTTP response headers (positive hdr_sz if parsed) -+ // followed by HTTP response body. This code math accommodates all cases. -+ return NaturalSum(hdr_sz, copyInto.offset, parsingBuffer->contentSize()).value(); -+} -+ -+/// Copies at least some of the requested body bytes from MemObject memory, -+/// satisfying the copy() request. -+/// \pre canReadFromMemory() is true - void --store_client::scheduleMemRead() -+store_client::readFromMemory() - { -- /* What the client wants is in memory */ -- /* Old style */ -- debugs(90, 3, "store_client::doCopy: Copying normal from memory"); -- const auto sz = entry->mem_obj->data_hdr.copy(copyInto); // may be <= 0 per copy() API -- callback(sz); -- flags.store_copying = false; -+ Assure(parsingBuffer); -+ const auto readInto = parsingBuffer->space().positionAt(nextHttpReadOffset()); -+ -+ debugs(90, 3, "copying HTTP body bytes from memory into " << readInto); -+ const auto sz = entry->mem_obj->data_hdr.copy(readInto); -+ Assure(sz > 0); // our canReadFromMemory() precondition guarantees that -+ parsingBuffer->appended(readInto.data, sz); - } - - void -@@ -499,52 +528,132 @@ store_client::fileRead() - assert(!flags.disk_io_pending); - flags.disk_io_pending = true; - -+ // mem->swap_hdr_sz is zero here during initial read(s) -+ const auto nextStoreReadOffset = NaturalSum(mem->swap_hdr_sz, nextHttpReadOffset()).value(); -+ -+ // XXX: If fileRead() is called when we do not yet know mem->swap_hdr_sz, -+ // then we must start reading from disk offset zero to learn it: we cannot -+ // compute correct HTTP response start offset on disk without it. However, -+ // late startSwapin() calls imply that the assertion below might fail. -+ Assure(mem->swap_hdr_sz > 0 || !nextStoreReadOffset); -+ -+ // TODO: Remove this assertion. Introduced in 1998 commit 3157c72, it -+ // assumes that swapped out memory is freed unconditionally, but we no -+ // longer do that because trimMemory() path checks lowestMemReaderOffset(). -+ // It is also misplaced: We are not swapping out anything here and should -+ // not care about any swapout invariants. - if (mem->swap_hdr_sz != 0) - if (entry->swappingOut()) -- assert(mem->swapout.sio->offset() > copyInto.offset + (int64_t)mem->swap_hdr_sz); -+ assert(mem->swapout.sio->offset() > nextStoreReadOffset); -+ -+ // XXX: We should let individual cache_dirs limit the read size instead, but -+ // we cannot do that without more fixes and research because: -+ // * larger reads corrupt responses when cache_dir uses SharedMemory::get(); -+ // * we do not know how to find all I/O code that assumes this limit; -+ // * performance effects of larger disk reads may be negative somewhere. -+ const decltype(StoreIOBuffer::length) maxReadSize = SM_PAGE_SIZE; -+ -+ Assure(parsingBuffer); -+ // also, do not read more than we can return (via a copyInto.length buffer) -+ const auto readSize = std::min(copyInto.length, maxReadSize); -+ lastDiskRead = parsingBuffer->makeSpace(readSize).positionAt(nextStoreReadOffset); -+ debugs(90, 5, "into " << lastDiskRead); - - storeRead(swapin_sio, -- copyInto.data, -- copyInto.length, -- copyInto.offset + mem->swap_hdr_sz, -+ lastDiskRead.data, -+ lastDiskRead.length, -+ lastDiskRead.offset, - mem->swap_hdr_sz == 0 ? storeClientReadHeader - : storeClientReadBody, - this); - } - - void --store_client::readBody(const char *, ssize_t len) -+store_client::readBody(const char * const buf, const ssize_t lastIoResult) - { -- // Don't assert disk_io_pending here.. may be called by read_header -+ Assure(flags.disk_io_pending); - flags.disk_io_pending = false; - assert(_callback.pending()); -- debugs(90, 3, "storeClientReadBody: len " << len << ""); -+ Assure(parsingBuffer); -+ debugs(90, 3, "got " << lastIoResult << " using " << *parsingBuffer); - -- if (len < 0) -+ if (lastIoResult < 0) - return fail(); - -- const auto rep = entry->mem_obj ? &entry->mem().baseReply() : nullptr; -- if (copyInto.offset == 0 && len > 0 && rep && rep->sline.status() == Http::scNone) { -- /* Our structure ! */ -- if (!entry->mem_obj->adjustableBaseReply().parseCharBuf(copyInto.data, headersEnd(copyInto.data, len))) { -- debugs(90, DBG_CRITICAL, "ERROR: Could not parse headers from on disk object"); -- } -+ if (!lastIoResult) { -+ if (answeredOnce()) -+ return noteNews(); -+ -+ debugs(90, DBG_CRITICAL, "ERROR: Truncated HTTP headers in on-disk object"); -+ return fail(); - } - -- if (len > 0 && rep && entry->mem_obj->inmem_lo == 0 && entry->objectLen() <= (int64_t)Config.Store.maxInMemObjSize && Config.onoff.memory_cache_disk) { -- storeGetMemSpace(len); -+ assert(lastDiskRead.data == buf); -+ lastDiskRead.length = lastIoResult; -+ -+ parsingBuffer->appended(buf, lastIoResult); -+ -+ // we know swap_hdr_sz by now and were reading beyond swap metadata because -+ // readHead() would have been called otherwise (to read swap metadata) -+ const auto swap_hdr_sz = entry->mem().swap_hdr_sz; -+ Assure(swap_hdr_sz > 0); -+ Assure(!Less(lastDiskRead.offset, swap_hdr_sz)); -+ -+ // Map lastDiskRead (i.e. the disk area we just read) to an HTTP reply part. -+ // The bytes are the same, but disk and HTTP offsets differ by swap_hdr_sz. -+ const auto httpOffset = lastDiskRead.offset - swap_hdr_sz; -+ const auto httpPart = StoreIOBuffer(lastDiskRead).positionAt(httpOffset); -+ -+ maybeWriteFromDiskToMemory(httpPart); -+ handleBodyFromDisk(); -+} -+ -+/// de-serializes HTTP response (partially) read from disk storage -+void -+store_client::handleBodyFromDisk() -+{ -+ // We cannot de-serialize on-disk HTTP response without MemObject because -+ // without MemObject::swap_hdr_sz we cannot know where that response starts. -+ Assure(entry->mem_obj); -+ Assure(entry->mem_obj->swap_hdr_sz > 0); -+ -+ if (!answeredOnce()) { -+ // All on-disk responses have HTTP headers. First disk body read(s) -+ // include HTTP headers that we must parse (if needed) and skip. -+ const auto haveHttpHeaders = entry->mem_obj->baseReply().pstate == Http::Message::psParsed; -+ if (!haveHttpHeaders && !parseHttpHeadersFromDisk()) -+ return; -+ skipHttpHeadersFromDisk(); -+ } -+ -+ noteNews(); -+} -+ -+/// Adds HTTP response data loaded from disk to the memory cache (if -+/// needed/possible). The given part may contain portions of HTTP response -+/// headers and/or HTTP response body. -+void -+store_client::maybeWriteFromDiskToMemory(const StoreIOBuffer &httpResponsePart) -+{ -+ // XXX: Reject [memory-]uncachable/unshareable responses instead of assuming -+ // that an HTTP response should be written to MemObject's data_hdr (and that -+ // it may purge already cached entries) just because it "fits" and was -+ // loaded from disk. For example, this response may already be marked for -+ // release. The (complex) cachability decision(s) should be made outside -+ // (and obeyed by) this low-level code. -+ if (httpResponsePart.length && entry->mem_obj->inmem_lo == 0 && entry->objectLen() <= (int64_t)Config.Store.maxInMemObjSize && Config.onoff.memory_cache_disk) { -+ storeGetMemSpace(httpResponsePart.length); -+ // XXX: This "recheck" is not needed because storeGetMemSpace() cannot -+ // purge mem_hdr bytes of a locked entry, and we do lock ours. And -+ // inmem_lo offset itself should not be relevant to appending new bytes. -+ // - // recheck for the above call may purge entry's data from the memory cache - if (entry->mem_obj->inmem_lo == 0) { -- /* Copy read data back into memory. -- * copyInto.offset includes headers, which is what mem cache needs -- */ -- if (copyInto.offset == entry->mem_obj->endOffset()) { -- entry->mem_obj->write(StoreIOBuffer(len, copyInto.offset, copyInto.data)); -- } -+ // XXX: This code assumes a non-shared memory cache. -+ if (httpResponsePart.offset == entry->mem_obj->endOffset()) -+ entry->mem_obj->write(httpResponsePart); - } - } -- -- callback(len); - } - - void -@@ -613,41 +722,25 @@ store_client::readHeader(char const *buf, ssize_t len) - if (!object_ok) - return; - -+ Assure(parsingBuffer); -+ debugs(90, 3, "got " << len << " using " << *parsingBuffer); -+ - if (len < 0) - return fail(); - - try { -+ Assure(!parsingBuffer->contentSize()); -+ parsingBuffer->appended(buf, len); - Store::UnpackHitSwapMeta(buf, len, *entry); -+ parsingBuffer->consume(mem->swap_hdr_sz); - } catch (...) { - debugs(90, DBG_IMPORTANT, "ERROR: Failed to unpack Store entry metadata: " << CurrentException); - fail(); - return; - } - -- /* -- * If our last read got some data the client wants, then give -- * it to them, otherwise schedule another read. -- */ -- size_t body_sz = len - mem->swap_hdr_sz; -- -- if (copyInto.offset < static_cast(body_sz)) { -- /* -- * we have (part of) what they want -- */ -- size_t copy_sz = min(copyInto.length, body_sz); -- debugs(90, 3, "storeClientReadHeader: copying " << copy_sz << " bytes of body"); -- memmove(copyInto.data, copyInto.data + mem->swap_hdr_sz, copy_sz); -- -- readBody(copyInto.data, copy_sz); -- -- return; -- } -- -- /* -- * we don't have what the client wants, but at least we now -- * know the swap header size. -- */ -- fileRead(); -+ maybeWriteFromDiskToMemory(parsingBuffer->content()); -+ handleBodyFromDisk(); - } - - /* -@@ -893,6 +986,63 @@ CheckQuickAbortIsReasonable(StoreEntry * entry) - return true; - } - -+/// parses HTTP header bytes loaded from disk -+/// \returns false if fail() or scheduleDiskRead() has been called and, hence, -+/// the caller should just quit without any further action -+bool -+store_client::parseHttpHeadersFromDisk() -+{ -+ try { -+ return tryParsingHttpHeaders(); -+ } catch (...) { -+ // XXX: Our parser enforces Config.maxReplyHeaderSize limit, but our -+ // packer does not. Since packing might increase header size, we may -+ // cache a header that we cannot parse and get here. Same for MemStore. -+ debugs(90, DBG_CRITICAL, "ERROR: Cannot parse on-disk HTTP headers" << -+ Debug::Extra << "exception: " << CurrentException << -+ Debug::Extra << "raw input size: " << parsingBuffer->contentSize() << " bytes" << -+ Debug::Extra << "current buffer capacity: " << parsingBuffer->capacity() << " bytes"); -+ fail(); -+ return false; -+ } -+} -+ -+/// parseHttpHeadersFromDisk() helper -+/// \copydoc parseHttpHeaders() -+bool -+store_client::tryParsingHttpHeaders() -+{ -+ Assure(parsingBuffer); -+ Assure(!copyInto.offset); // otherwise, parsingBuffer cannot have HTTP response headers -+ auto &adjustableReply = entry->mem().adjustableBaseReply(); -+ if (adjustableReply.parseTerminatedPrefix(parsingBuffer->c_str(), parsingBuffer->contentSize())) -+ return true; -+ -+ // TODO: Optimize by checking memory as well. For simplicity sake, we -+ // continue on the disk-reading path, but readFromMemory() can give us the -+ // missing header bytes immediately if a concurrent request put those bytes -+ // into memory while we were waiting for our disk response. -+ scheduleDiskRead(); -+ return false; -+} -+ -+/// skips HTTP header bytes previously loaded from disk -+void -+store_client::skipHttpHeadersFromDisk() -+{ -+ const auto hdr_sz = entry->mem_obj->baseReply().hdr_sz; -+ Assure(hdr_sz > 0); // all on-disk responses have HTTP headers -+ if (Less(parsingBuffer->contentSize(), hdr_sz)) { -+ debugs(90, 5, "discovered " << hdr_sz << "-byte HTTP headers in memory after reading some of them from disk: " << *parsingBuffer); -+ parsingBuffer->consume(parsingBuffer->contentSize()); // skip loaded HTTP header prefix -+ } else { -+ parsingBuffer->consume(hdr_sz); // skip loaded HTTP headers -+ const auto httpBodyBytesAfterHeader = parsingBuffer->contentSize(); // may be zero -+ Assure(httpBodyBytesAfterHeader <= copyInto.length); -+ debugs(90, 5, "read HTTP body prefix: " << httpBodyBytesAfterHeader); -+ } -+} -+ - void - store_client::dumpStats(MemBuf * output, int clientNumber) const - { -diff --git a/src/tests/stub_HttpReply.cc b/src/tests/stub_HttpReply.cc -index 176462fcbfe..e34ca4778e4 100644 ---- a/src/tests/stub_HttpReply.cc -+++ b/src/tests/stub_HttpReply.cc -@@ -24,6 +24,7 @@ void HttpReply::reset() STUB - bool HttpReply::sanityCheckStartLine(const char *, const size_t, Http::StatusCode *) STUB_RETVAL(false) - int HttpReply::httpMsgParseError() STUB_RETVAL(0) - bool HttpReply::expectingBody(const HttpRequestMethod&, int64_t&) const STUB_RETVAL(false) -+size_t HttpReply::parseTerminatedPrefix(const char *, size_t) STUB_RETVAL(0) - bool HttpReply::parseFirstLine(const char *, const char *) STUB_RETVAL(false) - void HttpReply::hdrCacheInit() STUB - HttpReply * HttpReply::clone() const STUB_RETVAL(nullptr) -diff --git a/src/urn.cc b/src/urn.cc -index be474cd00eb..7365c6ec802 100644 ---- a/src/urn.cc -+++ b/src/urn.cc -@@ -27,8 +27,6 @@ - #include "tools.h" - #include "urn.h" - --#define URN_REQBUF_SZ 4096 -- - class UrnState : public StoreClient - { - CBDATA_CLASS(UrnState); -@@ -48,8 +46,8 @@ class UrnState : public StoreClient - HttpRequest::Pointer urlres_r; - AccessLogEntry::Pointer ale; ///< details of the requesting transaction - -- char reqbuf[URN_REQBUF_SZ] = { '\0' }; -- int reqofs = 0; -+ /// for receiving a URN resolver reply body from Store and interpreting it -+ Store::ParsingBuffer parsingBuffer; - - private: - /* StoreClient API */ -@@ -70,7 +68,7 @@ typedef struct { - } url_entry; - - static STCB urnHandleReply; --static url_entry *urnParseReply(const char *inbuf, const HttpRequestMethod&); -+static url_entry *urnParseReply(const SBuf &, const HttpRequestMethod &); - static const char *const crlf = "\r\n"; - - CBDATA_CLASS_INIT(UrnState); -@@ -189,13 +187,8 @@ UrnState::start(HttpRequest * r, StoreEntry * e) - sc = storeClientListAdd(urlres_e, this); - } - -- reqofs = 0; -- StoreIOBuffer tempBuffer; -- tempBuffer.offset = reqofs; -- tempBuffer.length = URN_REQBUF_SZ; -- tempBuffer.data = reqbuf; - storeClientCopy(sc, urlres_e, -- tempBuffer, -+ parsingBuffer.makeInitialSpace(), - urnHandleReply, - this); - } -@@ -237,19 +230,14 @@ urnHandleReply(void *data, StoreIOBuffer result) - UrnState *urnState = static_cast(data); - StoreEntry *e = urnState->entry; - StoreEntry *urlres_e = urnState->urlres_e; -- char *s = nullptr; -- size_t k; -- HttpReply *rep; - url_entry *urls; - url_entry *u; - url_entry *min_u; - ErrorState *err; - int i; - int urlcnt = 0; -- char *buf = urnState->reqbuf; -- StoreIOBuffer tempBuffer; - -- debugs(52, 3, "urnHandleReply: Called with size=" << result.length << "."); -+ debugs(52, 3, result << " with " << *e); - - if (EBIT_TEST(urlres_e->flags, ENTRY_ABORTED) || result.flags.error) { - delete urnState; -@@ -262,59 +250,39 @@ urnHandleReply(void *data, StoreIOBuffer result) - return; - } - -- /* Update reqofs to point to where in the buffer we'd be */ -- urnState->reqofs += result.length; -- -- /* Handle reqofs being bigger than normal */ -- if (urnState->reqofs >= URN_REQBUF_SZ) { -- delete urnState; -- return; -- } -+ urnState->parsingBuffer.appended(result.data, result.length); - - /* If we haven't received the entire object (urn), copy more */ -- if (urlres_e->store_status == STORE_PENDING) { -- Must(result.length > 0); // zero length ought to imply STORE_OK -- tempBuffer.offset = urnState->reqofs; -- tempBuffer.length = URN_REQBUF_SZ - urnState->reqofs; -- tempBuffer.data = urnState->reqbuf + urnState->reqofs; -+ if (!urnState->sc->atEof()) { -+ const auto bufferedBytes = urnState->parsingBuffer.contentSize(); -+ const auto remainingSpace = urnState->parsingBuffer.space().positionAt(bufferedBytes); -+ -+ if (!remainingSpace.length) { -+ debugs(52, 3, "ran out of buffer space after " << bufferedBytes << " bytes"); -+ // TODO: Here and in other error cases, send ERR_URN_RESOLVE to client. -+ delete urnState; -+ return; -+ } -+ - storeClientCopy(urnState->sc, urlres_e, -- tempBuffer, -+ remainingSpace, - urnHandleReply, - urnState); - return; - } - -- /* we know its STORE_OK */ -- k = headersEnd(buf, urnState->reqofs); -- -- if (0 == k) { -- debugs(52, DBG_IMPORTANT, "urnHandleReply: didn't find end-of-headers for " << e->url() ); -- delete urnState; -- return; -- } -- -- s = buf + k; -- // TODO: Check whether we should parse urlres_e reply, as before 528b2c61. -- rep = new HttpReply; -- rep->parseCharBuf(buf, k); -- debugs(52, 3, "reply exists, code=" << rep->sline.status() << "."); -- -- if (rep->sline.status() != Http::scOkay) { -+ const auto &peerReply = urlres_e->mem().baseReply(); -+ debugs(52, 3, "got reply, code=" << peerReply.sline.status()); -+ if (peerReply.sline.status() != Http::scOkay) { - debugs(52, 3, "urnHandleReply: failed."); - err = new ErrorState(ERR_URN_RESOLVE, Http::scNotFound, urnState->request.getRaw(), urnState->ale); - err->url = xstrdup(e->url()); - errorAppendEntry(e, err); -- delete rep; - delete urnState; - return; - } - -- delete rep; -- -- while (xisspace(*s)) -- ++s; -- -- urls = urnParseReply(s, urnState->request->method); -+ urls = urnParseReply(urnState->parsingBuffer.toSBuf(), urnState->request->method); - - if (!urls) { /* unknown URN error */ - debugs(52, 3, "urnTranslateDone: unknown URN " << e->url()); -@@ -362,7 +330,7 @@ urnHandleReply(void *data, StoreIOBuffer result) - "Generated by %s@%s\n" - "\n", - visible_appname_string, getMyHostname()); -- rep = new HttpReply; -+ const auto rep = new HttpReply; - rep->setHeaders(Http::scFound, nullptr, "text/html", mb->length(), 0, squid_curtime); - - if (min_u) { -@@ -384,9 +352,8 @@ urnHandleReply(void *data, StoreIOBuffer result) - } - - static url_entry * --urnParseReply(const char *inbuf, const HttpRequestMethod& m) -+urnParseReply(const SBuf &inBuf, const HttpRequestMethod &m) - { -- char *buf = xstrdup(inbuf); - char *token; - url_entry *list; - url_entry *old; -@@ -395,6 +362,13 @@ urnParseReply(const char *inbuf, const HttpRequestMethod& m) - debugs(52, 3, "urnParseReply"); - list = (url_entry *)xcalloc(n + 1, sizeof(*list)); - -+ // XXX: Switch to tokenizer-based parsing. -+ const auto allocated = SBufToCstring(inBuf); -+ -+ auto buf = allocated; -+ while (xisspace(*buf)) -+ ++buf; -+ - for (token = strtok(buf, crlf); token; token = strtok(nullptr, crlf)) { - debugs(52, 3, "urnParseReply: got '" << token << "'"); - -@@ -430,7 +404,7 @@ urnParseReply(const char *inbuf, const HttpRequestMethod& m) - } - - debugs(52, 3, "urnParseReply: Found " << i << " URLs"); -- xfree(buf); -+ xfree(allocated); - return list; - } - diff --git a/backport-CVE-2023-46724.patch b/backport-CVE-2023-46724.patch deleted file mode 100644 index 7809dd6..0000000 --- a/backport-CVE-2023-46724.patch +++ /dev/null @@ -1,39 +0,0 @@ -From b70f864940225dfe69f9f653f948e787f99c3810 Mon Sep 17 00:00:00 2001 -From: Andreas Weigel -Date: Wed, 18 Oct 2023 04:14:31 +0000 -Subject: [PATCH] Fix validation of certificates with CN=* (#1523) - -The bug was discovered and detailed by Joshua Rogers at -https://megamansec.github.io/Squid-Security-Audit/ -where it was filed as "Buffer UnderRead in SSL CN Parsing". - -Conflict:NA -Reference:https://github.com/squid-cache/squid/commit/b70f864940225dfe69f9f653f948e787f99c3810 ---- - src/anyp/Uri.cc | 6 ++++++ - 1 file changed, 6 insertions(+) - -diff --git a/src/anyp/Uri.cc b/src/anyp/Uri.cc -index 3eed2366abd..ef77d4f766b 100644 ---- a/src/anyp/Uri.cc -+++ b/src/anyp/Uri.cc -@@ -175,6 +175,10 @@ urlInitialize(void) - assert(0 == matchDomainName("*.foo.com", ".foo.com", mdnHonorWildcards)); - assert(0 != matchDomainName("*.foo.com", "foo.com", mdnHonorWildcards)); - -+ assert(0 != matchDomainName("foo.com", "")); -+ assert(0 != matchDomainName("foo.com", "", mdnHonorWildcards)); -+ assert(0 != matchDomainName("foo.com", "", mdnRejectSubsubDomains)); -+ - /* more cases? */ - } - -@@ -828,6 +832,8 @@ matchDomainName(const char *h, const char *d, MatchDomainNameFlags flags) - return -1; - - dl = strlen(d); -+ if (dl == 0) -+ return 1; - - /* - * Start at the ends of the two strings and work towards the diff --git a/backport-CVE-2023-46846.patch b/backport-CVE-2023-46846.patch deleted file mode 100644 index 088ecc1..0000000 --- a/backport-CVE-2023-46846.patch +++ /dev/null @@ -1,180 +0,0 @@ -From 6cfa10d94ca15a764a1d975597d8024582ef19be Mon Sep 17 00:00:00 2001 -From: Amos Jeffries -Date: Fri, 13 Oct 2023 08:44:16 +0000 -Subject: [PATCH] RFC 9112: Improve HTTP chunked encoding compliance (#1498) - -Reference:http://www.squid-cache.org/Versions/v6/SQUID-2023_1.patch -Conflict:NA ---- - src/http/one/Parser.cc | 8 +------- - src/http/one/Parser.h | 4 +--- - src/http/one/TeChunkedParser.cc | 23 ++++++++++++++++++----- - src/parser/Tokenizer.cc | 12 ++++++++++++ - src/parser/Tokenizer.h | 7 +++++++ - 5 files changed, 39 insertions(+), 15 deletions(-) - -diff --git a/src/http/one/Parser.cc b/src/http/one/Parser.cc -index 964371b4e..b1908316a 100644 ---- a/src/http/one/Parser.cc -+++ b/src/http/one/Parser.cc -@@ -65,16 +65,10 @@ Http::One::Parser::DelimiterCharacters() - void - Http::One::Parser::skipLineTerminator(Tokenizer &tok) const - { -- if (tok.skip(Http1::CrLf())) -- return; -- - if (Config.onoff.relaxed_header_parser && tok.skipOne(CharacterSet::LF)) - return; - -- if (tok.atEnd() || (tok.remaining().length() == 1 && tok.remaining().at(0) == '\r')) -- throw InsufficientInput(); -- -- throw TexcHere("garbage instead of CRLF line terminator"); -+ tok.skipRequired("line-terminating CRLF", Http1::CrLf()); - } - - /// all characters except the LF line terminator -diff --git a/src/http/one/Parser.h b/src/http/one/Parser.h -index 5892a7a59..503c61d3f 100644 ---- a/src/http/one/Parser.h -+++ b/src/http/one/Parser.h -@@ -124,9 +124,7 @@ protected: - * detect and skip the CRLF or (if tolerant) LF line terminator - * consume from the tokenizer. - * -- * \throws exception on bad or InsuffientInput. -- * \retval true only if line terminator found. -- * \retval false incomplete or missing line terminator, need more data. -+ * \throws exception on bad or InsufficientInput - */ - void skipLineTerminator(Tokenizer &) const; - -diff --git a/src/http/one/TeChunkedParser.cc b/src/http/one/TeChunkedParser.cc -index d9138fe9a..9cce10fdc 100644 ---- a/src/http/one/TeChunkedParser.cc -+++ b/src/http/one/TeChunkedParser.cc -@@ -91,6 +91,11 @@ Http::One::TeChunkedParser::parseChunkSize(Tokenizer &tok) - { - Must(theChunkSize <= 0); // Should(), really - -+ static const SBuf bannedHexPrefixLower("0x"); -+ static const SBuf bannedHexPrefixUpper("0X"); -+ if (tok.skip(bannedHexPrefixLower) || tok.skip(bannedHexPrefixUpper)) -+ throw TextException("chunk starts with 0x", Here()); -+ - int64_t size = -1; - if (tok.int64(size, 16, false) && !tok.atEnd()) { - if (size < 0) -@@ -121,7 +126,7 @@ Http::One::TeChunkedParser::parseChunkMetadataSuffix(Tokenizer &tok) - // bad or insufficient input, like in the code below. TODO: Expand up. - try { - parseChunkExtensions(tok); // a possibly empty chunk-ext list -- skipLineTerminator(tok); -+ tok.skipRequired("CRLF after [chunk-ext]", Http1::CrLf()); - buf_ = tok.remaining(); - parsingStage_ = theChunkSize ? Http1::HTTP_PARSE_CHUNK : Http1::HTTP_PARSE_MIME; - return true; -@@ -132,12 +137,14 @@ Http::One::TeChunkedParser::parseChunkMetadataSuffix(Tokenizer &tok) - // other exceptions bubble up to kill message parsing - } - --/// Parses the chunk-ext list (RFC 7230 section 4.1.1 and its Errata #4667): -+/// Parses the chunk-ext list (RFC 9112 section 7.1.1: - /// chunk-ext = *( BWS ";" BWS chunk-ext-name [ BWS "=" BWS chunk-ext-val ] ) - void --Http::One::TeChunkedParser::parseChunkExtensions(Tokenizer &tok) -+Http::One::TeChunkedParser::parseChunkExtensions(Tokenizer &callerTok) - { - do { -+ auto tok = callerTok; -+ - ParseBws(tok); // Bug 4492: IBM_HTTP_Server sends SP after chunk-size - - if (!tok.skip(';')) -@@ -145,6 +152,7 @@ Http::One::TeChunkedParser::parseChunkExtensions(Tokenizer &tok) - - parseOneChunkExtension(tok); - buf_ = tok.remaining(); // got one extension -+ callerTok = tok; - } while (true); - } - -@@ -158,11 +166,14 @@ Http::One::ChunkExtensionValueParser::Ignore(Tokenizer &tok, const SBuf &extName - /// Parses a single chunk-ext list element: - /// chunk-ext = *( BWS ";" BWS chunk-ext-name [ BWS "=" BWS chunk-ext-val ] ) - void --Http::One::TeChunkedParser::parseOneChunkExtension(Tokenizer &tok) -+Http::One::TeChunkedParser::parseOneChunkExtension(Tokenizer &callerTok) - { -+ auto tok = callerTok; -+ - ParseBws(tok); // Bug 4492: ICAP servers send SP before chunk-ext-name - - const auto extName = tok.prefix("chunk-ext-name", CharacterSet::TCHAR); -+ callerTok = tok; // in case we determine that this is a valueless chunk-ext - - ParseBws(tok); - -@@ -176,6 +187,8 @@ Http::One::TeChunkedParser::parseOneChunkExtension(Tokenizer &tok) - customExtensionValueParser->parse(tok, extName); - else - ChunkExtensionValueParser::Ignore(tok, extName); -+ -+ callerTok = tok; - } - - bool -@@ -209,7 +222,7 @@ Http::One::TeChunkedParser::parseChunkEnd(Tokenizer &tok) - Must(theLeftBodySize == 0); // Should(), really - - try { -- skipLineTerminator(tok); -+ tok.skipRequired("chunk CRLF", Http1::CrLf()); - buf_ = tok.remaining(); // parse checkpoint - theChunkSize = 0; // done with the current chunk - parsingStage_ = Http1::HTTP_PARSE_CHUNK_SZ; -diff --git a/src/parser/Tokenizer.cc b/src/parser/Tokenizer.cc -index 2bac7fb1f..2843a44af 100644 ---- a/src/parser/Tokenizer.cc -+++ b/src/parser/Tokenizer.cc -@@ -147,6 +147,18 @@ Parser::Tokenizer::skipAll(const CharacterSet &tokenChars) - return success(prefixLen); - } - -+void -+Parser::Tokenizer::skipRequired(const char *description, const SBuf &tokenToSkip) -+{ -+ if (skip(tokenToSkip) || tokenToSkip.isEmpty()) -+ return; -+ -+ if (tokenToSkip.startsWith(buf_)) -+ throw InsufficientInput(); -+ -+ throw TextException(ToSBuf("cannot skip ", description), Here()); -+} -+ - bool - Parser::Tokenizer::skipOne(const CharacterSet &chars) - { -diff --git a/src/parser/Tokenizer.h b/src/parser/Tokenizer.h -index 7bae1ccbb..3cfa7dd6c 100644 ---- a/src/parser/Tokenizer.h -+++ b/src/parser/Tokenizer.h -@@ -115,6 +115,13 @@ public: - */ - SBuf::size_type skipAll(const CharacterSet &discardables); - -+ /** skips a given character sequence (string); -+ * does nothing if the sequence is empty -+ * -+ * \throws exception on mismatching prefix or InsufficientInput -+ */ -+ void skipRequired(const char *description, const SBuf &tokenToSkip); -+ - /** Removes a single trailing character from the set. - * - * \return whether a character was removed --- -2.25.1 - diff --git a/backport-CVE-2023-46847.patch b/backport-CVE-2023-46847.patch deleted file mode 100644 index 6344008..0000000 --- a/backport-CVE-2023-46847.patch +++ /dev/null @@ -1,42 +0,0 @@ -From dc0e10bec3334053c1a5297e50dd7052ea18aef0 Mon Sep 17 00:00:00 2001 -From: Alex Bason -Date: Sun, 15 Oct 2023 13:04:47 +0000 -Subject: [PATCH] Fix stack buffer overflow when parsing Digest Authorization - (#1517) - -The bug was discovered and detailed by Joshua Rogers at -https://megamansec.github.io/Squid-Security-Audit/digest-overflow.html -where it was filed as "Stack Buffer Overflow in Digest Authentication". - -Reference:http://www.squid-cache.org/Versions/v6/SQUID-2023_3.patch -Conflict:NA ---- - src/auth/digest/Config.cc | 10 +++++++--- - 1 file changed, 7 insertions(+), 3 deletions(-) - -diff --git a/src/auth/digest/Config.cc b/src/auth/digest/Config.cc -index f00e2ba68..3c070d242 100644 ---- a/src/auth/digest/Config.cc -+++ b/src/auth/digest/Config.cc -@@ -827,11 +827,15 @@ Auth::Digest::Config::decode(char const *proxy_auth, const HttpRequest *request, - break; - - case DIGEST_NC: -- if (value.size() != 8) { -+ if (value.size() == 8) { -+ // for historical reasons, the nc value MUST be exactly 8 bytes -+ static_assert(sizeof(digest_request->nc) == 8 + 1); -+ xstrncpy(digest_request->nc, value.rawBuf(), value.size() + 1); -+ debugs(29, 9, "Found noncecount '" << digest_request->nc << "'"); -+ } else { - debugs(29, 9, "Invalid nc '" << value << "' in '" << temp << "'"); -+ digest_request->nc[0] = 0; - } -- xstrncpy(digest_request->nc, value.rawBuf(), value.size() + 1); -- debugs(29, 9, "Found noncecount '" << digest_request->nc << "'"); - break; - - case DIGEST_CNONCE: --- -2.25.1 - diff --git a/backport-CVE-2023-46848.patch b/backport-CVE-2023-46848.patch deleted file mode 100644 index 8a8a0c8..0000000 --- a/backport-CVE-2023-46848.patch +++ /dev/null @@ -1,49 +0,0 @@ -From 7de01969a793b2fdb476e354a9fcda272d400d27 Mon Sep 17 00:00:00 2001 -From: Alex Rousskov -Date: Thu, 25 May 2023 02:10:28 +0000 -Subject: [PATCH] Fix userinfo percent-encoding (#1367) - -%X expects an unsigned int, and that is what we were giving it. However, -to get to the correct unsigned int value from a (signed) char, one has -to cast to an unsigned char (or equivalent) first. - -Broken since inception in commit 7b75100. - -Also adjusted similar (commented out) ext_edirectory_userip_acl code. - -Reference:http://www.squid-cache.org/Versions/v6/SQUID-2023_5.patch -Conflict:NA ---- - src/acl/external/eDirectory_userip/ext_edirectory_userip_acl.cc | 2 +- - src/anyp/Uri.cc | 2 +- - 2 files changed, 2 insertions(+), 2 deletions(-) - -diff --git a/src/acl/external/eDirectory_userip/ext_edirectory_userip_acl.cc b/src/acl/external/eDirectory_userip/ext_edirectory_userip_acl.cc -index bf124d24f..e3f33e209 100644 ---- a/src/acl/external/eDirectory_userip/ext_edirectory_userip_acl.cc -+++ b/src/acl/external/eDirectory_userip/ext_edirectory_userip_acl.cc -@@ -1555,7 +1555,7 @@ MainSafe(int argc, char **argv) - /* BINARY DEBUGGING * - local_printfx("while() -> bufa[%" PRIuSIZE "]: %s", k, bufa); - for (i = 0; i < k; ++i) -- local_printfx("%02X", bufa[i]); -+ local_printfx("%02X", static_cast(static_cast(bufa[i]))); - local_printfx("\n"); - * BINARY DEBUGGING */ - /* Check for CRLF */ -diff --git a/src/anyp/Uri.cc b/src/anyp/Uri.cc -index e37293996..eca2c2357 100644 ---- a/src/anyp/Uri.cc -+++ b/src/anyp/Uri.cc -@@ -71,7 +71,7 @@ AnyP::Uri::Encode(const SBuf &buf, const CharacterSet &ignore) - while (!tk.atEnd()) { - // TODO: Add Tokenizer::parseOne(void). - const auto ch = tk.remaining()[0]; -- output.appendf("%%%02X", static_cast(ch)); // TODO: Optimize using a table -+ output.appendf("%%%02X", static_cast(static_cast(ch))); // TODO: Optimize using a table - (void)tk.skip(ch); - - if (tk.prefix(goodSection, ignore)) --- -2.25.1 - diff --git a/backport-CVE-2023-49285.patch b/backport-CVE-2023-49285.patch deleted file mode 100644 index 578693c..0000000 --- a/backport-CVE-2023-49285.patch +++ /dev/null @@ -1,34 +0,0 @@ -From deee944f9a12c9fd399ce52f3e2526bb573a9470 Mon Sep 17 00:00:00 2001 -From: Alex Rousskov -Date: Wed, 25 Oct 2023 19:41:45 +0000 -Subject: [PATCH] RFC 1123: Fix date parsing (#1538) - -The bug was discovered and detailed by Joshua Rogers at -https://megamansec.github.io/Squid-Security-Audit/datetime-overflow.html -where it was filed as "1-Byte Buffer OverRead in RFC 1123 date/time -Handling". - -Conflict:NA -Reference:https://github.com/squid-cache/squid/commit/deee944f9a12c9fd399ce52f3e2526bb573a9470 ---- - src/time/rfc1123.cc | 6 ++++++ - 1 file changed, 6 insertions(+) - -diff --git a/src/time/rfc1123.cc b/src/time/rfc1123.cc -index d89d22262f6..7524959edb0 100644 ---- a/src/time/rfc1123.cc -+++ b/src/time/rfc1123.cc -@@ -50,7 +50,13 @@ make_month(const char *s) - char month[3]; - - month[0] = xtoupper(*s); -+ if (!month[0]) -+ return -1; // protects *(s + 1) below -+ - month[1] = xtolower(*(s + 1)); -+ if (!month[1]) -+ return -1; // protects *(s + 2) below -+ - month[2] = xtolower(*(s + 2)); - - for (i = 0; i < 12; i++) diff --git a/backport-CVE-2023-49286.patch b/backport-CVE-2023-49286.patch deleted file mode 100644 index 386cd12..0000000 --- a/backport-CVE-2023-49286.patch +++ /dev/null @@ -1,85 +0,0 @@ -From 6014c6648a2a54a4ecb7f952ea1163e0798f9264 Mon Sep 17 00:00:00 2001 -From: Alex Rousskov -Date: Fri, 27 Oct 2023 21:27:20 +0000 -Subject: [PATCH] Exit without asserting when helper process startup fails - (#1543) - -... to dup() after fork() and before execvp(). - -Assertions are for handling program logic errors. Helper initialization -code already handled system call errors correctly (i.e. by exiting the -newly created helper process with an error), except for a couple of -assert()s that could be triggered by dup(2) failures. - -This bug was discovered and detailed by Joshua Rogers at -https://megamansec.github.io/Squid-Security-Audit/ipc-assert.html -where it was filed as 'Assertion in Squid "Helper" Process Creator'. - -Conflict:NA -Reference:https://github.com/squid-cache/squid/commit/6014c6648a2a54a4ecb7f952ea1163e0798f9264 ---- - src/ipc.cc | 32 ++++++++++++++++++++++++++------ - 1 file changed, 26 insertions(+), 6 deletions(-) - -diff --git a/src/ipc.cc b/src/ipc.cc -index 40d34b4755a..1afc4d5cf3c 100644 ---- a/src/ipc.cc -+++ b/src/ipc.cc -@@ -22,6 +22,11 @@ - - #include - #include -+#include -+ -+#if HAVE_UNISTD_H -+#include -+#endif - - static const char *hello_string = "hi there\n"; - #ifndef HELLO_BUF_SZ -@@ -362,6 +367,22 @@ ipcCreate(int type, const char *prog, const char *const args[], const char *name - } - - PutEnvironment(); -+ -+ // A dup(2) wrapper that reports and exits the process on errors. The -+ // exiting logic is only suitable for this child process context. -+ const auto dupOrExit = [prog,name](const int oldFd) { -+ const auto newFd = dup(oldFd); -+ if (newFd < 0) { -+ const auto savedErrno = errno; -+ debugs(54, DBG_CRITICAL, "ERROR: Helper process initialization failure: " << name << -+ Debug::Extra << "helper (CHILD) PID: " << getpid() << -+ Debug::Extra << "helper program name: " << prog << -+ Debug::Extra << "dup(2) system call error for FD " << oldFd << ": " << xstrerr(savedErrno)); -+ _exit(EXIT_FAILURE); -+ } -+ return newFd; -+ }; -+ - /* - * This double-dup stuff avoids problems when one of - * crfd, cwfd, or debug_log are in the rage 0-2. -@@ -369,17 +390,16 @@ ipcCreate(int type, const char *prog, const char *const args[], const char *name - - do { - /* First make sure 0-2 is occupied by something. Gets cleaned up later */ -- x = dup(crfd); -- assert(x > -1); -- } while (x < 3 && x > -1); -+ x = dupOrExit(crfd); -+ } while (x < 3); - - close(x); - -- t1 = dup(crfd); -+ t1 = dupOrExit(crfd); - -- t2 = dup(cwfd); -+ t2 = dupOrExit(cwfd); - -- t3 = dup(fileno(debug_log)); -+ t3 = dupOrExit(fileno(debug_log)); - - assert(t1 > 2 && t2 > 2 && t3 > 2); - diff --git a/backport-CVE-2023-50269.patch b/backport-CVE-2023-50269.patch deleted file mode 100644 index 9853182..0000000 --- a/backport-CVE-2023-50269.patch +++ /dev/null @@ -1,86 +0,0 @@ -From 45b6522eb80a6d12f75630fe1c132b52fc3f1624 Mon Sep 17 00:00:00 2001 -From: Thomas Leroy <32497783+p4zuu@users.noreply.github.com> -Date: Tue, 28 Nov 2023 07:35:46 +0000 -Subject: [PATCH] Limit the number of allowed X-Forwarded-For hops (#1589) - -Squid will ignore all X-Forwarded-For elements listed after the first 64 -addresses allowed by the follow_x_forwarded_for directive. A different -limit can be specified by defining a C++ SQUID_X_FORWARDED_FOR_HOP_MAX -macro, but that macro is not a supported Squid configuration interface -and may change or disappear at any time. - -Squid will log a cache.log ERROR if the hop limit has been reached. - -This change works around problematic ACLChecklist and/or slow ACLs -implementation that results in immediate nonBlockingCheck() callbacks. -Such callbacks have caused many bugs and development complications. In -clientFollowXForwardedForCheck() context, they lead to indirect -recursion that was bound only by the number of allowed XFF entries, -which could reach thousands and exhaust Squid process call stack. - -This recursion bug was discovered and detailed by Joshua Rogers at -https://megamansec.github.io/Squid-Security-Audit/xff-stackoverflow.html -where it was filed as "X-Forwarded-For Stack Overflow". - -Conflict: NA -Reference: https://github.com/squid-cache/squid/commit/45b6522eb80a6d12f75630fe1c132b52fc3f1624 ---- - src/ClientRequestContext.h | 7 ++++++- - src/client_side_request.cc | 17 +++++++++++++++-- - 2 files changed, 21 insertions(+), 3 deletions(-) - -diff --git a/src/ClientRequestContext.h b/src/ClientRequestContext.h -index 16c33bba4bc..0997f5d22b6 100644 ---- a/src/ClientRequestContext.h -+++ b/src/ClientRequestContext.h -@@ -80,8 +80,13 @@ class ClientRequestContext : public RefCountable - #if USE_OPENSSL - bool sslBumpCheckDone = false; - #endif -- ErrorState *error = nullptr; ///< saved error page for centralized/delayed processing -+ - bool readNextRequest = false; ///< whether Squid should read after error handling -+ ErrorState *error = nullptr; ///< saved error page for centralized/delayed processing -+ -+#if FOLLOW_X_FORWARDED_FOR -+ size_t currentXffHopNumber = 0; ///< number of X-Forwarded-For header values processed so far -+#endif - }; - - #endif /* SQUID_CLIENTREQUESTCONTEXT_H */ -diff --git a/src/client_side_request.cc b/src/client_side_request.cc -index 5b5b5af8086..7f802d4219e 100644 ---- a/src/client_side_request.cc -+++ b/src/client_side_request.cc -@@ -75,6 +75,11 @@ - #endif - - #if FOLLOW_X_FORWARDED_FOR -+ -+#if !defined(SQUID_X_FORWARDED_FOR_HOP_MAX) -+#define SQUID_X_FORWARDED_FOR_HOP_MAX 64 -+#endif -+ - static void clientFollowXForwardedForCheck(Acl::Answer answer, void *data); - #endif /* FOLLOW_X_FORWARDED_FOR */ - -@@ -437,8 +442,16 @@ clientFollowXForwardedForCheck(Acl::Answer answer, void *data) - /* override the default src_addr tested if we have to go deeper than one level into XFF */ - Filled(calloutContext->acl_checklist)->src_addr = request->indirect_client_addr; - } -- calloutContext->acl_checklist->nonBlockingCheck(clientFollowXForwardedForCheck, data); -- return; -+ if (++calloutContext->currentXffHopNumber < SQUID_X_FORWARDED_FOR_HOP_MAX) { -+ calloutContext->acl_checklist->nonBlockingCheck(clientFollowXForwardedForCheck, data); -+ return; -+ } -+ const auto headerName = Http::HeaderLookupTable.lookup(Http::HdrType::X_FORWARDED_FOR).name; -+ debugs(28, DBG_CRITICAL, "ERROR: Ignoring trailing " << headerName << " addresses" << -+ Debug::Extra << "addresses allowed by follow_x_forwarded_for: " << calloutContext->currentXffHopNumber << -+ Debug::Extra << "last/accepted address: " << request->indirect_client_addr << -+ Debug::Extra << "ignored trailing addresses: " << request->x_forwarded_for_iterator); -+ // fall through to resume clientAccessCheck() processing - } - } - --- diff --git a/backport-squid-crash-half-closed.patch b/backport-squid-crash-half-closed.patch new file mode 100644 index 0000000..f5046c1 --- /dev/null +++ b/backport-squid-crash-half-closed.patch @@ -0,0 +1,196 @@ +From 5da786ef2a708559f5b53a05b7db6de0b64ce885 Mon Sep 17 00:00:00 2001 +From: Alex Rousskov +Date: Mon, 11 Sep 2023 07:49:36 +0000 +Subject: [PATCH] Bug 4156: comm.cc "!commHasHalfClosedMonitor(fd)" assertion + (#1443) + +This bug is specific to "half_closed_clients on" configurations. + + assertion failed: ... "isOpen(fd) && !commHasHalfClosedMonitor(fd)" + location: comm.cc:1583 in commStartHalfClosedMonitor() + +Squid asserts because Server schedules comm_read() after receiving EOF: +That extra read results in another EOF notification, and an attempt to +start monitoring an already monitored half-closed connection. + +Upon detecting a potentially half-closed connection, +Server::doClientRead() should clear flags.readMore to prevent Server +from scheduling another comm_read(), but it does not and cannot do that +(without significant refactoring) because + +* Server does not have access to flags.readMore +* flags.readMore hack is used for more than just "read more" + +We worked around the above limitation by re-detecting half-closed +conditions and clearing flags.readMore after clientParseRequests(). That +fixed the bug but further increased poor code duplication across +ConnStateData::afterClientRead() and ConnStateData::kick() methods. We +then refactored by merging and moving that duplicated code into +clientParseRequests() and renamed that method to make backports safer. + +Conflict: NA +Reference: https://github.com/squid-cache/squid/commit/5da786 +--- + src/client_side.cc | 67 ++++++++++++----------------------- + src/client_side.h | 2 +- + src/tests/stub_client_side.cc | 2 +- + 3 files changed, 24 insertions(+), 47 deletions(-) + +diff --git a/src/client_side.cc b/src/client_side.cc +index e62bcf0fc7d..bd9cf6a7d5f 100644 +--- a/src/client_side.cc ++++ b/src/client_side.cc +@@ -932,7 +932,7 @@ ConnStateData::kick() + * We are done with the response, and we are either still receiving request + * body (early response!) or have already stopped receiving anything. + * +- * If we are still receiving, then clientParseRequest() below will fail. ++ * If we are still receiving, then parseRequests() below will fail. + * (XXX: but then we will call readNextRequest() which may succeed and + * execute a smuggled request as we are not done with the current request). + * +@@ -952,28 +952,12 @@ ConnStateData::kick() + * Attempt to parse a request from the request buffer. + * If we've been fed a pipelined request it may already + * be in our read buffer. +- * +- \par +- * This needs to fall through - if we're unlucky and parse the _last_ request +- * from our read buffer we may never re-register for another client read. + */ + +- if (clientParseRequests()) { +- debugs(33, 3, clientConnection << ": parsed next request from buffer"); +- } ++ parseRequests(); + +- /** \par +- * Either we need to kick-start another read or, if we have +- * a half-closed connection, kill it after the last request. +- * This saves waiting for half-closed connections to finished being +- * half-closed _AND_ then, sometimes, spending "Timeout" time in +- * the keepalive "Waiting for next request" state. +- */ +- if (commIsHalfClosed(clientConnection->fd) && pipeline.empty()) { +- debugs(33, 3, "half-closed client with no pending requests, closing"); +- clientConnection->close(); ++ if (!isOpen()) + return; +- } + + /** \par + * At this point we either have a parsed request (which we've +@@ -1882,16 +1866,11 @@ ConnStateData::receivedFirstByte() + resetReadTimeout(Config.Timeout.request); + } + +-/** +- * Attempt to parse one or more requests from the input buffer. +- * Returns true after completing parsing of at least one request [header]. That +- * includes cases where parsing ended with an error (e.g., a huge request). +- */ +-bool +-ConnStateData::clientParseRequests() ++/// Attempt to parse one or more requests from the input buffer. ++/// May close the connection. ++void ++ConnStateData::parseRequests() + { +- bool parsed_req = false; +- + debugs(33, 5, clientConnection << ": attempting to parse"); + + // Loop while we have read bytes that are not needed for producing the body +@@ -1936,8 +1915,6 @@ ConnStateData::clientParseRequests() + + processParsedRequest(context); + +- parsed_req = true; // XXX: do we really need to parse everything right NOW ? +- + if (context->mayUseConnection()) { + debugs(33, 3, "Not parsing new requests, as this request may need the connection"); + break; +@@ -1950,8 +1927,19 @@ ConnStateData::clientParseRequests() + } + } + +- /* XXX where to 'finish' the parsing pass? */ +- return parsed_req; ++ debugs(33, 7, "buffered leftovers: " << inBuf.length()); ++ ++ if (isOpen() && commIsHalfClosed(clientConnection->fd)) { ++ if (pipeline.empty()) { ++ // we processed what we could parse, and no more data is coming ++ debugs(33, 5, "closing half-closed without parsed requests: " << clientConnection); ++ clientConnection->close(); ++ } else { ++ // we parsed what we could, and no more data is coming ++ debugs(33, 5, "monitoring half-closed while processing parsed requests: " << clientConnection); ++ flags.readMore = false; // may already be false ++ } ++ } + } + + void +@@ -1968,18 +1956,7 @@ ConnStateData::afterClientRead() + if (pipeline.empty()) + fd_note(clientConnection->fd, "Reading next request"); + +- if (!clientParseRequests()) { +- if (!isOpen()) +- return; +- // We may get here if the client half-closed after sending a partial +- // request. See doClientRead() and shouldCloseOnEof(). +- // XXX: This partially duplicates ConnStateData::kick(). +- if (pipeline.empty() && commIsHalfClosed(clientConnection->fd)) { +- debugs(33, 5, clientConnection << ": half-closed connection, no completed request parsed, connection closing."); +- clientConnection->close(); +- return; +- } +- } ++ parseRequests(); + + if (!isOpen()) + return; +@@ -3767,7 +3744,7 @@ ConnStateData::notePinnedConnectionBecameIdle(PinnedIdleContext pic) + startPinnedConnectionMonitoring(); + + if (pipeline.empty()) +- kick(); // in case clientParseRequests() was blocked by a busy pic.connection ++ kick(); // in case parseRequests() was blocked by a busy pic.connection + } + + /// Forward future client requests using the given server connection. +diff --git a/src/client_side.h b/src/client_side.h +index e37ab27da1a..9f36d864c2c 100644 +--- a/src/client_side.h ++++ b/src/client_side.h +@@ -98,7 +98,6 @@ class ConnStateData: + void doneWithControlMsg() override; + + /// Traffic parsing +- bool clientParseRequests(); + void readNextRequest(); + + /// try to make progress on a transaction or read more I/O +@@ -443,6 +442,7 @@ class ConnStateData: + + void checkLogging(); + ++ void parseRequests(); + void clientAfterReadingRequests(); + bool concurrentRequestQueueFilled() const; + +diff --git a/src/tests/stub_client_side.cc b/src/tests/stub_client_side.cc +index 8c160e56340..f49d5dceeed 100644 +--- a/src/tests/stub_client_side.cc ++++ b/src/tests/stub_client_side.cc +@@ -14,7 +14,7 @@ + #include "tests/STUB.h" + + #include "client_side.h" +-bool ConnStateData::clientParseRequests() STUB_RETVAL(false) ++void ConnStateData::parseRequests() STUB + void ConnStateData::readNextRequest() STUB + bool ConnStateData::isOpen() const STUB_RETVAL(false) + void ConnStateData::kick() STUB diff --git a/squid-6.1.tar.xz b/squid-6.1.tar.xz deleted file mode 100644 index 4a56881..0000000 Binary files a/squid-6.1.tar.xz and /dev/null differ diff --git a/squid-6.1.tar.xz.asc b/squid-6.1.tar.xz.asc deleted file mode 100644 index 023c376..0000000 --- a/squid-6.1.tar.xz.asc +++ /dev/null @@ -1,25 +0,0 @@ -File: squid-6.1.tar.xz -Date: Thu 06 Jul 2023 05:42:59 UTC -Size: 2546668 -MD5 : 64841841e06ea487b0305070c33b04dd -SHA1: 4a3711e42ca9acbba580fd0c306cc2f6f84db1f8 -Key : CD6DBF8EF3B17D3E - B068 84ED B779 C89B 044E 64E3 CD6D BF8E F3B1 7D3E - keyring = http://www.squid-cache.org/pgp.asc - keyserver = pool.sks-keyservers.net ------BEGIN PGP SIGNATURE----- - -iQIzBAABCgAdFiEEsGiE7bd5yJsETmTjzW2/jvOxfT4FAmSmVGsACgkQzW2/jvOx -fT6rPA/9Gkkw4w3h0EFPI7mHvjquoc2mW/zX5clfZmhoA81u9xRUazgZeaPcLydF -rwRxNPITWzZ+emqeTMLrrVndHXAqKqg8VRbHymWEAh3aqVzvaj98h9iZylvPLX1N -rdToqDBET8E4YHYUsmUdoegg5XgGiFEC3fqcS7/3eOUUoWsrFsKy6WAF2g1lS5yx -kqfRA5Nim23ckACsoRcMPfVPHxRFuyoYiLAsgMjx2cZAGDVtFos260N1QK8xdQkE -o/LEv2zp4wXFMeLSJJsvgl9SfqA7XtC91ZH8nrAvmWwj99Totnt5KEa8MiF0Wn0K -dpB2X1meb/dx8CI9AMNex9hedUspPlAMLCI8ggR8KtzW31g/7GagXpsKmJIEdk+S -Yjq4NXHS0eDmiMcI2ZDBp6Sk/ty1VrnH61GqA8FDEOTTAJGvFu6DahVgxHE6s0aj -pOw8AmM/0yP2kuafhchbRQQ9bCFBsO4z4sUyGNkHNHCjX3XimW3m4mBPSlEkDAF2 -dbUdAwIjBdS8zdU0N6wB+WXy7y459bsQBkWQMc7P4TqQ02IeL+4boF2c8RpgWiDf -hHS03U60zAP36m6HlC1nSnGnMABlwvBPg928yMq/jrf75T5DQHOSEuQ69NxF61ge -SLGX+aEGwwXGsHhGfW6W9sORKaiJNI683US3vGOn33CX+L5rCIU= -=hwBL ------END PGP SIGNATURE----- diff --git a/squid-6.6.tar.xz b/squid-6.6.tar.xz new file mode 100644 index 0000000..221c6ca Binary files /dev/null and b/squid-6.6.tar.xz differ diff --git a/squid-6.6.tar.xz.asc b/squid-6.6.tar.xz.asc new file mode 100644 index 0000000..5059576 --- /dev/null +++ b/squid-6.6.tar.xz.asc @@ -0,0 +1,25 @@ +File: squid-6.6.tar.xz +Date: Thu 07 Dec 2023 04:03:46 UTC +Size: 2554824 +MD5 : 5a41134ee1b7e75f62088acdec92d2ca +SHA1: f05e06a9dd3bf7501d2844e43d9ae1bd00e9edcc +Key : CD6DBF8EF3B17D3E + B068 84ED B779 C89B 044E 64E3 CD6D BF8E F3B1 7D3E + keyring = http://www.squid-cache.org/pgp.asc + keyserver = pool.sks-keyservers.net +-----BEGIN PGP SIGNATURE----- + +iQIzBAABCgAdFiEEsGiE7bd5yJsETmTjzW2/jvOxfT4FAmVxRCsACgkQzW2/jvOx +fT5VtQ/+M+mhaGYCp9YBi1GG9vyQwkkIyngL3vPpz7UxZHAR+mzk29zwlgdDgwWA +Zasaomg8S1Clq2dhNr7oo6RuZ7mKlhEeHba2WvL+1/VcBsPnazUwzYQiW7k9KxYe +n1At62duit+YnswTNnj6HJRKKK0nKlPmJycL1AThh9Tj6oHTsWBCItnSZ5eUjGX0 +aKiMrkrHtq3qheWkVZPCJEFDs88ECDrJD7s9cpAhun+/0v+4ECE65uJ2bZHK4f/E +TH5OIf8vltEB8sA/SSanMM/C+gZObET3TssrgHz92j0svMOlALLtitb0aHly21JV +fEKB200Ngac2y6rq3xDNiznmMn+SeCNUsiDcdauCrsUHNW9S9FhOxeWXy/Z7JK4A +mqVnnqvN9GFvv2EEC8J9lj+cwGOdaSW6L2aPVkub8Ij5O+e2Tg+uBm4ZC8vcACYz ++1oo8YyvcfO9EmNRE0vpFTWH9Ux5ptgdvsIxv41QN40RUYN7FBbOgey59mP3uq2Q +0g/b8lr1PnrwB74OrVGcXLwREFLXtkRC9vcdNjvdchCg60KlBNWEPSGJA2adS8HJ +4AGyVpU8GCpV3q74rJxIG6FUffL85CfT+1HRmQhzYiGJDzy1AaUJmcelyS4e6cjn +urAWH3mlAaPzj87OuaeZYGAZMWh/5iAarU+VHkZn6vI2Mvl9yMA= +=oyMI +-----END PGP SIGNATURE----- diff --git a/squid.spec b/squid.spec index 554c826..f44b0fa 100644 --- a/squid.spec +++ b/squid.spec @@ -1,8 +1,8 @@ %define __perl_requires %{SOURCE8} Name: squid -Version: 6.1 -Release: 5 +Version: 6.6 +Release: 1 Summary: The Squid proxy caching server Epoch: 7 License: GPLv2+ and (LGPLv2+ and MIT and BSD and Public Domain) @@ -21,15 +21,7 @@ Patch0: squid-4.0.11-config.patch Patch1: squid-3.1.0.9-location.patch Patch2: squid-3.0.STABLE1-perlpath.patch Patch3: backport-squid-6.1-symlink-lang-err.patch -Patch4: backport-0001-CVE-2023-5824.patch -Patch5: backport-0002-CVE-2023-5824.patch -Patch6: backport-CVE-2023-46846.patch -Patch7: backport-CVE-2023-46847.patch -Patch8: backport-CVE-2023-46848.patch -Patch9: backport-CVE-2023-46724.patch -Patch10: backport-CVE-2023-49285.patch -Patch11: backport-CVE-2023-49286.patch -Patch12: backport-CVE-2023-50269.patch +Patch4: backport-squid-crash-half-closed.patch Requires: bash Requires: httpd-filesystem @@ -252,6 +244,12 @@ fi chgrp squid /var/cache/samba/winbindd_privileged >/dev/null 2>&1 || : %changelog +* Tue Dec 26 2023 xinghe - 7:6.6-1 +- Type:requirements +- ID:NA +- SUG:NA +- DESC:upgrade to 6.6 + * Fri Dec 15 2023 xinghe - 7:6.1-5 - Type:cves - ID:CVE-2023-50269