summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVijay Anusuri <vanusuri@mvista.com>2024-09-02 19:25:08 +0530
committerArmin Kuster <akuster808@gmail.com>2024-09-22 10:12:40 -0400
commit07b6c57f4aa315f1a5292a2ed0c43f5f992e0a61 (patch)
tree011e6ff60bfddd94b45acfdd2dd121992ffc43f1
parent31d7500290b8ac7f8d41b910ca2dbb9893a07be7 (diff)
downloadmeta-openembedded-07b6c57f4aa315f1a5292a2ed0c43f5f992e0a61.tar.gz
squid: Security fix CVE-2023-5824
References: https://access.redhat.com/security/cve/cve-2023-5824 https://access.redhat.com/errata/RHSA-2023:7668 The patch is from RHEL8. Signed-off-by: Vijay Anusuri <vanusuri@mvista.com> Signed-off-by: Armin Kuster <akuster808@gmail.com>
Diffstat
-rw-r--r--meta-networking/recipes-daemons/squid/files/CVE-2023-5824.patch4340
-rw-r--r--meta-networking/recipes-daemons/squid/squid_4.15.bb1
2 files changed, 4341 insertions, 0 deletions
diff --git a/meta-networking/recipes-daemons/squid/files/CVE-2023-5824.patch b/meta-networking/recipes-daemons/squid/files/CVE-2023-5824.patch
new file mode 100644
index 0000000000..4946060313
--- /dev/null
+++ b/meta-networking/recipes-daemons/squid/files/CVE-2023-5824.patch
@@ -0,0 +1,4340 @@
1commit bf9a9ec5329bde6acc26797d1fa7a7a165fec01f
2Author: Tomas Korbar <tkorbar@redhat.com>
3Date: Tue Nov 21 13:21:43 2023 +0100
4
5 Fix CVE-2023-5824 (#1335) (#1561) (#1562)
6 Supply ALE with HttpReply before checking http_reply_access (#398)
7 Replace adjustable base reply - downstream change neccessary for
8 backport
9
10Upstream-Status: Backport [RedHat RHEL8 squid-4.15-7.module+el8.9.0+20806+014d88aa.3.src.rpm]
11CVE: CVE-2023-5824
12Signed-off-by: Vijay Anusuri <vanusuri@mvista.com>
13
14diff --git a/src/AccessLogEntry.cc b/src/AccessLogEntry.cc
15index 1956c9b..4f1e73e 100644
16--- a/src/AccessLogEntry.cc
17+++ b/src/AccessLogEntry.cc
18@@ -10,6 +10,7 @@
19 #include "AccessLogEntry.h"
20 #include "HttpReply.h"
21 #include "HttpRequest.h"
22+#include "MemBuf.h"
23 #include "SquidConfig.h"
24
25 #if USE_OPENSSL
26@@ -89,6 +90,8 @@ AccessLogEntry::getExtUser() const
27 return nullptr;
28 }
29
30+AccessLogEntry::AccessLogEntry() {}
31+
32 AccessLogEntry::~AccessLogEntry()
33 {
34 safe_free(headers.request);
35@@ -97,14 +100,11 @@ AccessLogEntry::~AccessLogEntry()
36 safe_free(adapt.last_meta);
37 #endif
38
39- safe_free(headers.reply);
40-
41 safe_free(headers.adapted_request);
42 HTTPMSGUNLOCK(adapted_request);
43
44 safe_free(lastAclName);
45
46- HTTPMSGUNLOCK(reply);
47 HTTPMSGUNLOCK(request);
48 #if ICAP_CLIENT
49 HTTPMSGUNLOCK(icap.reply);
50@@ -124,3 +124,10 @@ AccessLogEntry::effectiveVirginUrl() const
51 return nullptr;
52 }
53
54+void
55+AccessLogEntry::packReplyHeaders(MemBuf &mb) const
56+{
57+ if (reply)
58+ reply->packHeadersUsingFastPacker(mb);
59+}
60+
61diff --git a/src/AccessLogEntry.h b/src/AccessLogEntry.h
62index 1f29e61..f1d2ecc 100644
63--- a/src/AccessLogEntry.h
64+++ b/src/AccessLogEntry.h
65@@ -40,13 +40,7 @@ class AccessLogEntry: public RefCountable
66 public:
67 typedef RefCount<AccessLogEntry> Pointer;
68
69- AccessLogEntry() :
70- url(nullptr),
71- lastAclName(nullptr),
72- reply(nullptr),
73- request(nullptr),
74- adapted_request(nullptr)
75- {}
76+ AccessLogEntry();
77 ~AccessLogEntry();
78
79 /// Fetch the client IP log string into the given buffer.
80@@ -63,6 +57,9 @@ public:
81 /// Fetch the transaction method string (ICP opcode, HTCP opcode or HTTP method)
82 SBuf getLogMethod() const;
83
84+ /// dump all reply headers (for sending or risky logging)
85+ void packReplyHeaders(MemBuf &mb) const;
86+
87 SBuf url;
88
89 /// TCP/IP level details about the client connection
90@@ -187,14 +184,12 @@ public:
91
92 public:
93 Headers() : request(NULL),
94- adapted_request(NULL),
95- reply(NULL) {}
96+ adapted_request(NULL)
97+ {}
98
99 char *request; //< virgin HTTP request headers
100
101 char *adapted_request; //< HTTP request headers after adaptation and redirection
102-
103- char *reply;
104 } headers;
105
106 #if USE_ADAPTATION
107@@ -212,13 +207,13 @@ public:
108 } adapt;
109 #endif
110
111- const char *lastAclName; ///< string for external_acl_type %ACL format code
112+ const char *lastAclName = nullptr; ///< string for external_acl_type %ACL format code
113 SBuf lastAclData; ///< string for external_acl_type %DATA format code
114
115 HierarchyLogEntry hier;
116- HttpReply *reply;
117- HttpRequest *request; //< virgin HTTP request
118- HttpRequest *adapted_request; //< HTTP request after adaptation and redirection
119+ HttpReplyPointer reply;
120+ HttpRequest *request = nullptr; //< virgin HTTP request
121+ HttpRequest *adapted_request = nullptr; //< HTTP request after adaptation and redirection
122
123 /// key:value pairs set by squid.conf note directive and
124 /// key=value pairs returned from URL rewrite/redirect helper
125diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc
126index 8dcc7e3..21206a9 100644
127--- a/src/HttpHeader.cc
128+++ b/src/HttpHeader.cc
129@@ -9,6 +9,7 @@
130 /* DEBUG: section 55 HTTP Header */
131
132 #include "squid.h"
133+#include "base/Assure.h"
134 #include "base/EnumIterator.h"
135 #include "base64.h"
136 #include "globals.h"
137diff --git a/src/HttpHeaderTools.cc b/src/HttpHeaderTools.cc
138index f1e45a4..1337b8d 100644
139--- a/src/HttpHeaderTools.cc
140+++ b/src/HttpHeaderTools.cc
141@@ -479,7 +479,7 @@ httpHdrAdd(HttpHeader *heads, HttpRequest *request, const AccessLogEntryPointer
142
143 checklist.al = al;
144 if (al && al->reply) {
145- checklist.reply = al->reply;
146+ checklist.reply = al->reply.getRaw();
147 HTTPMSGLOCK(checklist.reply);
148 }
149
150diff --git a/src/HttpReply.cc b/src/HttpReply.cc
151index 6feb262..e74960b 100644
152--- a/src/HttpReply.cc
153+++ b/src/HttpReply.cc
154@@ -20,7 +20,9 @@
155 #include "HttpReply.h"
156 #include "HttpRequest.h"
157 #include "MemBuf.h"
158+#include "sbuf/Stream.h"
159 #include "SquidConfig.h"
160+#include "SquidMath.h"
161 #include "SquidTime.h"
162 #include "Store.h"
163 #include "StrList.h"
164@@ -524,6 +526,38 @@ HttpReply::expectedBodyTooLarge(HttpRequest& request)
165 return expectedSize > bodySizeMax;
166 }
167
168+size_t
169+HttpReply::parseTerminatedPrefix(const char * const terminatedBuf, const size_t bufSize)
170+{
171+ auto error = Http::scNone;
172+ const bool eof = false; // TODO: Remove after removing atEnd from HttpHeader::parse()
173+ if (parse(terminatedBuf, bufSize, eof, &error)) {
174+ debugs(58, 7, "success after accumulating " << bufSize << " bytes and parsing " << hdr_sz);
175+ Assure(pstate == psParsed);
176+ Assure(hdr_sz > 0);
177+ Assure(!Less(bufSize, hdr_sz)); // cannot parse more bytes than we have
178+ return hdr_sz; // success
179+ }
180+
181+ Assure(pstate != psParsed);
182+ hdr_sz = 0;
183+
184+ if (error) {
185+ throw TextException(ToSBuf("failed to parse HTTP headers",
186+ Debug::Extra, "parser error code: ", error,
187+ Debug::Extra, "accumulated unparsed bytes: ", bufSize,
188+ Debug::Extra, "reply_header_max_size: ", Config.maxReplyHeaderSize),
189+ Here());
190+ }
191+
192+ debugs(58, 3, "need more bytes after accumulating " << bufSize << " out of " << Config.maxReplyHeaderSize);
193+
194+ // the parse() call above enforces Config.maxReplyHeaderSize limit
195+ // XXX: Make this a strict comparison after fixing Http::Message::parse() enforcement
196+ Assure(bufSize <= Config.maxReplyHeaderSize);
197+ return 0; // parsed nothing, need more data
198+}
199+
200 void
201 HttpReply::calcMaxBodySize(HttpRequest& request) const
202 {
203diff --git a/src/HttpReply.h b/src/HttpReply.h
204index 6c90e20..4301cfd 100644
205--- a/src/HttpReply.h
206+++ b/src/HttpReply.h
207@@ -121,6 +121,13 @@ public:
208 /// \returns false if any information is missing
209 bool olderThan(const HttpReply *them) const;
210
211+ /// Parses response status line and headers at the start of the given
212+ /// NUL-terminated buffer of the given size. Respects reply_header_max_size.
213+ /// Assures pstate becomes Http::Message::psParsed on (and only on) success.
214+ /// \returns the number of bytes in a successfully parsed prefix (or zero)
215+ /// \retval 0 implies that more data is needed to parse the response prefix
216+ size_t parseTerminatedPrefix(const char *, size_t);
217+
218 private:
219 /** initialize */
220 void init();
221diff --git a/src/MemObject.cc b/src/MemObject.cc
222index df7791f..650d3fd 100644
223--- a/src/MemObject.cc
224+++ b/src/MemObject.cc
225@@ -196,8 +196,8 @@ struct LowestMemReader : public unary_function<store_client, void> {
226 LowestMemReader(int64_t seed):current(seed) {}
227
228 void operator() (store_client const &x) {
229- if (x.memReaderHasLowerOffset(current))
230- current = x.copyInto.offset;
231+ if (x.getType() == STORE_MEM_CLIENT)
232+ current = std::min(current, x.discardableHttpEnd());
233 }
234
235 int64_t current;
236@@ -369,6 +369,12 @@ MemObject::policyLowestOffsetToKeep(bool swap) const
237 */
238 int64_t lowest_offset = lowestMemReaderOffset();
239
240+ // XXX: Remove the last (Config.onoff.memory_cache_first-based) condition
241+ // and update keepForLocalMemoryCache() accordingly. The caller wants to
242+ // remove all local memory that is safe to remove. Honoring caching
243+ // preferences is its responsibility. Our responsibility is safety. The
244+ // situation was different when ff4b33f added that condition -- there was no
245+ // keepInLocalMemory/keepForLocalMemoryCache() call guard back then.
246 if (endOffset() < lowest_offset ||
247 endOffset() - inmem_lo > (int64_t)Config.Store.maxInMemObjSize ||
248 (swap && !Config.onoff.memory_cache_first))
249@@ -492,7 +498,7 @@ MemObject::mostBytesAllowed() const
250
251 #endif
252
253- j = sc->delayId.bytesWanted(0, sc->copyInto.length);
254+ j = sc->bytesWanted();
255
256 if (j > jmax) {
257 jmax = j;
258diff --git a/src/MemObject.h b/src/MemObject.h
259index 711966d..9f4add0 100644
260--- a/src/MemObject.h
261+++ b/src/MemObject.h
262@@ -56,9 +56,23 @@ public:
263
264 void write(const StoreIOBuffer &buf);
265 void unlinkRequest();
266+
267+ /// HTTP response before 304 (Not Modified) updates
268+ /// starts "empty"; modified via replaceBaseReply() or adjustableBaseReply()
269+ HttpReply &baseReply() const { return *_reply; }
270+
271 HttpReply const *getReply() const;
272 void replaceHttpReply(HttpReply *newrep);
273 void stat (MemBuf * mb) const;
274+
275+ /// The offset of the last memory-stored HTTP response byte plus one.
276+ /// * HTTP response headers (if any) are stored at offset zero.
277+ /// * HTTP response body byte[n] usually has offset (hdr_sz + n), where
278+ /// hdr_sz is the size of stored HTTP response headers (zero if none); and
279+ /// n is the corresponding byte offset in the whole resource body.
280+ /// However, some 206 (Partial Content) response bodies are stored (and
281+ /// retrieved) as regular 200 response bodies, disregarding offsets of
282+ /// their body parts. \sa HttpStateData::decideIfWeDoRanges().
283 int64_t endOffset () const;
284 void markEndOfReplyHeaders(); ///< sets _reply->hdr_sz to endOffset()
285 /// negative if unknown; otherwise, expected object_sz, expected endOffset
286diff --git a/src/MemStore.cc b/src/MemStore.cc
287index a4a6ab2..6762c4f 100644
288--- a/src/MemStore.cc
289+++ b/src/MemStore.cc
290@@ -17,6 +17,8 @@
291 #include "MemObject.h"
292 #include "MemStore.h"
293 #include "mime_header.h"
294+#include "sbuf/SBuf.h"
295+#include "sbuf/Stream.h"
296 #include "SquidConfig.h"
297 #include "SquidMath.h"
298 #include "StoreStats.h"
299@@ -316,19 +318,25 @@ MemStore::get(const cache_key *key)
300 // create a brand new store entry and initialize it with stored info
301 StoreEntry *e = new StoreEntry();
302
303- // XXX: We do not know the URLs yet, only the key, but we need to parse and
304- // store the response for the Root().find() callers to be happy because they
305- // expect IN_MEMORY entries to already have the response headers and body.
306- e->createMemObject();
307-
308- anchorEntry(*e, index, *slot);
309-
310- const bool copied = copyFromShm(*e, index, *slot);
311-
312- if (copied)
313- return e;
314+ try {
315+ // XXX: We do not know the URLs yet, only the key, but we need to parse and
316+ // store the response for the Root().find() callers to be happy because they
317+ // expect IN_MEMORY entries to already have the response headers and body.
318+ e->createMemObject();
319+
320+ anchorEntry(*e, index, *slot);
321+
322+ // TODO: make copyFromShm() throw on all failures, simplifying this code
323+ if (copyFromShm(*e, index, *slot))
324+ return e;
325+ debugs(20, 3, "failed for " << *e);
326+ } catch (...) {
327+ // see store_client::parseHttpHeadersFromDisk() for problems this may log
328+ debugs(20, DBG_IMPORTANT, "ERROR: Cannot load a cache hit from shared memory" <<
329+ Debug::Extra << "exception: " << CurrentException <<
330+ Debug::Extra << "cache_mem entry: " << *e);
331+ }
332
333- debugs(20, 3, "failed for " << *e);
334 map->freeEntry(index); // do not let others into the same trap
335 destroyStoreEntry(static_cast<hash_link *>(e));
336 return NULL;
337@@ -473,6 +481,8 @@ MemStore::copyFromShm(StoreEntry &e, const sfileno index, const Ipc::StoreMapAnc
338 Ipc::StoreMapSliceId sid = anchor.start; // optimize: remember the last sid
339 bool wasEof = anchor.complete() && sid < 0;
340 int64_t sliceOffset = 0;
341+
342+ SBuf httpHeaderParsingBuffer;
343 while (sid >= 0) {
344 const Ipc::StoreMapSlice &slice = map->readableSlice(index, sid);
345 // slice state may change during copying; take snapshots now
346@@ -495,10 +505,18 @@ MemStore::copyFromShm(StoreEntry &e, const sfileno index, const Ipc::StoreMapAnc
347 const StoreIOBuffer sliceBuf(wasSize - prefixSize,
348 e.mem_obj->endOffset(),
349 page + prefixSize);
350- if (!copyFromShmSlice(e, sliceBuf, wasEof))
351- return false;
352+
353+ copyFromShmSlice(e, sliceBuf);
354 debugs(20, 8, "entry " << index << " copied slice " << sid <<
355 " from " << extra.page << '+' << prefixSize);
356+
357+ // parse headers if needed; they might span multiple slices!
358+ auto &reply = e.mem().baseReply();
359+ if (reply.pstate != psParsed) {
360+ httpHeaderParsingBuffer.append(sliceBuf.data, sliceBuf.length);
361+ if (reply.parseTerminatedPrefix(httpHeaderParsingBuffer.c_str(), httpHeaderParsingBuffer.length()))
362+ httpHeaderParsingBuffer = SBuf(); // we do not need these bytes anymore
363+ }
364 }
365 // else skip a [possibly incomplete] slice that we copied earlier
366
367@@ -524,6 +542,9 @@ MemStore::copyFromShm(StoreEntry &e, const sfileno index, const Ipc::StoreMapAnc
368 debugs(20, 5, "mem-loaded all " << e.mem_obj->endOffset() << '/' <<
369 anchor.basics.swap_file_sz << " bytes of " << e);
370
371+ if (e.mem().baseReply().pstate != psParsed)
372+ throw TextException(ToSBuf("truncated mem-cached headers; accumulated: ", httpHeaderParsingBuffer.length()), Here());
373+
374 // from StoreEntry::complete()
375 e.mem_obj->object_sz = e.mem_obj->endOffset();
376 e.store_status = STORE_OK;
377@@ -539,32 +560,11 @@ MemStore::copyFromShm(StoreEntry &e, const sfileno index, const Ipc::StoreMapAnc
378 }
379
380 /// imports one shared memory slice into local memory
381-bool
382-MemStore::copyFromShmSlice(StoreEntry &e, const StoreIOBuffer &buf, bool eof)
383+void
384+MemStore::copyFromShmSlice(StoreEntry &e, const StoreIOBuffer &buf)
385 {
386 debugs(20, 7, "buf: " << buf.offset << " + " << buf.length);
387
388- // from store_client::readBody()
389- // parse headers if needed; they might span multiple slices!
390- HttpReply *rep = (HttpReply *)e.getReply();
391- if (rep->pstate < psParsed) {
392- // XXX: have to copy because httpMsgParseStep() requires 0-termination
393- MemBuf mb;
394- mb.init(buf.length+1, buf.length+1);
395- mb.append(buf.data, buf.length);
396- mb.terminate();
397- const int result = rep->httpMsgParseStep(mb.buf, buf.length, eof);
398- if (result > 0) {
399- assert(rep->pstate == psParsed);
400- } else if (result < 0) {
401- debugs(20, DBG_IMPORTANT, "Corrupted mem-cached headers: " << e);
402- return false;
403- } else { // more slices are needed
404- assert(!eof);
405- }
406- }
407- debugs(20, 7, "rep pstate: " << rep->pstate);
408-
409 // local memory stores both headers and body so copy regardless of pstate
410 const int64_t offBefore = e.mem_obj->endOffset();
411 assert(e.mem_obj->data_hdr.write(buf)); // from MemObject::write()
412@@ -572,7 +572,6 @@ MemStore::copyFromShmSlice(StoreEntry &e, const StoreIOBuffer &buf, bool eof)
413 // expect to write the entire buf because StoreEntry::write() never fails
414 assert(offAfter >= 0 && offBefore <= offAfter &&
415 static_cast<size_t>(offAfter - offBefore) == buf.length);
416- return true;
417 }
418
419 /// whether we should cache the entry
420diff --git a/src/MemStore.h b/src/MemStore.h
421index 516da3c..31a2015 100644
422--- a/src/MemStore.h
423+++ b/src/MemStore.h
424@@ -76,7 +76,7 @@ protected:
425 void copyToShm(StoreEntry &e);
426 void copyToShmSlice(StoreEntry &e, Ipc::StoreMapAnchor &anchor, Ipc::StoreMap::Slice &slice);
427 bool copyFromShm(StoreEntry &e, const sfileno index, const Ipc::StoreMapAnchor &anchor);
428- bool copyFromShmSlice(StoreEntry &e, const StoreIOBuffer &buf, bool eof);
429+ void copyFromShmSlice(StoreEntry &, const StoreIOBuffer &);
430
431 void updateHeadersOrThrow(Ipc::StoreMapUpdate &update);
432
433diff --git a/src/SquidMath.h b/src/SquidMath.h
434index c70acd1..bfca0cc 100644
435--- a/src/SquidMath.h
436+++ b/src/SquidMath.h
437@@ -9,6 +9,11 @@
438 #ifndef _SQUID_SRC_SQUIDMATH_H
439 #define _SQUID_SRC_SQUIDMATH_H
440
441+#include <limits>
442+#include <optional>
443+
444+// TODO: Move to src/base/Math.h and drop the Math namespace
445+
446 /* Math functions we define locally for Squid */
447 namespace Math
448 {
449@@ -21,5 +26,165 @@ double doubleAverage(const double, const double, int, const int);
450
451 } // namespace Math
452
453+// If Sum() performance becomes important, consider using GCC and clang
454+// built-ins like __builtin_add_overflow() instead of manual overflow checks.
455+
456+/// detects a pair of unsigned types
457+/// reduces code duplication in declarations further below
458+template <typename T, typename U>
459+using AllUnsigned = typename std::conditional<
460+ std::is_unsigned<T>::value && std::is_unsigned<U>::value,
461+ std::true_type,
462+ std::false_type
463+ >::type;
464+
465+// TODO: Replace with std::cmp_less() after migrating to C++20.
466+/// whether integer a is less than integer b, with correct overflow handling
467+template <typename A, typename B>
468+constexpr bool
469+Less(const A a, const B b) {
470+ // The casts below make standard C++ integer conversions explicit. They
471+ // quell compiler warnings about signed/unsigned comparison. The first two
472+ // lines exclude different-sign a and b, making the casts/comparison safe.
473+ using AB = typename std::common_type<A, B>::type;
474+ return
475+ (a >= 0 && b < 0) ? false :
476+ (a < 0 && b >= 0) ? true :
477+ /* (a >= 0) == (b >= 0) */ static_cast<AB>(a) < static_cast<AB>(b);
478+}
479+
480+/// ensure that T is supported by NaturalSum() and friends
481+template<typename T>
482+constexpr void
483+AssertNaturalType()
484+{
485+ static_assert(std::numeric_limits<T>::is_bounded, "std::numeric_limits<T>::max() is meaningful");
486+ static_assert(std::numeric_limits<T>::is_exact, "no silent loss of precision");
487+ static_assert(!std::is_enum<T>::value, "no silent creation of non-enumerated values");
488+}
489+
490+// TODO: Investigate whether this optimization can be expanded to [signed] types
491+// A and B when std::numeric_limits<decltype(A(0)+B(0))>::is_modulo is true.
492+/// This IncreaseSumInternal() overload is optimized for speed.
493+/// \returns a non-overflowing sum of the two unsigned arguments (or nothing)
494+/// \prec both argument types are unsigned
495+template <typename S, typename A, typename B, std::enable_if_t<AllUnsigned<A,B>::value, int> = 0>
496+std::pair<S, bool>
497+IncreaseSumInternal(const A a, const B b) {
498+ // paranoid: AllUnsigned<A,B> precondition established that already
499+ static_assert(std::is_unsigned<A>::value, "AllUnsigned dispatch worked for A");
500+ static_assert(std::is_unsigned<B>::value, "AllUnsigned dispatch worked for B");
501+
502+ AssertNaturalType<S>();
503+ AssertNaturalType<A>();
504+ AssertNaturalType<B>();
505+
506+ // we should only be called by IncreaseSum(); it forces integer promotion
507+ static_assert(std::is_same<A, decltype(+a)>::value, "a will not be promoted");
508+ static_assert(std::is_same<B, decltype(+b)>::value, "b will not be promoted");
509+ // and without integer promotions, a sum of unsigned integers is unsigned
510+ static_assert(std::is_unsigned<decltype(a+b)>::value, "a+b is unsigned");
511+
512+ // with integer promotions ruled out, a or b can only undergo integer
513+ // conversion to the higher rank type (A or B, we do not know which)
514+ using AB = typename std::common_type<A, B>::type;
515+ static_assert(std::is_same<AB, A>::value || std::is_same<AB, B>::value, "no unexpected conversions");
516+ static_assert(std::is_same<AB, decltype(a+b)>::value, "lossless assignment");
517+ const AB sum = a + b;
518+
519+ static_assert(std::numeric_limits<AB>::is_modulo, "we can detect overflows");
520+ // 1. modulo math: overflowed sum is smaller than any of its operands
521+ // 2. the sum may overflow S (i.e. the return base type)
522+ // We do not need Less() here because we compare promoted unsigned types.
523+ return (sum >= a && sum <= std::numeric_limits<S>::max()) ?
524+ std::make_pair(sum, true) : std::make_pair(S(), false);
525+}
526+
527+/// This IncreaseSumInternal() overload supports a larger variety of types.
528+/// \returns a non-overflowing sum of the two arguments (or nothing)
529+/// \returns nothing if at least one of the arguments is negative
530+/// \prec at least one of the argument types is signed
531+template <typename S, typename A, typename B, std::enable_if_t<!AllUnsigned<A,B>::value, int> = 0>
532+std::pair<S, bool> constexpr
533+IncreaseSumInternal(const A a, const B b) {
534+ AssertNaturalType<S>();
535+ AssertNaturalType<A>();
536+ AssertNaturalType<B>();
537+
538+ // we should only be called by IncreaseSum() that does integer promotion
539+ static_assert(std::is_same<A, decltype(+a)>::value, "a will not be promoted");
540+ static_assert(std::is_same<B, decltype(+b)>::value, "b will not be promoted");
541+
542+ return
543+ // We could support a non-under/overflowing sum of negative numbers, but
544+ // our callers use negative values specially (e.g., for do-not-use or
545+ // do-not-limit settings) and are not supposed to do math with them.
546+ (a < 0 || b < 0) ? std::make_pair(S(), false) :
547+ // To avoid undefined behavior of signed overflow, we must not compute
548+ // the raw a+b sum if it may overflow. When A is not B, a or b undergoes
549+ // (safe for non-negatives) integer conversion in these expressions, so
550+ // we do not know the resulting a+b type AB and its maximum. We must
551+ // also detect subsequent casting-to-S overflows.
552+ // Overflow condition: (a + b > maxAB) or (a + b > maxS).
553+ // A is an integer promotion of S, so maxS <= maxA <= maxAB.
554+ // Since maxS <= maxAB, it is sufficient to just check: a + b > maxS,
555+ // which is the same as the overflow-safe condition here: maxS - a < b.
556+ // Finally, (maxS - a) cannot overflow because a is not negative and
557+ // cannot underflow because a is a promotion of s: 0 <= a <= maxS.
558+ Less(std::numeric_limits<S>::max() - a, b) ? std::make_pair(S(), false) :
559+ std::make_pair(S(a + b), true);
560+}
561+
562+/// argument pack expansion termination for IncreaseSum<S, T, Args...>()
563+template <typename S, typename T>
564+std::pair<S, bool>
565+IncreaseSum(const S s, const T t)
566+{
567+ // Force (always safe) integer promotions now, to give std::enable_if_t<>
568+ // promoted types instead of entering IncreaseSumInternal<AllUnsigned>(s,t)
569+ // but getting a _signed_ promoted value of s or t in s + t.
570+ return IncreaseSumInternal<S>(+s, +t);
571+}
572+
573+/// \returns a non-overflowing sum of the arguments (or nothing)
574+template <typename S, typename T, typename... Args>
575+std::pair<S, bool>
576+IncreaseSum(const S sum, const T t, const Args... args) {
577+ const auto head = IncreaseSum(sum, t);
578+ if (head.second) {
579+ return IncreaseSum(head.first, args...);
580+ } else {
581+ // std::optional<S>() triggers bogus -Wmaybe-uninitialized warnings in GCC v10.3
582+ return std::make_pair(S(), false);
583+ }
584+}
585+
586+/// \returns an exact, non-overflowing sum of the arguments (or nothing)
587+template <typename SummationType, typename... Args>
588+std::pair<SummationType, bool>
589+NaturalSum(const Args... args) {
590+ return IncreaseSum<SummationType>(0, args...);
591+}
592+
593+/// Safely resets the given variable to NaturalSum() of the given arguments.
594+/// If the sum overflows, resets to variable's maximum possible value.
595+/// \returns the new variable value (like an assignment operator would)
596+template <typename S, typename... Args>
597+S
598+SetToNaturalSumOrMax(S &var, const Args... args)
599+{
600+ var = NaturalSum<S>(args...).value_or(std::numeric_limits<S>::max());
601+ return var;
602+}
603+
604+/// converts a given non-negative integer into an integer of a given type
605+/// without loss of information or undefined behavior
606+template <typename Result, typename Source>
607+Result
608+NaturalCast(const Source s)
609+{
610+ return NaturalSum<Result>(s).value();
611+}
612+
613 #endif /* _SQUID_SRC_SQUIDMATH_H */
614
615diff --git a/src/Store.h b/src/Store.h
616index 3eb6b84..2475fe0 100644
617--- a/src/Store.h
618+++ b/src/Store.h
619@@ -49,6 +49,9 @@ public:
620 StoreEntry();
621 virtual ~StoreEntry();
622
623+ MemObject &mem() { assert(mem_obj); return *mem_obj; }
624+ const MemObject &mem() const { assert(mem_obj); return *mem_obj; }
625+
626 virtual HttpReply const *getReply() const;
627 virtual void write (StoreIOBuffer);
628
629diff --git a/src/StoreClient.h b/src/StoreClient.h
630index 65472d8..942f9fc 100644
631--- a/src/StoreClient.h
632+++ b/src/StoreClient.h
633@@ -9,11 +9,13 @@
634 #ifndef SQUID_STORECLIENT_H
635 #define SQUID_STORECLIENT_H
636
637+#include "base/AsyncCall.h"
638 #include "dlink.h"
639+#include "store/ParsingBuffer.h"
640 #include "StoreIOBuffer.h"
641 #include "StoreIOState.h"
642
643-typedef void STCB(void *, StoreIOBuffer); /* store callback */
644+using STCB = void (void *, StoreIOBuffer); /* store callback */
645
646 class StoreEntry;
647
648@@ -39,17 +41,34 @@ class store_client
649 public:
650 store_client(StoreEntry *);
651 ~store_client();
652- bool memReaderHasLowerOffset(int64_t) const;
653+
654+ /// the client will not use HTTP response bytes with lower offsets (if any)
655+ auto discardableHttpEnd() const { return discardableHttpEnd_; }
656+
657 int getType() const;
658- void fail();
659- void callback(ssize_t len, bool error = false);
660+
661+ /// React to the end of reading the response from disk. There will be no
662+ /// more readHeader() and readBody() callbacks for the current storeRead()
663+ /// swapin after this notification.
664+ void noteSwapInDone(bool error);
665+
666 void doCopy (StoreEntry *e);
667 void readHeader(const char *buf, ssize_t len);
668 void readBody(const char *buf, ssize_t len);
669+
670+ /// Request StoreIOBuffer-described response data via an asynchronous STCB
671+ /// callback. At most one outstanding request is allowed per store_client.
672 void copy(StoreEntry *, StoreIOBuffer, STCB *, void *);
673+
674 void dumpStats(MemBuf * output, int clientNumber) const;
675
676- int64_t cmp_offset;
677+ // TODO: When STCB gets a dedicated Answer type, move this info there.
678+ /// Whether the last successful storeClientCopy() answer was known to
679+ /// contain the last body bytes of the HTTP response
680+ /// \retval true requesting bytes at higher offsets is futile
681+ /// \sa STCB
682+ bool atEof() const { return atEof_; }
683+
684 #if STORE_CLIENT_LIST_DEBUG
685
686 void *owner;
687@@ -59,33 +78,86 @@ public:
688 StoreIOState::Pointer swapin_sio;
689
690 struct {
691+ /// whether we are expecting a response to be swapped in from disk
692+ /// (i.e. whether async storeRead() is currently in progress)
693+ // TODO: a better name reflecting the 'in' scope of the flag
694 bool disk_io_pending;
695+
696+ /// whether the store_client::doCopy()-initiated STCB sequence is
697+ /// currently in progress
698 bool store_copying;
699- bool copy_event_pending;
700 } flags;
701
702 #if USE_DELAY_POOLS
703 DelayId delayId;
704+
705+ /// The maximum number of bytes the Store client can read/copy next without
706+ /// overflowing its buffer and without violating delay pool limits. Store
707+ /// I/O is not rate-limited, but we assume that the same number of bytes may
708+ /// be read from the Squid-to-server connection that may be rate-limited.
709+ int bytesWanted() const;
710+
711 void setDelayId(DelayId delay_id);
712 #endif
713
714 dlink_node node;
715- /* Below here is private - do no alter outside storeClient calls */
716- StoreIOBuffer copyInto;
717
718 private:
719- bool moreToSend() const;
720+ bool moreToRead() const;
721+ bool canReadFromMemory() const;
722+ bool answeredOnce() const { return answers >= 1; }
723+ bool sendingHttpHeaders() const;
724+ int64_t nextHttpReadOffset() const;
725
726 void fileRead();
727 void scheduleDiskRead();
728- void scheduleMemRead();
729+ void readFromMemory();
730 void scheduleRead();
731 bool startSwapin();
732 bool unpackHeader(char const *buf, ssize_t len);
733+ void handleBodyFromDisk();
734+ void maybeWriteFromDiskToMemory(const StoreIOBuffer &);
735+
736+ bool parseHttpHeadersFromDisk();
737+ bool tryParsingHttpHeaders();
738+ void skipHttpHeadersFromDisk();
739+
740+ void fail();
741+ void callback(ssize_t);
742+ void noteCopiedBytes(size_t);
743+ void noteNews();
744+ void finishCallback();
745+ static void FinishCallback(store_client *);
746
747 int type;
748 bool object_ok;
749
750+ /// \copydoc atEof()
751+ bool atEof_;
752+
753+ /// Storage and metadata associated with the current copy() request. Ought
754+ /// to be ignored when not answering a copy() request.
755+ /// * copyInto.offset is the requested HTTP response body offset;
756+ /// * copyInto.data is the client-owned, client-provided result buffer;
757+ /// * copyInto.length is the size of the .data result buffer;
758+ /// * copyInto.flags are unused by this class.
759+ StoreIOBuffer copyInto;
760+
761+ // TODO: Convert to uint64_t after fixing mem_hdr::endOffset() and friends.
762+ /// \copydoc discardableHttpEnd()
763+ int64_t discardableHttpEnd_ = 0;
764+
765+ /// the total number of finishCallback() calls
766+ uint64_t answers;
767+
768+ /// Accumulates raw bytes read from Store while answering the current copy()
769+ /// request. Buffer contents depends on the source and parsing stage; it may
770+ /// hold (parts of) swap metadata, HTTP response headers, and/or HTTP
771+ /// response body bytes.
772+ std::pair<Store::ParsingBuffer, bool> parsingBuffer = std::make_pair(Store::ParsingBuffer(), false);
773+
774+ StoreIOBuffer lastDiskRead; ///< buffer used for the last storeRead() call
775+
776 /* Until we finish stuffing code into store_client */
777
778 public:
779@@ -97,6 +169,7 @@ public:
780 bool pending() const;
781 STCB *callback_handler;
782 void *callback_data;
783+ AsyncCall::Pointer notifier;
784 } _callback;
785 };
786
787diff --git a/src/StoreIOBuffer.h b/src/StoreIOBuffer.h
788index 009aafe..ad1c491 100644
789--- a/src/StoreIOBuffer.h
790+++ b/src/StoreIOBuffer.h
791@@ -43,6 +43,9 @@ public:
792 return Range<int64_t>(offset, offset + length);
793 }
794
795+ /// convenience method for changing the offset of a being-configured buffer
796+ StoreIOBuffer &positionAt(const int64_t newOffset) { offset = newOffset; return *this; }
797+
798 void dump() const {
799 if (fwrite(data, length, 1, stderr)) {}
800 if (fwrite("\n", 1, 1, stderr)) {}
801diff --git a/src/acl/Asn.cc b/src/acl/Asn.cc
802index 94ec862..07353d6 100644
803--- a/src/acl/Asn.cc
804+++ b/src/acl/Asn.cc
805@@ -16,20 +16,22 @@
806 #include "acl/DestinationIp.h"
807 #include "acl/SourceAsn.h"
808 #include "acl/Strategised.h"
809+#include "base/CharacterSet.h"
810 #include "FwdState.h"
811 #include "HttpReply.h"
812 #include "HttpRequest.h"
813 #include "ipcache.h"
814 #include "MasterXaction.h"
815 #include "mgr/Registration.h"
816+#include "parser/Tokenizer.h"
817 #include "radix.h"
818 #include "RequestFlags.h"
819+#include "sbuf/SBuf.h"
820 #include "SquidConfig.h"
821 #include "Store.h"
822 #include "StoreClient.h"
823
824 #define WHOIS_PORT 43
825-#define AS_REQBUF_SZ 4096
826
827 /* BEGIN of definitions for radix tree entries */
828
829@@ -70,33 +72,18 @@ class ASState
830 CBDATA_CLASS(ASState);
831
832 public:
833- ASState();
834+ ASState() = default;
835 ~ASState();
836
837 StoreEntry *entry;
838 store_client *sc;
839 HttpRequest::Pointer request;
840 int as_number;
841- int64_t offset;
842- int reqofs;
843- char reqbuf[AS_REQBUF_SZ];
844- bool dataRead;
845+ Store::ParsingBuffer parsingBuffer;
846 };
847
848 CBDATA_CLASS_INIT(ASState);
849
850-ASState::ASState() :
851- entry(NULL),
852- sc(NULL),
853- request(NULL),
854- as_number(0),
855- offset(0),
856- reqofs(0),
857- dataRead(false)
858-{
859- memset(reqbuf, 0, AS_REQBUF_SZ);
860-}
861-
862 ASState::~ASState()
863 {
864 debugs(53, 3, entry->url());
865@@ -112,7 +99,7 @@ struct rtentry_t {
866 m_ADDR e_mask;
867 };
868
869-static int asnAddNet(char *, int);
870+static int asnAddNet(const SBuf &, int);
871
872 static void asnCacheStart(int as);
873
874@@ -256,8 +243,7 @@ asnCacheStart(int as)
875 }
876
877 asState->entry = e;
878- StoreIOBuffer readBuffer (AS_REQBUF_SZ, asState->offset, asState->reqbuf);
879- storeClientCopy(asState->sc, e, readBuffer, asHandleReply, asState);
880+ storeClientCopy(asState->sc, e, asState->parsingBuffer.makeInitialSpace(), asHandleReply, asState);
881 }
882
883 static void
884@@ -265,13 +251,8 @@ asHandleReply(void *data, StoreIOBuffer result)
885 {
886 ASState *asState = (ASState *)data;
887 StoreEntry *e = asState->entry;
888- char *s;
889- char *t;
890- char *buf = asState->reqbuf;
891- int leftoversz = -1;
892
893- debugs(53, 3, "asHandleReply: Called with size=" << (unsigned int)result.length);
894- debugs(53, 3, "asHandleReply: buffer='" << buf << "'");
895+ debugs(53, 3, result << " for " << asState->as_number << " with " << *e);
896
897 /* First figure out whether we should abort the request */
898
899@@ -280,11 +261,7 @@ asHandleReply(void *data, StoreIOBuffer result)
900 return;
901 }
902
903- if (result.length == 0 && asState->dataRead) {
904- debugs(53, 3, "asHandleReply: Done: " << e->url());
905- delete asState;
906- return;
907- } else if (result.flags.error) {
908+ if (result.flags.error) {
909 debugs(53, DBG_IMPORTANT, "asHandleReply: Called with Error set and size=" << (unsigned int) result.length);
910 delete asState;
911 return;
912@@ -294,117 +271,85 @@ asHandleReply(void *data, StoreIOBuffer result)
913 return;
914 }
915
916- /*
917- * Next, attempt to parse our request
918- * Remembering that the actual buffer size is retsize + reqofs!
919- */
920- s = buf;
921+ asState->parsingBuffer.appended(result.data, result.length);
922+ Parser::Tokenizer tok(SBuf(asState->parsingBuffer.content().data, asState->parsingBuffer.contentSize()));
923+ SBuf address;
924+ // Word delimiters in WHOIS ASN replies. RFC 3912 mentions SP, CR, and LF.
925+ // Others are added to mimic an earlier isspace()-based implementation.
926+ static const auto WhoisSpaces = CharacterSet("ASCII_spaces", " \f\r\n\t\v");
927+ while (tok.token(address, WhoisSpaces)) {
928+ (void)asnAddNet(address, asState->as_number);
929+ }
930+ asState->parsingBuffer.consume(tok.parsedSize());
931+ const auto leftoverBytes = asState->parsingBuffer.contentSize();
932
933- while ((size_t)(s - buf) < result.length + asState->reqofs && *s != '\0') {
934- while (*s && xisspace(*s))
935- ++s;
936+ if (asState->sc->atEof()) {
937+ if (leftoverBytes)
938+ debugs(53, 2, "WHOIS: Discarding the last " << leftoverBytes << " received bytes of a truncated AS response");
939+ delete asState;
940+ return;
941+ }
942
943- for (t = s; *t; ++t) {
944- if (xisspace(*t))
945- break;
946- }
947+ if (asState->sc->atEof()) {
948+ if (leftoverBytes)
949+ debugs(53, 2, "WHOIS: Discarding the last " << leftoverBytes << " received bytes of a truncated AS response");
950+ delete asState;
951+ return;
952+ }
953
954- if (*t == '\0') {
955- /* oof, word should continue on next block */
956- break;
957- }
958+ const auto remainingSpace = asState->parsingBuffer.space().positionAt(result.offset + result.length);
959
960- *t = '\0';
961- debugs(53, 3, "asHandleReply: AS# " << s << " (" << asState->as_number << ")");
962- asnAddNet(s, asState->as_number);
963- s = t + 1;
964- asState->dataRead = true;
965+ if (!remainingSpace.length) {
966+ Assure(leftoverBytes);
967+ debugs(53, DBG_IMPORTANT, "WARNING: Ignoring the tail of a WHOIS AS response" <<
968+ " with an unparsable section of " << leftoverBytes <<
969+ " bytes ending at offset " << remainingSpace.offset);
970+ delete asState;
971+ return;
972 }
973
974- /*
975- * Next, grab the end of the 'valid data' in the buffer, and figure
976- * out how much data is left in our buffer, which we need to keep
977- * around for the next request
978- */
979- leftoversz = (asState->reqofs + result.length) - (s - buf);
980-
981- assert(leftoversz >= 0);
982-
983- /*
984- * Next, copy the left over data, from s to s + leftoversz to the
985- * beginning of the buffer
986- */
987- memmove(buf, s, leftoversz);
988-
989- /*
990- * Next, update our offset and reqofs, and kick off a copy if required
991- */
992- asState->offset += result.length;
993-
994- asState->reqofs = leftoversz;
995-
996- debugs(53, 3, "asState->offset = " << asState->offset);
997-
998- if (e->store_status == STORE_PENDING) {
999- debugs(53, 3, "asHandleReply: store_status == STORE_PENDING: " << e->url() );
1000- StoreIOBuffer tempBuffer (AS_REQBUF_SZ - asState->reqofs,
1001- asState->offset,
1002- asState->reqbuf + asState->reqofs);
1003- storeClientCopy(asState->sc,
1004- e,
1005- tempBuffer,
1006- asHandleReply,
1007- asState);
1008- } else {
1009- StoreIOBuffer tempBuffer;
1010- debugs(53, 3, "asHandleReply: store complete, but data received " << e->url() );
1011- tempBuffer.offset = asState->offset;
1012- tempBuffer.length = AS_REQBUF_SZ - asState->reqofs;
1013- tempBuffer.data = asState->reqbuf + asState->reqofs;
1014- storeClientCopy(asState->sc,
1015- e,
1016- tempBuffer,
1017- asHandleReply,
1018- asState);
1019- }
1020+ const decltype(StoreIOBuffer::offset) stillReasonableOffset = 100000; // an arbitrary limit in bytes
1021+ if (remainingSpace.offset > stillReasonableOffset) {
1022+ // stop suspicious accumulation of parsed addresses and/or work
1023+ debugs(53, DBG_IMPORTANT, "WARNING: Ignoring the tail of a suspiciously large WHOIS AS response" <<
1024+ " exceeding " << stillReasonableOffset << " bytes");
1025+ delete asState;
1026+ return;
1027+ }
1028+
1029+ storeClientCopy(asState->sc, e, remainingSpace, asHandleReply, asState);
1030 }
1031
1032 /**
1033 * add a network (addr, mask) to the radix tree, with matching AS number
1034 */
1035 static int
1036-asnAddNet(char *as_string, int as_number)
1037+asnAddNet(const SBuf &addressAndMask, const int as_number)
1038 {
1039 struct squid_radix_node *rn;
1040 CbDataList<int> **Tail = NULL;
1041 CbDataList<int> *q = NULL;
1042 as_info *asinfo = NULL;
1043
1044- Ip::Address mask;
1045- Ip::Address addr;
1046- char *t;
1047- int bitl;
1048-
1049- t = strchr(as_string, '/');
1050-
1051- if (t == NULL) {
1052+ static const CharacterSet NonSlashSet = CharacterSet("slash", "/").complement("non-slash");
1053+ Parser::Tokenizer tok(addressAndMask);
1054+ SBuf addressToken;
1055+ if (!(tok.prefix(addressToken, NonSlashSet) && tok.skip('/'))) {
1056 debugs(53, 3, "asnAddNet: failed, invalid response from whois server.");
1057 return 0;
1058 }
1059
1060- *t = '\0';
1061- addr = as_string;
1062- bitl = atoi(t + 1);
1063-
1064- if (bitl < 0)
1065- bitl = 0;
1066+ const Ip::Address addr = addressToken.c_str();
1067
1068 // INET6 TODO : find a better way of identifying the base IPA family for mask than this.
1069- t = strchr(as_string, '.');
1070+ const auto addrFamily = (addressToken.find('.') != SBuf::npos) ? AF_INET : AF_INET6;
1071
1072 // generate Netbits Format Mask
1073+ Ip::Address mask;
1074 mask.setNoAddr();
1075- mask.applyMask(bitl, (t!=NULL?AF_INET:AF_INET6) );
1076+ int64_t bitl = 0;
1077+ if (tok.int64(bitl, 10, false))
1078+ mask.applyMask(bitl, addrFamily);
1079
1080 debugs(53, 3, "asnAddNet: called for " << addr << "/" << mask );
1081
1082diff --git a/src/acl/FilledChecklist.cc b/src/acl/FilledChecklist.cc
1083index 9826c24..33eeb67 100644
1084--- a/src/acl/FilledChecklist.cc
1085+++ b/src/acl/FilledChecklist.cc
1086@@ -116,7 +116,6 @@ ACLFilledChecklist::verifyAle() const
1087 if (reply && !al->reply) {
1088 showDebugWarning("HttpReply object");
1089 al->reply = reply;
1090- HTTPMSGLOCK(al->reply);
1091 }
1092
1093 #if USE_IDENT
1094diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc
1095index 370f077..2bcc917 100644
1096--- a/src/adaptation/icap/ModXact.cc
1097+++ b/src/adaptation/icap/ModXact.cc
1098@@ -1292,11 +1292,8 @@ void Adaptation::Icap::ModXact::finalizeLogInfo()
1099 al.adapted_request = adapted_request_;
1100 HTTPMSGLOCK(al.adapted_request);
1101
1102- if (adapted_reply_) {
1103- al.reply = adapted_reply_;
1104- HTTPMSGLOCK(al.reply);
1105- } else
1106- al.reply = NULL;
1107+ // XXX: This reply (and other ALE members!) may have been needed earlier.
1108+ al.reply = adapted_reply_;
1109
1110 if (h->rfc931.size())
1111 al.cache.rfc931 = h->rfc931.termedBuf();
1112@@ -1331,12 +1328,6 @@ void Adaptation::Icap::ModXact::finalizeLogInfo()
1113 if (replyHttpBodySize >= 0)
1114 al.cache.highOffset = replyHttpBodySize;
1115 //don't set al.cache.objectSize because it hasn't exist yet
1116-
1117- MemBuf mb;
1118- mb.init();
1119- adapted_reply_->header.packInto(&mb);
1120- al.headers.reply = xstrdup(mb.buf);
1121- mb.clean();
1122 }
1123 prepareLogWithRequestDetails(adapted_request_, alep);
1124 Xaction::finalizeLogInfo();
1125diff --git a/src/adaptation/icap/icap_log.cc b/src/adaptation/icap/icap_log.cc
1126index ecc4baf..6bb5a6d 100644
1127--- a/src/adaptation/icap/icap_log.cc
1128+++ b/src/adaptation/icap/icap_log.cc
1129@@ -62,7 +62,7 @@ void icapLogLog(AccessLogEntry::Pointer &al)
1130 if (IcapLogfileStatus == LOG_ENABLE) {
1131 ACLFilledChecklist checklist(NULL, al->adapted_request, NULL);
1132 if (al->reply) {
1133- checklist.reply = al->reply;
1134+ checklist.reply = al->reply.getRaw();
1135 HTTPMSGLOCK(checklist.reply);
1136 }
1137 accessLogLogTo(Config.Log.icaplogs, al, &checklist);
1138diff --git a/src/base/Assure.cc b/src/base/Assure.cc
1139new file mode 100644
1140index 0000000..cb69fc5
1141--- /dev/null
1142+++ b/src/base/Assure.cc
1143@@ -0,0 +1,24 @@
1144+/*
1145+ * Copyright (C) 1996-2022 The Squid Software Foundation and contributors
1146+ *
1147+ * Squid software is distributed under GPLv2+ license and includes
1148+ * contributions from numerous individuals and organizations.
1149+ * Please see the COPYING and CONTRIBUTORS files for details.
1150+ */
1151+
1152+#include "squid.h"
1153+#include "base/Assure.h"
1154+#include "base/TextException.h"
1155+#include "sbuf/Stream.h"
1156+
1157+[[ noreturn ]] void
1158+ReportAndThrow_(const int debugLevel, const char *description, const SourceLocation &location)
1159+{
1160+ const TextException ex(description, location);
1161+ const auto label = debugLevel <= DBG_IMPORTANT ? "ERROR: Squid BUG: " : "";
1162+ // TODO: Consider also printing the number of BUGs reported so far. It would
1163+ // require GC, but we could even print the number of same-location reports.
1164+ debugs(0, debugLevel, label << ex);
1165+ throw ex;
1166+}
1167+
1168diff --git a/src/base/Assure.h b/src/base/Assure.h
1169new file mode 100644
1170index 0000000..bb571d2
1171--- /dev/null
1172+++ b/src/base/Assure.h
1173@@ -0,0 +1,52 @@
1174+/*
1175+ * Copyright (C) 1996-2022 The Squid Software Foundation and contributors
1176+ *
1177+ * Squid software is distributed under GPLv2+ license and includes
1178+ * contributions from numerous individuals and organizations.
1179+ * Please see the COPYING and CONTRIBUTORS files for details.
1180+ */
1181+
1182+#ifndef SQUID_SRC_BASE_ASSURE_H
1183+#define SQUID_SRC_BASE_ASSURE_H
1184+
1185+#include "base/Here.h"
1186+
1187+/// Reports the description (at the given debugging level) and throws
1188+/// the corresponding exception. Reduces compiled code size of Assure() and
1189+/// Must() callers. Do not call directly; use Assure() instead.
1190+/// \param description explains the condition (i.e. what MUST happen)
1191+[[ noreturn ]] void ReportAndThrow_(int debugLevel, const char *description, const SourceLocation &);
1192+
1193+/// Calls ReportAndThrow() if needed. Reduces caller code duplication.
1194+/// Do not call directly; use Assure() instead.
1195+/// \param description c-string explaining the condition (i.e. what MUST happen)
1196+#define Assure_(debugLevel, condition, description, location) \
1197+ while (!(condition)) \
1198+ ReportAndThrow_((debugLevel), (description), (location))
1199+
1200+#if !defined(NDEBUG)
1201+
1202+/// Like assert() but throws an exception instead of aborting the process. Use
1203+/// this macro to detect code logic mistakes (i.e. bugs) where aborting the
1204+/// current AsyncJob or a similar task is unlikely to jeopardize Squid service
1205+/// integrity. For example, this macro is _not_ appropriate for detecting bugs
1206+/// that indicate a dangerous global state corruption which may go unnoticed by
1207+/// other jobs after the current job or task is aborted.
1208+#define Assure(condition) \
1209+ Assure2((condition), #condition)
1210+
1211+/// Like Assure() but allows the caller to customize the exception message.
1212+/// \param description string literal describing the condition (i.e. what MUST happen)
1213+#define Assure2(condition, description) \
1214+ Assure_(0, (condition), ("assurance failed: " description), Here())
1215+
1216+#else
1217+
1218+/* do-nothing implementations for NDEBUG builds */
1219+#define Assure(condition) ((void)0)
1220+#define Assure2(condition, description) ((void)0)
1221+
1222+#endif /* NDEBUG */
1223+
1224+#endif /* SQUID_SRC_BASE_ASSURE_H */
1225+
1226diff --git a/src/base/Makefile.am b/src/base/Makefile.am
1227index 9b0f4cf..d5f4c01 100644
1228--- a/src/base/Makefile.am
1229+++ b/src/base/Makefile.am
1230@@ -11,6 +11,8 @@ include $(top_srcdir)/src/TestHeaders.am
1231 noinst_LTLIBRARIES = libbase.la
1232
1233 libbase_la_SOURCES = \
1234+ Assure.cc \
1235+ Assure.h \
1236 AsyncCall.cc \
1237 AsyncCall.h \
1238 AsyncCallQueue.cc \
1239diff --git a/src/base/Makefile.in b/src/base/Makefile.in
1240index 90a4f5b..6a83aa4 100644
1241--- a/src/base/Makefile.in
1242+++ b/src/base/Makefile.in
1243@@ -163,7 +163,7 @@ CONFIG_CLEAN_FILES =
1244 CONFIG_CLEAN_VPATH_FILES =
1245 LTLIBRARIES = $(noinst_LTLIBRARIES)
1246 libbase_la_LIBADD =
1247-am_libbase_la_OBJECTS = AsyncCall.lo AsyncCallQueue.lo AsyncJob.lo \
1248+am_libbase_la_OBJECTS = Assure.lo AsyncCall.lo AsyncCallQueue.lo AsyncJob.lo \
1249 CharacterSet.lo File.lo Here.lo RegexPattern.lo \
1250 RunnersRegistry.lo TextException.lo
1251 libbase_la_OBJECTS = $(am_libbase_la_OBJECTS)
1252@@ -186,7 +186,7 @@ am__v_at_1 =
1253 DEFAULT_INCLUDES =
1254 depcomp = $(SHELL) $(top_srcdir)/cfgaux/depcomp
1255 am__maybe_remake_depfiles = depfiles
1256-am__depfiles_remade = ./$(DEPDIR)/AsyncCall.Plo \
1257+am__depfiles_remade = ./$(DEPDIR)/Assure.Plo ./$(DEPDIR)/AsyncCall.Plo \
1258 ./$(DEPDIR)/AsyncCallQueue.Plo ./$(DEPDIR)/AsyncJob.Plo \
1259 ./$(DEPDIR)/CharacterSet.Plo ./$(DEPDIR)/File.Plo \
1260 ./$(DEPDIR)/Here.Plo ./$(DEPDIR)/RegexPattern.Plo \
1261@@ -729,6 +729,8 @@ COMPAT_LIB = $(top_builddir)/compat/libcompatsquid.la $(LIBPROFILER)
1262 subst_perlshell = sed -e 's,[@]PERL[@],$(PERL),g' <$(srcdir)/$@.pl.in >$@ || ($(RM) -f $@ ; exit 1)
1263 noinst_LTLIBRARIES = libbase.la
1264 libbase_la_SOURCES = \
1265+ Assure.cc \
1266+ Assure.h \
1267 AsyncCall.cc \
1268 AsyncCall.h \
1269 AsyncCallQueue.cc \
1270@@ -827,6 +829,7 @@ mostlyclean-compile:
1271 distclean-compile:
1272 -rm -f *.tab.c
1273
1274+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/Assure.Plo@am__quote@ # am--include-marker
1275 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/AsyncCall.Plo@am__quote@ # am--include-marker
1276 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/AsyncCallQueue.Plo@am__quote@ # am--include-marker
1277 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/AsyncJob.Plo@am__quote@ # am--include-marker
1278@@ -1167,7 +1170,8 @@ clean-am: clean-checkPROGRAMS clean-generic clean-libtool \
1279 clean-noinstLTLIBRARIES mostlyclean-am
1280
1281 distclean: distclean-am
1282- -rm -f ./$(DEPDIR)/AsyncCall.Plo
1283+ -rm -f ./$(DEPDIR)/Assure.Plo
1284+ -rm -f ./$(DEPDIR)/AsyncCall.Plo
1285 -rm -f ./$(DEPDIR)/AsyncCallQueue.Plo
1286 -rm -f ./$(DEPDIR)/AsyncJob.Plo
1287 -rm -f ./$(DEPDIR)/CharacterSet.Plo
1288@@ -1221,7 +1225,8 @@ install-ps-am:
1289 installcheck-am:
1290
1291 maintainer-clean: maintainer-clean-am
1292- -rm -f ./$(DEPDIR)/AsyncCall.Plo
1293+ -rm -f ./$(DEPDIR)/Assure.Plo
1294+ -rm -f ./$(DEPDIR)/AsyncCall.Plo
1295 -rm -f ./$(DEPDIR)/AsyncCallQueue.Plo
1296 -rm -f ./$(DEPDIR)/AsyncJob.Plo
1297 -rm -f ./$(DEPDIR)/CharacterSet.Plo
1298diff --git a/src/base/TextException.cc b/src/base/TextException.cc
1299index 5cfeb26..f895ae9 100644
1300--- a/src/base/TextException.cc
1301+++ b/src/base/TextException.cc
1302@@ -58,6 +58,13 @@ TextException::what() const throw()
1303 return result.what();
1304 }
1305
1306+std::ostream &
1307+operator <<(std::ostream &os, const TextException &ex)
1308+{
1309+ ex.print(os);
1310+ return os;
1311+}
1312+
1313 std::ostream &
1314 CurrentException(std::ostream &os)
1315 {
1316diff --git a/src/base/TextException.h b/src/base/TextException.h
1317index 6a79536..1f9ca11 100644
1318--- a/src/base/TextException.h
1319+++ b/src/base/TextException.h
1320@@ -9,6 +9,7 @@
1321 #ifndef SQUID__TEXTEXCEPTION_H
1322 #define SQUID__TEXTEXCEPTION_H
1323
1324+#include "base/Assure.h"
1325 #include "base/Here.h"
1326
1327 #include <stdexcept>
1328@@ -51,11 +52,12 @@ public:
1329 /// prints active (i.e., thrown but not yet handled) exception
1330 std::ostream &CurrentException(std::ostream &);
1331
1332+/// efficiently prints TextException
1333+std::ostream &operator <<(std::ostream &, const TextException &);
1334+
1335 /// legacy convenience macro; it is not difficult to type Here() now
1336 #define TexcHere(msg) TextException((msg), Here())
1337
1338-/// Like assert() but throws an exception instead of aborting the process
1339-/// and allows the caller to specify a custom exception message.
1340 #define Must2(condition, message) \
1341 do { \
1342 if (!(condition)) { \
1343@@ -65,8 +67,13 @@ std::ostream &CurrentException(std::ostream &);
1344 } \
1345 } while (/*CONSTCOND*/ false)
1346
1347+/// Like assert() but throws an exception instead of aborting the process
1348+/// and allows the caller to specify a custom exception message.
1349+#define Must3(condition, description, location) \
1350+ Assure_(3, (condition), ("check failed: " description), (location))
1351+
1352 /// Like assert() but throws an exception instead of aborting the process.
1353-#define Must(condition) Must2((condition), "check failed: " #condition)
1354+#define Must(condition) Must3((condition), #condition, Here())
1355
1356 /// Reports and swallows all exceptions to prevent compiler warnings and runtime
1357 /// errors related to throwing class destructors. Should be used for most dtors.
1358diff --git a/src/clientStream.cc b/src/clientStream.cc
1359index 04d89c0..bd5dd09 100644
1360--- a/src/clientStream.cc
1361+++ b/src/clientStream.cc
1362@@ -154,8 +154,7 @@ clientStreamCallback(clientStreamNode * thisObject, ClientHttpRequest * http,
1363 assert(thisObject && http && thisObject->node.next);
1364 next = thisObject->next();
1365
1366- debugs(87, 3, "clientStreamCallback: Calling " << next->callback << " with cbdata " <<
1367- next->data.getRaw() << " from node " << thisObject);
1368+ debugs(87, 3, thisObject << " gives " << next->data << ' ' << replyBuffer);
1369 next->callback(next, http, rep, replyBuffer);
1370 }
1371
1372diff --git a/src/client_side.cc b/src/client_side.cc
1373index ab393e4..c46a845 100644
1374--- a/src/client_side.cc
1375+++ b/src/client_side.cc
1376@@ -429,7 +429,7 @@ ClientHttpRequest::logRequest()
1377 // The al->notes and request->notes must point to the same object.
1378 (void)SyncNotes(*al, *request);
1379 for (auto i = Config.notes.begin(); i != Config.notes.end(); ++i) {
1380- if (const char *value = (*i)->match(request, al->reply, al)) {
1381+ if (const char *value = (*i)->match(request, al->reply.getRaw(), al)) {
1382 NotePairs &notes = SyncNotes(*al, *request);
1383 notes.add((*i)->key.termedBuf(), value);
1384 debugs(33, 3, (*i)->key.termedBuf() << " " << value);
1385@@ -439,7 +439,7 @@ ClientHttpRequest::logRequest()
1386
1387 ACLFilledChecklist checklist(NULL, request, NULL);
1388 if (al->reply) {
1389- checklist.reply = al->reply;
1390+ checklist.reply = al->reply.getRaw();
1391 HTTPMSGLOCK(checklist.reply);
1392 }
1393
1394@@ -457,7 +457,7 @@ ClientHttpRequest::logRequest()
1395 ACLFilledChecklist statsCheck(Config.accessList.stats_collection, request, NULL);
1396 statsCheck.al = al;
1397 if (al->reply) {
1398- statsCheck.reply = al->reply;
1399+ statsCheck.reply = al->reply.getRaw();
1400 HTTPMSGLOCK(statsCheck.reply);
1401 }
1402 updatePerformanceCounters = statsCheck.fastCheck().allowed();
1403@@ -3844,6 +3844,11 @@ ConnStateData::finishDechunkingRequest(bool withSuccess)
1404 void
1405 ConnStateData::sendControlMsg(HttpControlMsg msg)
1406 {
1407+ if (const auto context = pipeline.front()) {
1408+ if (context->http)
1409+ context->http->al->reply = msg.reply;
1410+ }
1411+
1412 if (!isOpen()) {
1413 debugs(33, 3, HERE << "ignoring 1xx due to earlier closure");
1414 return;
1415diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
1416index c919af4..fea5ecb 100644
1417--- a/src/client_side_reply.cc
1418+++ b/src/client_side_reply.cc
1419@@ -34,6 +34,7 @@
1420 #include "RequestFlags.h"
1421 #include "SquidConfig.h"
1422 #include "SquidTime.h"
1423+#include "SquidMath.h"
1424 #include "Store.h"
1425 #include "StrList.h"
1426 #include "tools.h"
1427@@ -76,11 +77,7 @@ clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) :
1428 purgeStatus(Http::scNone),
1429 lookingforstore(0),
1430 http(cbdataReference(clientContext)),
1431- headers_sz(0),
1432 sc(NULL),
1433- old_reqsize(0),
1434- reqsize(0),
1435- reqofs(0),
1436 #if USE_CACHE_DIGESTS
1437 lookup_type(NULL),
1438 #endif
1439@@ -166,8 +163,6 @@ void clientReplyContext::setReplyToStoreEntry(StoreEntry *entry, const char *rea
1440 #if USE_DELAY_POOLS
1441 sc->setDelayId(DelayId::DelayClient(http));
1442 #endif
1443- reqofs = 0;
1444- reqsize = 0;
1445 if (http->request)
1446 http->request->ignoreRange(reason);
1447 flags.storelogiccomplete = 1;
1448@@ -206,13 +201,10 @@ clientReplyContext::saveState()
1449 old_sc = sc;
1450 old_lastmod = http->request->lastmod;
1451 old_etag = http->request->etag;
1452- old_reqsize = reqsize;
1453- tempBuffer.offset = reqofs;
1454+
1455 /* Prevent accessing the now saved entries */
1456 http->storeEntry(NULL);
1457 sc = NULL;
1458- reqsize = 0;
1459- reqofs = 0;
1460 }
1461
1462 void
1463@@ -223,8 +215,6 @@ clientReplyContext::restoreState()
1464 removeClientStoreReference(&sc, http);
1465 http->storeEntry(old_entry);
1466 sc = old_sc;
1467- reqsize = old_reqsize;
1468- reqofs = tempBuffer.offset;
1469 http->request->lastmod = old_lastmod;
1470 http->request->etag = old_etag;
1471 /* Prevent accessed the old saved entries */
1472@@ -232,7 +222,7 @@ clientReplyContext::restoreState()
1473 old_sc = NULL;
1474 old_lastmod = -1;
1475 old_etag.clean();
1476- old_reqsize = 0;
1477+
1478 tempBuffer.offset = 0;
1479 }
1480
1481@@ -250,18 +240,27 @@ clientReplyContext::getNextNode() const
1482 return (clientStreamNode *)ourNode->node.next->data;
1483 }
1484
1485-/* This function is wrong - the client parameters don't include the
1486- * header offset
1487- */
1488+/// Request HTTP response headers from Store, to be sent to the given recipient.
1489+/// That recipient also gets zero, some, or all HTTP response body bytes (into
1490+/// next()->readBuffer).
1491 void
1492-clientReplyContext::triggerInitialStoreRead()
1493+clientReplyContext::triggerInitialStoreRead(STCB recipient)
1494 {
1495- /* when confident, 0 becomes reqofs, and then this factors into
1496- * startSendProcess
1497- */
1498- assert(reqofs == 0);
1499+ Assure(recipient != HandleIMSReply);
1500+ lastStreamBufferedBytes = StoreIOBuffer(); // storeClientCopy(next()->readBuffer) invalidates
1501 StoreIOBuffer localTempBuffer (next()->readBuffer.length, 0, next()->readBuffer.data);
1502- storeClientCopy(sc, http->storeEntry(), localTempBuffer, SendMoreData, this);
1503+ ::storeClientCopy(sc, http->storeEntry(), localTempBuffer, recipient, this);
1504+}
1505+
1506+/// Request HTTP response body bytes from Store into next()->readBuffer. This
1507+/// method requests body bytes at readerBuffer.offset and, hence, it should only
1508+/// be called after we triggerInitialStoreRead() and get the requested HTTP
1509+/// response headers (using zero offset).
1510+void
1511+clientReplyContext::requestMoreBodyFromStore()
1512+{
1513+ lastStreamBufferedBytes = StoreIOBuffer(); // storeClientCopy(next()->readBuffer) invalidates
1514+ ::storeClientCopy(sc, http->storeEntry(), next()->readBuffer, SendMoreData, this);
1515 }
1516
1517 /* there is an expired entry in the store.
1518@@ -358,30 +357,23 @@ clientReplyContext::processExpired()
1519 {
1520 /* start counting the length from 0 */
1521 StoreIOBuffer localTempBuffer(HTTP_REQBUF_SZ, 0, tempbuf);
1522- storeClientCopy(sc, entry, localTempBuffer, HandleIMSReply, this);
1523+ // keep lastStreamBufferedBytes: tempbuf is not a Client Stream buffer
1524+ ::storeClientCopy(sc, entry, localTempBuffer, HandleIMSReply, this);
1525 }
1526 }
1527
1528 void
1529-clientReplyContext::sendClientUpstreamResponse()
1530+clientReplyContext::sendClientUpstreamResponse(const StoreIOBuffer &upstreamResponse)
1531 {
1532- StoreIOBuffer tempresult;
1533 removeStoreReference(&old_sc, &old_entry);
1534
1535 if (collapsedRevalidation)
1536 http->storeEntry()->clearPublicKeyScope();
1537
1538 /* here the data to send is the data we just received */
1539- tempBuffer.offset = 0;
1540- old_reqsize = 0;
1541- /* sendMoreData tracks the offset as well.
1542- * Force it back to zero */
1543- reqofs = 0;
1544 assert(!EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED));
1545- /* TODO: provide sendMoreData with the ready parsed reply */
1546- tempresult.length = reqsize;
1547- tempresult.data = tempbuf;
1548- sendMoreData(tempresult);
1549+
1550+ sendMoreData(upstreamResponse);
1551 }
1552
1553 void
1554@@ -398,11 +390,9 @@ clientReplyContext::sendClientOldEntry()
1555 restoreState();
1556 /* here the data to send is in the next nodes buffers already */
1557 assert(!EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED));
1558- /* sendMoreData tracks the offset as well.
1559- * Force it back to zero */
1560- reqofs = 0;
1561- StoreIOBuffer tempresult (reqsize, reqofs, next()->readBuffer.data);
1562- sendMoreData(tempresult);
1563+ Assure(matchesStreamBodyBuffer(lastStreamBufferedBytes));
1564+ Assure(!lastStreamBufferedBytes.offset);
1565+ sendMoreData(lastStreamBufferedBytes);
1566 }
1567
1568 /* This is the workhorse of the HandleIMSReply callback.
1569@@ -416,11 +406,11 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result)
1570 if (deleting)
1571 return;
1572
1573- debugs(88, 3, http->storeEntry()->url() << ", " << (long unsigned) result.length << " bytes");
1574-
1575 if (http->storeEntry() == NULL)
1576 return;
1577
1578+ debugs(88, 3, http->storeEntry()->url() << " got " << result);
1579+
1580 if (result.flags.error && !EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED))
1581 return;
1582
1583@@ -433,9 +423,6 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result)
1584 return;
1585 }
1586
1587- /* update size of the request */
1588- reqsize = result.length + reqofs;
1589-
1590 const Http::StatusCode status = http->storeEntry()->getReply()->sline.status();
1591
1592 // request to origin was aborted
1593@@ -460,7 +447,7 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result)
1594 if (http->request->flags.ims && !old_entry->modifiedSince(http->request->ims, http->request->imslen)) {
1595 // forward the 304 from origin
1596 debugs(88, 3, "origin replied 304, revalidating existing entry and forwarding 304 to client");
1597- sendClientUpstreamResponse();
1598+ sendClientUpstreamResponse(result);
1599 } else {
1600 // send existing entry, it's still valid
1601 debugs(88, 3, "origin replied 304, revalidating existing entry and sending " <<
1602@@ -484,7 +471,7 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result)
1603 http->logType = LOG_TCP_REFRESH_MODIFIED;
1604 debugs(88, 3, "origin replied " << status <<
1605 ", replacing existing entry and forwarding to client");
1606- sendClientUpstreamResponse();
1607+ sendClientUpstreamResponse(result);
1608 }
1609 }
1610
1611@@ -493,7 +480,7 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result)
1612 http->logType = LOG_TCP_REFRESH_FAIL_ERR;
1613 debugs(88, 3, "origin replied with error " << status <<
1614 ", forwarding to client due to fail_on_validation_err");
1615- sendClientUpstreamResponse();
1616+ sendClientUpstreamResponse(result);
1617 } else {
1618 // ignore and let client have old entry
1619 http->logType = LOG_TCP_REFRESH_FAIL_OLD;
1620@@ -506,13 +493,7 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result)
1621 SQUIDCEXTERN CSR clientGetMoreData;
1622 SQUIDCEXTERN CSD clientReplyDetach;
1623
1624-/**
1625- * clientReplyContext::cacheHit Should only be called until the HTTP reply headers
1626- * have been parsed. Normally this should be a single call, but
1627- * it might take more than one. As soon as we have the headers,
1628- * we hand off to clientSendMoreData, processExpired, or
1629- * processMiss.
1630- */
1631+/// \copydoc clientReplyContext::cacheHit()
1632 void
1633 clientReplyContext::CacheHit(void *data, StoreIOBuffer result)
1634 {
1635@@ -520,11 +501,11 @@ clientReplyContext::CacheHit(void *data, StoreIOBuffer result)
1636 context->cacheHit(result);
1637 }
1638
1639-/**
1640- * Process a possible cache HIT.
1641- */
1642+/// Processes HTTP response headers received from Store on a suspected cache hit
1643+/// path. May be called several times (e.g., a Vary marker object hit followed
1644+/// by the corresponding variant hit).
1645 void
1646-clientReplyContext::cacheHit(StoreIOBuffer result)
1647+clientReplyContext::cacheHit(const StoreIOBuffer result)
1648 {
1649 /** Ignore if the HIT object is being deleted. */
1650 if (deleting) {
1651@@ -536,7 +517,7 @@ clientReplyContext::cacheHit(StoreIOBuffer result)
1652
1653 HttpRequest *r = http->request;
1654
1655- debugs(88, 3, "clientCacheHit: " << http->uri << ", " << result.length << " bytes");
1656+ debugs(88, 3, http->uri << " got " << result);
1657
1658 if (http->storeEntry() == NULL) {
1659 debugs(88, 3, "clientCacheHit: request aborted");
1660@@ -560,20 +541,7 @@ clientReplyContext::cacheHit(StoreIOBuffer result)
1661 return;
1662 }
1663
1664- if (result.length == 0) {
1665- debugs(88, 5, "store IO buffer has no content. MISS");
1666- /* the store couldn't get enough data from the file for us to id the
1667- * object
1668- */
1669- /* treat as a miss */
1670- http->logType = LOG_TCP_MISS;
1671- processMiss();
1672- return;
1673- }
1674-
1675 assert(!EBIT_TEST(e->flags, ENTRY_ABORTED));
1676- /* update size of the request */
1677- reqsize = result.length + reqofs;
1678
1679 /*
1680 * Got the headers, now grok them
1681@@ -587,6 +555,8 @@ clientReplyContext::cacheHit(StoreIOBuffer result)
1682 return;
1683 }
1684
1685+ noteStreamBufferredBytes(result);
1686+
1687 switch (varyEvaluateMatch(e, r)) {
1688
1689 case VARY_NONE:
1690@@ -687,7 +657,7 @@ clientReplyContext::cacheHit(StoreIOBuffer result)
1691 return;
1692 } else if (r->conditional()) {
1693 debugs(88, 5, "conditional HIT");
1694- if (processConditional(result))
1695+ if (processConditional())
1696 return;
1697 }
1698
1699@@ -806,7 +776,7 @@ clientReplyContext::processOnlyIfCachedMiss()
1700
1701 /// process conditional request from client
1702 bool
1703-clientReplyContext::processConditional(StoreIOBuffer &result)
1704+clientReplyContext::processConditional()
1705 {
1706 StoreEntry *const e = http->storeEntry();
1707
1708@@ -984,16 +954,7 @@ clientReplyContext::purgeFoundObject(StoreEntry *entry)
1709
1710 http->logType = LOG_TCP_HIT;
1711
1712- reqofs = 0;
1713-
1714- localTempBuffer.offset = http->out.offset;
1715-
1716- localTempBuffer.length = next()->readBuffer.length;
1717-
1718- localTempBuffer.data = next()->readBuffer.data;
1719-
1720- storeClientCopy(sc, http->storeEntry(),
1721- localTempBuffer, CacheHit, this);
1722+ triggerInitialStoreRead(CacheHit);
1723 }
1724
1725 void
1726@@ -1111,16 +1072,10 @@ clientReplyContext::purgeDoPurgeHead(StoreEntry *newEntry)
1727 }
1728
1729 void
1730-clientReplyContext::traceReply(clientStreamNode * node)
1731+clientReplyContext::traceReply()
1732 {
1733- clientStreamNode *nextNode = (clientStreamNode *)node->node.next->data;
1734- StoreIOBuffer localTempBuffer;
1735 createStoreEntry(http->request->method, RequestFlags());
1736- localTempBuffer.offset = nextNode->readBuffer.offset + headers_sz;
1737- localTempBuffer.length = nextNode->readBuffer.length;
1738- localTempBuffer.data = nextNode->readBuffer.data;
1739- storeClientCopy(sc, http->storeEntry(),
1740- localTempBuffer, SendMoreData, this);
1741+ triggerInitialStoreRead();
1742 http->storeEntry()->releaseRequest();
1743 http->storeEntry()->buffer();
1744 HttpReply *rep = new HttpReply;
1745@@ -1169,16 +1124,16 @@ int
1746 clientReplyContext::storeOKTransferDone() const
1747 {
1748 assert(http->storeEntry()->objectLen() >= 0);
1749+ const auto headers_sz = http->storeEntry()->mem().baseReply().hdr_sz;
1750 assert(http->storeEntry()->objectLen() >= headers_sz);
1751- if (http->out.offset >= http->storeEntry()->objectLen() - headers_sz) {
1752- debugs(88,3,HERE << "storeOKTransferDone " <<
1753- " out.offset=" << http->out.offset <<
1754- " objectLen()=" << http->storeEntry()->objectLen() <<
1755- " headers_sz=" << headers_sz);
1756- return 1;
1757- }
1758
1759- return 0;
1760+ const auto done = http->out.offset >= http->storeEntry()->objectLen() - headers_sz;
1761+ const auto debugLevel = done ? 3 : 5;
1762+ debugs(88, debugLevel, done <<
1763+ " out.offset=" << http->out.offset <<
1764+ " objectLen()=" << http->storeEntry()->objectLen() <<
1765+ " headers_sz=" << headers_sz);
1766+ return done ? 1 : 0;
1767 }
1768
1769 int
1770@@ -1190,10 +1145,9 @@ clientReplyContext::storeNotOKTransferDone() const
1771 MemObject *mem = http->storeEntry()->mem_obj;
1772 assert(mem != NULL);
1773 assert(http->request != NULL);
1774- /* mem->reply was wrong because it uses the UPSTREAM header length!!! */
1775- HttpReply const *curReply = mem->getReply();
1776+ const auto expectedBodySize = mem->baseReply().content_length;
1777
1778- if (headers_sz == 0)
1779+ if (mem->baseReply().pstate != psParsed)
1780 /* haven't found end of headers yet */
1781 return 0;
1782
1783@@ -1202,19 +1156,14 @@ clientReplyContext::storeNotOKTransferDone() const
1784 * If we are sending a body and we don't have a content-length,
1785 * then we must wait for the object to become STORE_OK.
1786 */
1787- if (curReply->content_length < 0)
1788- return 0;
1789-
1790- uint64_t expectedLength = curReply->content_length + http->out.headers_sz;
1791-
1792- if (http->out.size < expectedLength)
1793+ if (expectedBodySize < 0)
1794 return 0;
1795- else {
1796- debugs(88,3,HERE << "storeNotOKTransferDone " <<
1797- " out.size=" << http->out.size <<
1798- " expectedLength=" << expectedLength);
1799- return 1;
1800- }
1801+ const auto done = http->out.offset >= expectedBodySize;
1802+ const auto debugLevel = done ? 3 : 5;
1803+ debugs(88, debugLevel, done <<
1804+ " out.offset=" << http->out.offset <<
1805+ " expectedBodySize=" << expectedBodySize);
1806+ return done ? 1 : 0;
1807 }
1808
1809 /* A write has completed, what is the next status based on the
1810@@ -1632,6 +1581,8 @@ clientReplyContext::cloneReply()
1811 reply = http->storeEntry()->getReply()->clone();
1812 HTTPMSGLOCK(reply);
1813
1814+ http->al->reply = reply;
1815+
1816 if (reply->sline.protocol == AnyP::PROTO_HTTP) {
1817 /* RFC 2616 requires us to advertise our version (but only on real HTTP traffic) */
1818 reply->sline.version = Http::ProtocolVersion();
1819@@ -1778,20 +1729,12 @@ clientGetMoreData(clientStreamNode * aNode, ClientHttpRequest * http)
1820 assert (context);
1821 assert(context->http == http);
1822
1823- clientStreamNode *next = ( clientStreamNode *)aNode->node.next->data;
1824-
1825 if (!context->ourNode)
1826 context->ourNode = aNode;
1827
1828 /* no cbdatareference, this is only used once, and safely */
1829 if (context->flags.storelogiccomplete) {
1830- StoreIOBuffer tempBuffer;
1831- tempBuffer.offset = next->readBuffer.offset + context->headers_sz;
1832- tempBuffer.length = next->readBuffer.length;
1833- tempBuffer.data = next->readBuffer.data;
1834-
1835- storeClientCopy(context->sc, http->storeEntry(),
1836- tempBuffer, clientReplyContext::SendMoreData, context);
1837+ context->requestMoreBodyFromStore();
1838 return;
1839 }
1840
1841@@ -1804,7 +1747,7 @@ clientGetMoreData(clientStreamNode * aNode, ClientHttpRequest * http)
1842
1843 if (context->http->request->method == Http::METHOD_TRACE) {
1844 if (context->http->request->header.getInt64(Http::HdrType::MAX_FORWARDS) == 0) {
1845- context->traceReply(aNode);
1846+ context->traceReply();
1847 return;
1848 }
1849
1850@@ -1834,7 +1777,6 @@ clientReplyContext::doGetMoreData()
1851 #endif
1852
1853 assert(http->logType.oldType == LOG_TCP_HIT);
1854- reqofs = 0;
1855 /* guarantee nothing has been sent yet! */
1856 assert(http->out.size == 0);
1857 assert(http->out.offset == 0);
1858@@ -1849,10 +1791,7 @@ clientReplyContext::doGetMoreData()
1859 }
1860 }
1861
1862- localTempBuffer.offset = reqofs;
1863- localTempBuffer.length = getNextNode()->readBuffer.length;
1864- localTempBuffer.data = getNextNode()->readBuffer.data;
1865- storeClientCopy(sc, http->storeEntry(), localTempBuffer, CacheHit, this);
1866+ triggerInitialStoreRead(CacheHit);
1867 } else {
1868 /* MISS CASE, http->logType is already set! */
1869 processMiss();
1870@@ -1887,12 +1826,11 @@ clientReplyContext::makeThisHead()
1871 }
1872
1873 bool
1874-clientReplyContext::errorInStream(StoreIOBuffer const &result, size_t const &sizeToProcess)const
1875+clientReplyContext::errorInStream(const StoreIOBuffer &result) const
1876 {
1877 return /* aborted request */
1878 (http->storeEntry() && EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED)) ||
1879- /* Upstream read error */ (result.flags.error) ||
1880- /* Upstream EOF */ (sizeToProcess == 0);
1881+ /* Upstream read error */ (result.flags.error);
1882 }
1883
1884 void
1885@@ -1913,24 +1851,17 @@ clientReplyContext::sendStreamError(StoreIOBuffer const &result)
1886 }
1887
1888 void
1889-clientReplyContext::pushStreamData(StoreIOBuffer const &result, char *source)
1890+clientReplyContext::pushStreamData(const StoreIOBuffer &result)
1891 {
1892- StoreIOBuffer localTempBuffer;
1893-
1894 if (result.length == 0) {
1895 debugs(88, 5, "clientReplyContext::pushStreamData: marking request as complete due to 0 length store result");
1896 flags.complete = 1;
1897 }
1898
1899- assert(result.offset - headers_sz == next()->readBuffer.offset);
1900- localTempBuffer.offset = result.offset - headers_sz;
1901- localTempBuffer.length = result.length;
1902-
1903- if (localTempBuffer.length)
1904- localTempBuffer.data = source;
1905+ assert(!result.length || result.offset == next()->readBuffer.offset);
1906
1907 clientStreamCallback((clientStreamNode*)http->client_stream.head->data, http, NULL,
1908- localTempBuffer);
1909+ result);
1910 }
1911
1912 clientStreamNode *
1913@@ -2022,7 +1953,6 @@ clientReplyContext::processReplyAccess ()
1914 if (http->logType.oldType == LOG_TCP_DENIED ||
1915 http->logType.oldType == LOG_TCP_DENIED_REPLY ||
1916 alwaysAllowResponse(reply->sline.status())) {
1917- headers_sz = reply->hdr_sz;
1918 processReplyAccessResult(ACCESS_ALLOWED);
1919 return;
1920 }
1921@@ -2033,8 +1963,6 @@ clientReplyContext::processReplyAccess ()
1922 return;
1923 }
1924
1925- headers_sz = reply->hdr_sz;
1926-
1927 /** check for absent access controls (permit by default) */
1928 if (!Config.accessList.reply) {
1929 processReplyAccessResult(ACCESS_ALLOWED);
1930@@ -2091,11 +2019,9 @@ clientReplyContext::processReplyAccessResult(const allow_t &accessAllowed)
1931 /* Ok, the reply is allowed, */
1932 http->loggingEntry(http->storeEntry());
1933
1934- ssize_t body_size = reqofs - reply->hdr_sz;
1935- if (body_size < 0) {
1936- reqofs = reply->hdr_sz;
1937- body_size = 0;
1938- }
1939+ Assure(matchesStreamBodyBuffer(lastStreamBufferedBytes));
1940+ Assure(!lastStreamBufferedBytes.offset);
1941+ auto body_size = lastStreamBufferedBytes.length; // may be zero
1942
1943 debugs(88, 3, "clientReplyContext::sendMoreData: Appending " <<
1944 (int) body_size << " bytes after " << reply->hdr_sz <<
1945@@ -2123,19 +2049,27 @@ clientReplyContext::processReplyAccessResult(const allow_t &accessAllowed)
1946 assert (!flags.headersSent);
1947 flags.headersSent = true;
1948
1949+ // next()->readBuffer.offset may be positive for Range requests, but our
1950+ // localTempBuffer initialization code assumes that next()->readBuffer.data
1951+ // points to the response body at offset 0 because the first
1952+ // storeClientCopy() request always has offset 0 (i.e. our first Store
1953+ // request ignores next()->readBuffer.offset).
1954+ //
1955+ // XXX: We cannot fully check that assumption: readBuffer.offset field is
1956+ // often out of sync with the buffer content, and if some buggy code updates
1957+ // the buffer while we were waiting for the processReplyAccessResult()
1958+ // callback, we may not notice.
1959+
1960 StoreIOBuffer localTempBuffer;
1961- char *buf = next()->readBuffer.data;
1962- char *body_buf = buf + reply->hdr_sz;
1963+ const auto body_buf = next()->readBuffer.data;
1964
1965 //Server side may disable ranges under some circumstances.
1966
1967 if ((!http->request->range))
1968 next()->readBuffer.offset = 0;
1969
1970- body_buf -= next()->readBuffer.offset;
1971-
1972- if (next()->readBuffer.offset != 0) {
1973- if (next()->readBuffer.offset > body_size) {
1974+ if (next()->readBuffer.offset > 0) {
1975+ if (Less(body_size, next()->readBuffer.offset)) {
1976 /* Can't use any of the body we received. send nothing */
1977 localTempBuffer.length = 0;
1978 localTempBuffer.data = NULL;
1979@@ -2148,7 +2082,6 @@ clientReplyContext::processReplyAccessResult(const allow_t &accessAllowed)
1980 localTempBuffer.data = body_buf;
1981 }
1982
1983- /* TODO??: move the data in the buffer back by the request header size */
1984 clientStreamCallback((clientStreamNode *)http->client_stream.head->data,
1985 http, reply, localTempBuffer);
1986
1987@@ -2161,6 +2094,8 @@ clientReplyContext::sendMoreData (StoreIOBuffer result)
1988 if (deleting)
1989 return;
1990
1991+ debugs(88, 5, http->uri << " got " << result);
1992+
1993 StoreEntry *entry = http->storeEntry();
1994
1995 if (ConnStateData * conn = http->getConn()) {
1996@@ -2173,7 +2108,9 @@ clientReplyContext::sendMoreData (StoreIOBuffer result)
1997 return;
1998 }
1999
2000- if (reqofs==0 && !http->logType.isTcpHit()) {
2001+ if (!flags.headersSent && !http->logType.isTcpHit()) {
2002+ // We get here twice if processReplyAccessResult() calls startError().
2003+ // TODO: Revise when we check/change QoS markings to reduce syscalls.
2004 if (Ip::Qos::TheConfig.isHitTosActive()) {
2005 Ip::Qos::doTosLocalMiss(conn->clientConnection, http->request->hier.code);
2006 }
2007@@ -2187,21 +2124,9 @@ clientReplyContext::sendMoreData (StoreIOBuffer result)
2008 " out.offset=" << http->out.offset);
2009 }
2010
2011- char *buf = next()->readBuffer.data;
2012-
2013- if (buf != result.data) {
2014- /* we've got to copy some data */
2015- assert(result.length <= next()->readBuffer.length);
2016- memcpy(buf, result.data, result.length);
2017- }
2018-
2019 /* We've got the final data to start pushing... */
2020 flags.storelogiccomplete = 1;
2021
2022- reqofs += result.length;
2023-
2024- assert(reqofs <= HTTP_REQBUF_SZ || flags.headersSent);
2025-
2026 assert(http->request != NULL);
2027
2028 /* ESI TODO: remove this assert once everything is stable */
2029@@ -2210,20 +2135,25 @@ clientReplyContext::sendMoreData (StoreIOBuffer result)
2030
2031 makeThisHead();
2032
2033- debugs(88, 5, "clientReplyContext::sendMoreData: " << http->uri << ", " <<
2034- reqofs << " bytes (" << result.length <<
2035- " new bytes)");
2036-
2037- /* update size of the request */
2038- reqsize = reqofs;
2039-
2040- if (errorInStream(result, reqofs)) {
2041+ if (errorInStream(result)) {
2042 sendStreamError(result);
2043 return;
2044 }
2045
2046+ if (!matchesStreamBodyBuffer(result)) {
2047+ // Subsequent processing expects response body bytes to be at the start
2048+ // of our Client Stream buffer. When given something else (e.g., bytes
2049+ // in our tempbuf), we copy and adjust to meet those expectations.
2050+ const auto &ourClientStreamsBuffer = next()->readBuffer;
2051+ assert(result.length <= ourClientStreamsBuffer.length);
2052+ memcpy(ourClientStreamsBuffer.data, result.data, result.length);
2053+ result.data = ourClientStreamsBuffer.data;
2054+ }
2055+
2056+ noteStreamBufferredBytes(result);
2057+
2058 if (flags.headersSent) {
2059- pushStreamData (result, buf);
2060+ pushStreamData(result);
2061 return;
2062 }
2063
2064@@ -2234,23 +2164,38 @@ clientReplyContext::sendMoreData (StoreIOBuffer result)
2065 sc->setDelayId(DelayId::DelayClient(http,reply));
2066 #endif
2067
2068- /* handle headers */
2069+ holdingBuffer = result;
2070+ processReplyAccess();
2071+ return;
2072+}
2073+
2074+/// Whether the given body area describes the start of our Client Stream buffer.
2075+/// An empty area does.
2076+bool
2077+clientReplyContext::matchesStreamBodyBuffer(const StoreIOBuffer &their) const
2078+{
2079+ // the answer is undefined for errors; they are not really "body buffers"
2080+ Assure(!their.flags.error);
2081
2082- if (Config.onoff.log_mime_hdrs) {
2083- size_t k;
2084+ if (!their.length)
2085+ return true; // an empty body area always matches our body area
2086
2087- if ((k = headersEnd(buf, reqofs))) {
2088- safe_free(http->al->headers.reply);
2089- http->al->headers.reply = (char *)xcalloc(k + 1, 1);
2090- xstrncpy(http->al->headers.reply, buf, k);
2091- }
2092+ if (their.data != next()->readBuffer.data) {
2093+ debugs(88, 7, "no: " << their << " vs. " << next()->readBuffer);
2094+ return false;
2095 }
2096
2097- holdingBuffer = result;
2098- processReplyAccess();
2099- return;
2100+ return true;
2101+}
2102+
2103+void
2104+clientReplyContext::noteStreamBufferredBytes(const StoreIOBuffer &result)
2105+{
2106+ Assure(matchesStreamBodyBuffer(result));
2107+ lastStreamBufferedBytes = result; // may be unchanged and/or zero-length
2108 }
2109
2110+
2111 /* Using this breaks the client layering just a little!
2112 */
2113 void
2114@@ -2289,13 +2234,6 @@ clientReplyContext::createStoreEntry(const HttpRequestMethod& m, RequestFlags re
2115 sc->setDelayId(DelayId::DelayClient(http));
2116 #endif
2117
2118- reqofs = 0;
2119-
2120- reqsize = 0;
2121-
2122- /* I don't think this is actually needed! -- adrian */
2123- /* http->reqbuf = http->norm_reqbuf; */
2124- // assert(http->reqbuf == http->norm_reqbuf);
2125 /* The next line is illegal because we don't know if the client stream
2126 * buffers have been set up
2127 */
2128diff --git a/src/client_side_reply.h b/src/client_side_reply.h
2129index dddab1a..bf705a4 100644
2130--- a/src/client_side_reply.h
2131+++ b/src/client_side_reply.h
2132@@ -39,7 +39,6 @@ public:
2133 void purgeFoundGet(StoreEntry *newEntry);
2134 void purgeFoundHead(StoreEntry *newEntry);
2135 void purgeFoundObject(StoreEntry *entry);
2136- void sendClientUpstreamResponse();
2137 void purgeDoPurgeGet(StoreEntry *entry);
2138 void purgeDoPurgeHead(StoreEntry *entry);
2139 void doGetMoreData();
2140@@ -67,7 +66,7 @@ public:
2141 void processExpired();
2142 clientStream_status_t replyStatus();
2143 void processMiss();
2144- void traceReply(clientStreamNode * node);
2145+ void traceReply();
2146 const char *storeId() const { return (http->store_id.size() > 0 ? http->store_id.termedBuf() : http->uri); }
2147
2148 Http::StatusCode purgeStatus;
2149@@ -77,13 +76,14 @@ public:
2150 virtual void created (StoreEntry *newEntry);
2151
2152 ClientHttpRequest *http;
2153- int headers_sz;
2154 store_client *sc; /* The store_client we're using */
2155 StoreIOBuffer tempBuffer; /* For use in validating requests via IMS */
2156 int old_reqsize; /* ... again, for the buffer */
2157- size_t reqsize;
2158- size_t reqofs;
2159- char tempbuf[HTTP_REQBUF_SZ]; ///< a temporary buffer if we need working storage
2160+ /// Buffer dedicated to receiving storeClientCopy() responses to generated
2161+ /// revalidation requests. These requests cannot use next()->readBuffer
2162+ /// because the latter keeps the contents of the stale HTTP response during
2163+ /// revalidation. sendClientOldEntry() uses that contents.
2164+ char tempbuf[HTTP_REQBUF_SZ];
2165 #if USE_CACHE_DIGESTS
2166
2167 const char *lookup_type; /* temporary hack: storeGet() result: HIT/MISS/NONE */
2168@@ -101,9 +101,10 @@ public:
2169 private:
2170 clientStreamNode *getNextNode() const;
2171 void makeThisHead();
2172- bool errorInStream(StoreIOBuffer const &result, size_t const &sizeToProcess)const ;
2173+ bool errorInStream(const StoreIOBuffer &result) const;
2174+ bool matchesStreamBodyBuffer(const StoreIOBuffer &) const;
2175 void sendStreamError(StoreIOBuffer const &result);
2176- void pushStreamData(StoreIOBuffer const &result, char *source);
2177+ void pushStreamData(const StoreIOBuffer &);
2178 clientStreamNode * next() const;
2179 StoreIOBuffer holdingBuffer;
2180 HttpReply *reply;
2181@@ -115,11 +116,13 @@ private:
2182 bool alwaysAllowResponse(Http::StatusCode sline) const;
2183 int checkTransferDone();
2184 void processOnlyIfCachedMiss();
2185- bool processConditional(StoreIOBuffer &result);
2186+ bool processConditional();
2187+ void noteStreamBufferredBytes(const StoreIOBuffer &);
2188 void cacheHit(StoreIOBuffer result);
2189 void handleIMSReply(StoreIOBuffer result);
2190 void sendMoreData(StoreIOBuffer result);
2191- void triggerInitialStoreRead();
2192+ void triggerInitialStoreRead(STCB = SendMoreData);
2193+ void requestMoreBodyFromStore();
2194 void sendClientOldEntry();
2195 void purgeAllCached();
2196 void forgetHit();
2197@@ -129,6 +132,13 @@ private:
2198 void sendPreconditionFailedError();
2199 void sendNotModified();
2200 void sendNotModifiedOrPreconditionFailedError();
2201+ void sendClientUpstreamResponse(const StoreIOBuffer &upstreamResponse);
2202+
2203+ /// Reduces a chance of an accidental direct storeClientCopy() call that
2204+ /// (should but) forgets to invalidate our lastStreamBufferedBytes. This
2205+ /// function is not defined; decltype() syntax prohibits "= delete", but
2206+ /// function usage will trigger deprecation warnings and linking errors.
2207+ static decltype(::storeClientCopy) storeClientCopy [[deprecated]];
2208
2209 StoreEntry *old_entry;
2210 /* ... for entry to be validated */
2211@@ -145,6 +155,12 @@ private:
2212 } CollapsedRevalidation;
2213
2214 CollapsedRevalidation collapsedRevalidation;
2215+
2216+ /// HTTP response body bytes stored in our Client Stream buffer (if any)
2217+ StoreIOBuffer lastStreamBufferedBytes;
2218+
2219+ // TODO: Remove after moving the meat of this function into a method.
2220+ friend CSR clientGetMoreData;
2221 };
2222
2223 #endif /* SQUID_CLIENTSIDEREPLY_H */
2224diff --git a/src/client_side_request.cc b/src/client_side_request.cc
2225index ab08fd2..92da530 100644
2226--- a/src/client_side_request.cc
2227+++ b/src/client_side_request.cc
2228@@ -2045,6 +2045,8 @@ ClientHttpRequest::handleAdaptedHeader(HttpMsg *msg)
2229 storeEntry()->replaceHttpReply(new_rep);
2230 storeEntry()->timestampsSet();
2231
2232+ al->reply = new_rep;
2233+
2234 if (!adaptedBodySource) // no body
2235 storeEntry()->complete();
2236 clientGetMoreData(node, this);
2237diff --git a/src/clients/Client.cc b/src/clients/Client.cc
2238index f5defbb..cada70e 100644
2239--- a/src/clients/Client.cc
2240+++ b/src/clients/Client.cc
2241@@ -136,6 +136,8 @@ Client::setVirginReply(HttpReply *rep)
2242 assert(rep);
2243 theVirginReply = rep;
2244 HTTPMSGLOCK(theVirginReply);
2245+ if (fwd->al)
2246+ fwd->al->reply = theVirginReply;
2247 return theVirginReply;
2248 }
2249
2250@@ -155,6 +157,8 @@ Client::setFinalReply(HttpReply *rep)
2251 assert(rep);
2252 theFinalReply = rep;
2253 HTTPMSGLOCK(theFinalReply);
2254+ if (fwd->al)
2255+ fwd->al->reply = theFinalReply;
2256
2257 // give entry the reply because haveParsedReplyHeaders() expects it there
2258 entry->replaceHttpReply(theFinalReply, false); // but do not write yet
2259@@ -550,6 +554,7 @@ Client::blockCaching()
2260 ACLFilledChecklist ch(acl, originalRequest(), NULL);
2261 ch.reply = const_cast<HttpReply*>(entry->getReply()); // ACLFilledChecklist API bug
2262 HTTPMSGLOCK(ch.reply);
2263+ ch.al = fwd->al;
2264 if (!ch.fastCheck().allowed()) { // when in doubt, block
2265 debugs(20, 3, "store_miss prohibits caching");
2266 return true;
2267diff --git a/src/enums.h b/src/enums.h
2268index 4a860d8..262d62c 100644
2269--- a/src/enums.h
2270+++ b/src/enums.h
2271@@ -203,7 +203,6 @@ enum {
2272 typedef enum {
2273 DIGEST_READ_NONE,
2274 DIGEST_READ_REPLY,
2275- DIGEST_READ_HEADERS,
2276 DIGEST_READ_CBLOCK,
2277 DIGEST_READ_MASK,
2278 DIGEST_READ_DONE
2279diff --git a/src/format/Format.cc b/src/format/Format.cc
2280index 3b6a44b..689bdf9 100644
2281--- a/src/format/Format.cc
2282+++ b/src/format/Format.cc
2283@@ -330,7 +330,7 @@ log_quoted_string(const char *str, char *out)
2284 static const HttpMsg *
2285 actualReplyHeader(const AccessLogEntry::Pointer &al)
2286 {
2287- const HttpMsg *msg = al->reply;
2288+ const HttpMsg *msg = al->reply.getRaw();
2289 #if ICAP_CLIENT
2290 // al->icap.reqMethod is methodNone in access.log context
2291 if (!msg && al->icap.reqMethod == Adaptation::methodReqmod)
2292@@ -853,24 +853,35 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS
2293 } else
2294 #endif
2295 {
2296+ // just headers without start-line and CRLF
2297+ // XXX: reconcile with '<h'
2298 out = al->headers.request;
2299 quote = 1;
2300 }
2301 break;
2302
2303 case LFT_ADAPTED_REQUEST_ALL_HEADERS:
2304+ // just headers without start-line and CRLF
2305+ // XXX: reconcile with '<h'
2306 out = al->headers.adapted_request;
2307 quote = 1;
2308 break;
2309
2310- case LFT_REPLY_ALL_HEADERS:
2311- out = al->headers.reply;
2312+ case LFT_REPLY_ALL_HEADERS: {
2313+ MemBuf allHeaders;
2314+ allHeaders.init();
2315+ // status-line + headers + CRLF
2316+ // XXX: reconcile with '>h' and '>ha'
2317+ al->packReplyHeaders(allHeaders);
2318+ sb.assign(allHeaders.content(), allHeaders.contentSize());
2319+ out = sb.c_str();
2320 #if ICAP_CLIENT
2321 if (!out && al->icap.reqMethod == Adaptation::methodReqmod)
2322 out = al->headers.adapted_request;
2323 #endif
2324 quote = 1;
2325- break;
2326+ }
2327+ break;
2328
2329 case LFT_USER_NAME:
2330 #if USE_AUTH
2331diff --git a/src/http.cc b/src/http.cc
2332index 017e492..877172d 100644
2333--- a/src/http.cc
2334+++ b/src/http.cc
2335@@ -775,6 +775,9 @@ HttpStateData::processReplyHeader()
2336 void
2337 HttpStateData::handle1xx(HttpReply *reply)
2338 {
2339+ if (fwd->al)
2340+ fwd->al->reply = reply;
2341+
2342 HttpReply::Pointer msg(reply); // will destroy reply if unused
2343
2344 // one 1xx at a time: we must not be called while waiting for previous 1xx
2345diff --git a/src/icmp/net_db.cc b/src/icmp/net_db.cc
2346index 7dc42a2..52595f6 100644
2347--- a/src/icmp/net_db.cc
2348+++ b/src/icmp/net_db.cc
2349@@ -33,6 +33,7 @@
2350 #include "mgr/Registration.h"
2351 #include "mime_header.h"
2352 #include "neighbors.h"
2353+#include "sbuf/SBuf.h"
2354 #include "SquidConfig.h"
2355 #include "SquidTime.h"
2356 #include "Store.h"
2357@@ -49,8 +50,6 @@
2358 #include "ipcache.h"
2359 #include "StoreClient.h"
2360
2361-#define NETDB_REQBUF_SZ 4096
2362-
2363 typedef enum {
2364 STATE_NONE,
2365 STATE_HEADER,
2366@@ -67,12 +66,8 @@ public:
2367 e(NULL),
2368 sc(NULL),
2369 r(theReq),
2370- used(0),
2371- buf_sz(NETDB_REQBUF_SZ),
2372- buf_ofs(0),
2373 connstate(STATE_HEADER)
2374 {
2375- *buf = 0;
2376
2377 assert(NULL != r);
2378 HTTPMSGLOCK(r);
2379@@ -92,10 +87,10 @@ public:
2380 StoreEntry *e;
2381 store_client *sc;
2382 HttpRequest *r;
2383- int64_t used;
2384- size_t buf_sz;
2385- char buf[NETDB_REQBUF_SZ];
2386- int buf_ofs;
2387+
2388+ /// for receiving a NetDB reply body from Store and interpreting it
2389+ Store::ParsingBuffer parsingBuffer;
2390+
2391 netdb_conn_state_t connstate;
2392 };
2393
2394@@ -698,24 +693,19 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData)
2395 Ip::Address addr;
2396
2397 netdbExchangeState *ex = (netdbExchangeState *)data;
2398- int rec_sz = 0;
2399- int o;
2400
2401 struct in_addr line_addr;
2402 double rtt;
2403 double hops;
2404- char *p;
2405 int j;
2406 HttpReply const *rep;
2407- size_t hdr_sz;
2408 int nused = 0;
2409- int size;
2410- int oldbufofs = ex->buf_ofs;
2411
2412- rec_sz = 0;
2413+ size_t rec_sz = 0; // received record size (TODO: make const)
2414 rec_sz += 1 + sizeof(struct in_addr);
2415 rec_sz += 1 + sizeof(int);
2416 rec_sz += 1 + sizeof(int);
2417+ Assure(rec_sz <= ex->parsingBuffer.capacity());
2418 debugs(38, 3, "netdbExchangeHandleReply: " << receivedData.length << " read bytes");
2419
2420 if (!cbdataReferenceValid(ex->p)) {
2421@@ -726,64 +716,29 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData)
2422
2423 debugs(38, 3, "netdbExchangeHandleReply: for '" << ex->p->host << ":" << ex->p->http_port << "'");
2424
2425- if (receivedData.length == 0 && !receivedData.flags.error) {
2426+ if (receivedData.flags.error) {
2427 debugs(38, 3, "netdbExchangeHandleReply: Done");
2428 delete ex;
2429 return;
2430 }
2431
2432- p = ex->buf;
2433-
2434- /* Get the size of the buffer now */
2435- size = ex->buf_ofs + receivedData.length;
2436- debugs(38, 3, "netdbExchangeHandleReply: " << size << " bytes buf");
2437-
2438- /* Check if we're still doing headers */
2439-
2440 if (ex->connstate == STATE_HEADER) {
2441-
2442- ex->buf_ofs += receivedData.length;
2443-
2444- /* skip reply headers */
2445-
2446- if ((hdr_sz = headersEnd(p, ex->buf_ofs))) {
2447- debugs(38, 5, "netdbExchangeHandleReply: hdr_sz = " << hdr_sz);
2448- rep = ex->e->getReply();
2449- assert(rep->sline.status() != Http::scNone);
2450- debugs(38, 3, "netdbExchangeHandleReply: reply status " << rep->sline.status());
2451-
2452- if (rep->sline.status() != Http::scOkay) {
2453- delete ex;
2454- return;
2455- }
2456-
2457- assert((size_t)ex->buf_ofs >= hdr_sz);
2458-
2459- /*
2460- * Now, point p to the part of the buffer where the data
2461- * starts, and update the size accordingly
2462- */
2463- assert(ex->used == 0);
2464- ex->used = hdr_sz;
2465- size = ex->buf_ofs - hdr_sz;
2466- p += hdr_sz;
2467-
2468- /* Finally, set the conn state mode to STATE_BODY */
2469- ex->connstate = STATE_BODY;
2470- } else {
2471- StoreIOBuffer tempBuffer;
2472- tempBuffer.offset = ex->buf_ofs;
2473- tempBuffer.length = ex->buf_sz - ex->buf_ofs;
2474- tempBuffer.data = ex->buf + ex->buf_ofs;
2475- /* Have more headers .. */
2476- storeClientCopy(ex->sc, ex->e, tempBuffer,
2477- netdbExchangeHandleReply, ex);
2478+ const auto scode = ex->e->mem().baseReply().sline.status();
2479+ assert(scode != Http::scNone);
2480+ debugs(38, 3, "reply status " << scode);
2481+ if (scode != Http::scOkay) {
2482+ delete ex;
2483 return;
2484- }
2485+ }
2486+ ex->connstate = STATE_BODY;
2487 }
2488
2489 assert(ex->connstate == STATE_BODY);
2490
2491+ ex->parsingBuffer.appended(receivedData.data, receivedData.length);
2492+ auto p = ex->parsingBuffer.c_str(); // current parsing position
2493+ auto size = ex->parsingBuffer.contentSize(); // bytes we still need to parse
2494+
2495 /* If we get here, we have some body to parse .. */
2496 debugs(38, 5, "netdbExchangeHandleReply: start parsing loop, size = " << size);
2497
2498@@ -792,6 +747,7 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData)
2499 addr.setAnyAddr();
2500 hops = rtt = 0.0;
2501
2502+ size_t o; // current record parsing offset
2503 for (o = 0; o < rec_sz;) {
2504 switch ((int) *(p + o)) {
2505
2506@@ -829,8 +785,6 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData)
2507
2508 assert(o == rec_sz);
2509
2510- ex->used += rec_sz;
2511-
2512 size -= rec_sz;
2513
2514 p += rec_sz;
2515@@ -838,32 +792,8 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData)
2516 ++nused;
2517 }
2518
2519- /*
2520- * Copy anything that is left over to the beginning of the buffer,
2521- * and adjust buf_ofs accordingly
2522- */
2523-
2524- /*
2525- * Evilly, size refers to the buf size left now,
2526- * ex->buf_ofs is the original buffer size, so just copy that
2527- * much data over
2528- */
2529- memmove(ex->buf, ex->buf + (ex->buf_ofs - size), size);
2530-
2531- ex->buf_ofs = size;
2532-
2533- /*
2534- * And don't re-copy the remaining data ..
2535- */
2536- ex->used += size;
2537-
2538- /*
2539- * Now the tricky bit - size _included_ the leftover bit from the _last_
2540- * storeClientCopy. We don't want to include that, or our offset will be wrong.
2541- * So, don't count the size of the leftover buffer we began with.
2542- * This can _disappear_ when we're not tracking offsets ..
2543- */
2544- ex->used -= oldbufofs;
2545+ const auto parsedSize = ex->parsingBuffer.contentSize() - size;
2546+ ex->parsingBuffer.consume(parsedSize);
2547
2548 debugs(38, 3, "netdbExchangeHandleReply: size left over in this buffer: " << size << " bytes");
2549
2550@@ -871,20 +801,26 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData)
2551 " entries, (x " << rec_sz << " bytes) == " << nused * rec_sz <<
2552 " bytes total");
2553
2554- debugs(38, 3, "netdbExchangeHandleReply: used " << ex->used);
2555-
2556 if (EBIT_TEST(ex->e->flags, ENTRY_ABORTED)) {
2557 debugs(38, 3, "netdbExchangeHandleReply: ENTRY_ABORTED");
2558 delete ex;
2559- } else if (ex->e->store_status == STORE_PENDING) {
2560- StoreIOBuffer tempBuffer;
2561- tempBuffer.offset = ex->used;
2562- tempBuffer.length = ex->buf_sz - ex->buf_ofs;
2563- tempBuffer.data = ex->buf + ex->buf_ofs;
2564- debugs(38, 3, "netdbExchangeHandleReply: EOF not received");
2565- storeClientCopy(ex->sc, ex->e, tempBuffer,
2566- netdbExchangeHandleReply, ex);
2567+ return;
2568 }
2569+
2570+ if (ex->sc->atEof()) {
2571+ if (const auto leftoverBytes = ex->parsingBuffer.contentSize())
2572+ debugs(38, 2, "discarding a partially received record due to Store EOF: " << leftoverBytes);
2573+ delete ex;
2574+ return;
2575+ }
2576+
2577+ // TODO: To protect us from a broken peer sending an "infinite" stream of
2578+ // new addresses, limit the cumulative number of received bytes or records?
2579+
2580+ const auto remainingSpace = ex->parsingBuffer.space().positionAt(receivedData.offset + receivedData.length);
2581+ // rec_sz is at most buffer capacity, and we consume all fully loaded records
2582+ Assure(remainingSpace.length);
2583+ storeClientCopy(ex->sc, ex->e, remainingSpace, netdbExchangeHandleReply, ex);
2584 }
2585
2586 #endif /* USE_ICMP */
2587@@ -1296,14 +1232,9 @@ netdbExchangeStart(void *data)
2588 ex->e = storeCreateEntry(uri, uri, RequestFlags(), Http::METHOD_GET);
2589 assert(NULL != ex->e);
2590
2591- StoreIOBuffer tempBuffer;
2592- tempBuffer.length = ex->buf_sz;
2593- tempBuffer.data = ex->buf;
2594-
2595 ex->sc = storeClientListAdd(ex->e, ex);
2596+ storeClientCopy(ex->sc, ex->e, ex->parsingBuffer.makeInitialSpace(), netdbExchangeHandleReply, ex);
2597
2598- storeClientCopy(ex->sc, ex->e, tempBuffer,
2599- netdbExchangeHandleReply, ex);
2600 ex->r->flags.loopDetected = true; /* cheat! -- force direct */
2601
2602 // XXX: send as Proxy-Authenticate instead
2603diff --git a/src/internal.cc b/src/internal.cc
2604index 81d5175..3a04ce0 100644
2605--- a/src/internal.cc
2606+++ b/src/internal.cc
2607@@ -9,6 +9,7 @@
2608 /* DEBUG: section 76 Internal Squid Object handling */
2609
2610 #include "squid.h"
2611+#include "base/Assure.h"
2612 #include "CacheManager.h"
2613 #include "comm/Connection.h"
2614 #include "errorpage.h"
2615diff --git a/src/log/FormatHttpdCombined.cc b/src/log/FormatHttpdCombined.cc
2616index 6639e88..70ea336 100644
2617--- a/src/log/FormatHttpdCombined.cc
2618+++ b/src/log/FormatHttpdCombined.cc
2619@@ -69,7 +69,10 @@ Log::Format::HttpdCombined(const AccessLogEntry::Pointer &al, Logfile * logfile)
2620
2621 if (Config.onoff.log_mime_hdrs) {
2622 char *ereq = ::Format::QuoteMimeBlob(al->headers.request);
2623- char *erep = ::Format::QuoteMimeBlob(al->headers.reply);
2624+ MemBuf mb;
2625+ mb.init();
2626+ al->packReplyHeaders(mb);
2627+ auto erep = ::Format::QuoteMimeBlob(mb.content());
2628 logfilePrintf(logfile, " [%s] [%s]\n", ereq, erep);
2629 safe_free(ereq);
2630 safe_free(erep);
2631diff --git a/src/log/FormatHttpdCommon.cc b/src/log/FormatHttpdCommon.cc
2632index 1613d0e..9e933a0 100644
2633--- a/src/log/FormatHttpdCommon.cc
2634+++ b/src/log/FormatHttpdCommon.cc
2635@@ -54,7 +54,10 @@ Log::Format::HttpdCommon(const AccessLogEntry::Pointer &al, Logfile * logfile)
2636
2637 if (Config.onoff.log_mime_hdrs) {
2638 char *ereq = ::Format::QuoteMimeBlob(al->headers.request);
2639- char *erep = ::Format::QuoteMimeBlob(al->headers.reply);
2640+ MemBuf mb;
2641+ mb.init();
2642+ al->packReplyHeaders(mb);
2643+ auto erep = ::Format::QuoteMimeBlob(mb.content());
2644 logfilePrintf(logfile, " [%s] [%s]\n", ereq, erep);
2645 safe_free(ereq);
2646 safe_free(erep);
2647diff --git a/src/log/FormatSquidNative.cc b/src/log/FormatSquidNative.cc
2648index 0ab97e4..23076b2 100644
2649--- a/src/log/FormatSquidNative.cc
2650+++ b/src/log/FormatSquidNative.cc
2651@@ -71,7 +71,10 @@ Log::Format::SquidNative(const AccessLogEntry::Pointer &al, Logfile * logfile)
2652
2653 if (Config.onoff.log_mime_hdrs) {
2654 char *ereq = ::Format::QuoteMimeBlob(al->headers.request);
2655- char *erep = ::Format::QuoteMimeBlob(al->headers.reply);
2656+ MemBuf mb;
2657+ mb.init();
2658+ al->packReplyHeaders(mb);
2659+ auto erep = ::Format::QuoteMimeBlob(mb.content());
2660 logfilePrintf(logfile, " [%s] [%s]\n", ereq, erep);
2661 safe_free(ereq);
2662 safe_free(erep);
2663diff --git a/src/peer_digest.cc b/src/peer_digest.cc
2664index 7b6314d..7c96ce8 100644
2665--- a/src/peer_digest.cc
2666+++ b/src/peer_digest.cc
2667@@ -39,7 +39,6 @@ static EVH peerDigestCheck;
2668 static void peerDigestRequest(PeerDigest * pd);
2669 static STCB peerDigestHandleReply;
2670 static int peerDigestFetchReply(void *, char *, ssize_t);
2671-int peerDigestSwapInHeaders(void *, char *, ssize_t);
2672 int peerDigestSwapInCBlock(void *, char *, ssize_t);
2673 int peerDigestSwapInMask(void *, char *, ssize_t);
2674 static int peerDigestFetchedEnough(DigestFetchState * fetch, char *buf, ssize_t size, const char *step_name);
2675@@ -374,6 +373,9 @@ peerDigestRequest(PeerDigest * pd)
2676 fetch->sc = storeClientListAdd(e, fetch);
2677 /* set lastmod to trigger IMS request if possible */
2678
2679+ // TODO: Also check for fetch->pd->cd presence as a precondition for sending
2680+ // IMS requests because peerDigestFetchReply() does not accept 304 responses
2681+ // without an in-memory cache digest.
2682 if (old_e)
2683 e->lastModified(old_e->lastModified());
2684
2685@@ -408,6 +410,11 @@ peerDigestHandleReply(void *data, StoreIOBuffer receivedData)
2686 digest_read_state_t prevstate;
2687 int newsize;
2688
2689+ if (receivedData.flags.error) {
2690+ peerDigestFetchAbort(fetch, fetch->buf, "failure loading digest reply from Store");
2691+ return;
2692+ }
2693+
2694 assert(fetch->pd && receivedData.data);
2695 /* The existing code assumes that the received pointer is
2696 * where we asked the data to be put
2697@@ -444,10 +451,6 @@ peerDigestHandleReply(void *data, StoreIOBuffer receivedData)
2698 retsize = peerDigestFetchReply(fetch, fetch->buf, fetch->bufofs);
2699 break;
2700
2701- case DIGEST_READ_HEADERS:
2702- retsize = peerDigestSwapInHeaders(fetch, fetch->buf, fetch->bufofs);
2703- break;
2704-
2705 case DIGEST_READ_CBLOCK:
2706 retsize = peerDigestSwapInCBlock(fetch, fetch->buf, fetch->bufofs);
2707 break;
2708@@ -487,7 +490,7 @@ peerDigestHandleReply(void *data, StoreIOBuffer receivedData)
2709 // checking at the beginning of this function. However, in this case, we would have to require
2710 // that the parser does not regard EOF as a special condition (it is true now but may change
2711 // in the future).
2712- if (!receivedData.length) { // EOF
2713+ if (fetch->sc->atEof()) {
2714 peerDigestFetchAbort(fetch, fetch->buf, "premature end of digest reply");
2715 return;
2716 }
2717@@ -506,19 +509,12 @@ peerDigestHandleReply(void *data, StoreIOBuffer receivedData)
2718 }
2719 }
2720
2721-/* wait for full http headers to be received then parse them */
2722-/*
2723- * This routine handles parsing the reply line.
2724- * If the reply line indicates an OK, the same data is thrown
2725- * to SwapInHeaders(). If the reply line is a NOT_MODIFIED,
2726- * we simply stop parsing.
2727- */
2728+/// handle HTTP response headers in the initial storeClientCopy() response
2729 static int
2730 peerDigestFetchReply(void *data, char *buf, ssize_t size)
2731 {
2732 DigestFetchState *fetch = (DigestFetchState *)data;
2733 PeerDigest *pd = fetch->pd;
2734- size_t hdr_size;
2735 assert(pd && buf);
2736 assert(!fetch->offset);
2737
2738@@ -527,7 +523,7 @@ peerDigestFetchReply(void *data, char *buf, ssize_t size)
2739 if (peerDigestFetchedEnough(fetch, buf, size, "peerDigestFetchReply"))
2740 return -1;
2741
2742- if ((hdr_size = headersEnd(buf, size))) {
2743+ {
2744 HttpReply const *reply = fetch->entry->getReply();
2745 assert(reply);
2746 assert(reply->sline.status() != Http::scNone);
2747@@ -563,6 +559,15 @@ peerDigestFetchReply(void *data, char *buf, ssize_t size)
2748 /* preserve request -- we need its size to update counters */
2749 /* requestUnlink(r); */
2750 /* fetch->entry->mem_obj->request = NULL; */
2751+
2752+ if (!fetch->pd->cd) {
2753+ peerDigestFetchAbort(fetch, buf, "304 without the old in-memory digest");
2754+ return -1;
2755+ }
2756+
2757+ // stay with the old in-memory digest
2758+ peerDigestFetchStop(fetch, buf, "Not modified");
2759+ fetch->state = DIGEST_READ_DONE;
2760 } else if (status == Http::scOkay) {
2761 /* get rid of old entry if any */
2762
2763@@ -573,67 +578,12 @@ peerDigestFetchReply(void *data, char *buf, ssize_t size)
2764 fetch->old_entry->unlock("peerDigestFetchReply 200");
2765 fetch->old_entry = NULL;
2766 }
2767+ fetch->state = DIGEST_READ_CBLOCK;
2768 } else {
2769 /* some kind of a bug */
2770 peerDigestFetchAbort(fetch, buf, reply->sline.reason());
2771 return -1; /* XXX -1 will abort stuff in ReadReply! */
2772 }
2773-
2774- /* must have a ready-to-use store entry if we got here */
2775- /* can we stay with the old in-memory digest? */
2776- if (status == Http::scNotModified && fetch->pd->cd) {
2777- peerDigestFetchStop(fetch, buf, "Not modified");
2778- fetch->state = DIGEST_READ_DONE;
2779- } else {
2780- fetch->state = DIGEST_READ_HEADERS;
2781- }
2782- } else {
2783- /* need more data, do we have space? */
2784-
2785- if (size >= SM_PAGE_SIZE)
2786- peerDigestFetchAbort(fetch, buf, "reply header too big");
2787- }
2788-
2789- /* We don't want to actually ack that we've handled anything,
2790- * otherwise SwapInHeaders() won't get the reply line .. */
2791- return 0;
2792-}
2793-
2794-/* fetch headers from disk, pass on to SwapInCBlock */
2795-int
2796-peerDigestSwapInHeaders(void *data, char *buf, ssize_t size)
2797-{
2798- DigestFetchState *fetch = (DigestFetchState *)data;
2799- size_t hdr_size;
2800-
2801- assert(fetch->state == DIGEST_READ_HEADERS);
2802-
2803- if (peerDigestFetchedEnough(fetch, buf, size, "peerDigestSwapInHeaders"))
2804- return -1;
2805-
2806- assert(!fetch->offset);
2807-
2808- if ((hdr_size = headersEnd(buf, size))) {
2809- assert(fetch->entry->getReply());
2810- assert(fetch->entry->getReply()->sline.status() != Http::scNone);
2811-
2812- if (fetch->entry->getReply()->sline.status() != Http::scOkay) {
2813- debugs(72, DBG_IMPORTANT, "peerDigestSwapInHeaders: " << fetch->pd->host <<
2814- " status " << fetch->entry->getReply()->sline.status() <<
2815- " got cached!");
2816-
2817- peerDigestFetchAbort(fetch, buf, "internal status error");
2818- return -1;
2819- }
2820-
2821- fetch->state = DIGEST_READ_CBLOCK;
2822- return hdr_size; /* Say how much data we read */
2823- }
2824-
2825- /* need more data, do we have space? */
2826- if (size >= SM_PAGE_SIZE) {
2827- peerDigestFetchAbort(fetch, buf, "stored header too big");
2828- return -1;
2829 }
2830
2831 return 0; /* We need to read more to parse .. */
2832diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc
2833index fab26cf..d3faa8d 100644
2834--- a/src/servers/FtpServer.cc
2835+++ b/src/servers/FtpServer.cc
2836@@ -777,12 +777,6 @@ Ftp::Server::handleReply(HttpReply *reply, StoreIOBuffer data)
2837 Http::StreamPointer context = pipeline.front();
2838 assert(context != nullptr);
2839
2840- if (context->http && context->http->al != NULL &&
2841- !context->http->al->reply && reply) {
2842- context->http->al->reply = reply;
2843- HTTPMSGLOCK(context->http->al->reply);
2844- }
2845-
2846 static ReplyHandler handlers[] = {
2847 NULL, // fssBegin
2848 NULL, // fssConnected
2849diff --git a/src/servers/Http1Server.cc b/src/servers/Http1Server.cc
2850index 7514779..e76fb3e 100644
2851--- a/src/servers/Http1Server.cc
2852+++ b/src/servers/Http1Server.cc
2853@@ -310,9 +310,6 @@ Http::One::Server::handleReply(HttpReply *rep, StoreIOBuffer receivedData)
2854 }
2855
2856 assert(rep);
2857- HTTPMSGUNLOCK(http->al->reply);
2858- http->al->reply = rep;
2859- HTTPMSGLOCK(http->al->reply);
2860 context->sendStartOfMessage(rep, receivedData);
2861 }
2862
2863diff --git a/src/stmem.cc b/src/stmem.cc
2864index d117c15..b627005 100644
2865--- a/src/stmem.cc
2866+++ b/src/stmem.cc
2867@@ -95,8 +95,6 @@ mem_hdr::freeDataUpto(int64_t target_offset)
2868 break;
2869 }
2870
2871- assert (lowestOffset () <= target_offset);
2872-
2873 return lowestOffset ();
2874 }
2875
2876diff --git a/src/store.cc b/src/store.cc
2877index 1948447..b4c7f82 100644
2878--- a/src/store.cc
2879+++ b/src/store.cc
2880@@ -273,6 +273,8 @@ StoreEntry::storeClientType() const
2881
2882 assert(mem_obj);
2883
2884+ debugs(20, 7, *this << " inmem_lo=" << mem_obj->inmem_lo);
2885+
2886 if (mem_obj->inmem_lo)
2887 return STORE_DISK_CLIENT;
2888
2889@@ -300,6 +302,7 @@ StoreEntry::storeClientType() const
2890 return STORE_MEM_CLIENT;
2891 }
2892 }
2893+ debugs(20, 7, "STORE_OK STORE_DISK_CLIENT");
2894 return STORE_DISK_CLIENT;
2895 }
2896
2897@@ -319,10 +322,18 @@ StoreEntry::storeClientType() const
2898 if (swap_status == SWAPOUT_NONE)
2899 return STORE_MEM_CLIENT;
2900
2901+ // TODO: The above "must make this a mem client" logic contradicts "Slight
2902+ // weirdness" logic in store_client::doCopy() that converts hits to misses
2903+ // on startSwapin() failures. We should probably attempt to open a swapin
2904+ // file _here_ instead (and avoid STORE_DISK_CLIENT designation for clients
2905+ // that fail to do so). That would also address a similar problem with Rock
2906+ // store that does not yet support swapin during SWAPOUT_WRITING.
2907+
2908 /*
2909 * otherwise, make subsequent clients read from disk so they
2910 * can not delay the first, and vice-versa.
2911 */
2912+ debugs(20, 7, "STORE_PENDING STORE_DISK_CLIENT");
2913 return STORE_DISK_CLIENT;
2914 }
2915
2916diff --git a/src/store/Makefile.am b/src/store/Makefile.am
2917index be177d8..ccfc2dd 100644
2918--- a/src/store/Makefile.am
2919+++ b/src/store/Makefile.am
2920@@ -23,4 +23,6 @@ libstore_la_SOURCES= \
2921 forward.h \
2922 LocalSearch.cc \
2923 LocalSearch.h \
2924+ ParsingBuffer.cc \
2925+ ParsingBuffer.h \
2926 Storage.h
2927diff --git a/src/store/Makefile.in b/src/store/Makefile.in
2928index bb4387d..1959c99 100644
2929--- a/src/store/Makefile.in
2930+++ b/src/store/Makefile.in
2931@@ -163,7 +163,7 @@ CONFIG_CLEAN_FILES =
2932 CONFIG_CLEAN_VPATH_FILES =
2933 LTLIBRARIES = $(noinst_LTLIBRARIES)
2934 libstore_la_LIBADD =
2935-am_libstore_la_OBJECTS = Controller.lo Disk.lo Disks.lo LocalSearch.lo
2936+am_libstore_la_OBJECTS = Controller.lo Disk.lo Disks.lo LocalSearch.lo ParsingBuffer.lo
2937 libstore_la_OBJECTS = $(am_libstore_la_OBJECTS)
2938 AM_V_lt = $(am__v_lt_@AM_V@)
2939 am__v_lt_ = $(am__v_lt_@AM_DEFAULT_V@)
2940@@ -185,7 +185,7 @@ DEFAULT_INCLUDES =
2941 depcomp = $(SHELL) $(top_srcdir)/cfgaux/depcomp
2942 am__maybe_remake_depfiles = depfiles
2943 am__depfiles_remade = ./$(DEPDIR)/Controller.Plo ./$(DEPDIR)/Disk.Plo \
2944- ./$(DEPDIR)/Disks.Plo ./$(DEPDIR)/LocalSearch.Plo
2945+ ./$(DEPDIR)/Disks.Plo ./$(DEPDIR)/LocalSearch.Plo ./$(DEPDIR)/ParsingBuffer.Plo
2946 am__mv = mv -f
2947 CXXCOMPILE = $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) \
2948 $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS)
2949@@ -776,6 +776,8 @@ libstore_la_SOURCES = \
2950 forward.h \
2951 LocalSearch.cc \
2952 LocalSearch.h \
2953+ ParsingBuffer.cc \
2954+ ParsingBuffer.h \
2955 Storage.h
2956
2957 all: all-recursive
2958@@ -846,6 +848,7 @@ distclean-compile:
2959 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/Disk.Plo@am__quote@ # am--include-marker
2960 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/Disks.Plo@am__quote@ # am--include-marker
2961 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/LocalSearch.Plo@am__quote@ # am--include-marker
2962+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ParsingBuffer.Plo@am__quote@ # am--include-marker
2963
2964 $(am__depfiles_remade):
2965 @$(MKDIR_P) $(@D)
2966@@ -1254,6 +1257,7 @@ distclean: distclean-recursive
2967 -rm -f ./$(DEPDIR)/Disk.Plo
2968 -rm -f ./$(DEPDIR)/Disks.Plo
2969 -rm -f ./$(DEPDIR)/LocalSearch.Plo
2970+ -rm -f ./$(DEPDIR)/ParsingBuffer.Plo
2971 -rm -f Makefile
2972 distclean-am: clean-am distclean-compile distclean-generic \
2973 distclean-tags
2974@@ -1303,6 +1307,7 @@ maintainer-clean: maintainer-clean-recursive
2975 -rm -f ./$(DEPDIR)/Disk.Plo
2976 -rm -f ./$(DEPDIR)/Disks.Plo
2977 -rm -f ./$(DEPDIR)/LocalSearch.Plo
2978+ -rm -f ./$(DEPDIR)/ParsingBuffer.Plo
2979 -rm -f Makefile
2980 maintainer-clean-am: distclean-am maintainer-clean-generic
2981
2982diff --git a/src/store/ParsingBuffer.cc b/src/store/ParsingBuffer.cc
2983new file mode 100644
2984index 0000000..ca6be72
2985--- /dev/null
2986+++ b/src/store/ParsingBuffer.cc
2987@@ -0,0 +1,199 @@
2988+/*
2989+ * Copyright (C) 1996-2023 The Squid Software Foundation and contributors
2990+ *
2991+ * Squid software is distributed under GPLv2+ license and includes
2992+ * contributions from numerous individuals and organizations.
2993+ * Please see the COPYING and CONTRIBUTORS files for details.
2994+ */
2995+
2996+#include "squid.h"
2997+#include "sbuf/Stream.h"
2998+#include "SquidMath.h"
2999+#include "store/ParsingBuffer.h"
3000+
3001+#include <iostream>
3002+
3003+// Several Store::ParsingBuffer() methods use assert() because the corresponding
3004+// failure means there is a good chance that somebody have already read from (or
3005+// written to) the wrong memory location. Since this buffer is used for storing
3006+// HTTP response bytes, such failures may corrupt traffic. No Assure() handling
3007+// code can safely recover from such failures.
3008+
3009+Store::ParsingBuffer::ParsingBuffer(StoreIOBuffer &initialSpace):
3010+ readerSuppliedMemory_(initialSpace)
3011+{
3012+}
3013+
3014+/// a read-only content start (or nil for some zero-size buffers)
3015+const char *
3016+Store::ParsingBuffer::memory() const
3017+{
3018+ return extraMemory_.second ? extraMemory_.first.rawContent() : readerSuppliedMemory_.data;
3019+}
3020+
3021+size_t
3022+Store::ParsingBuffer::capacity() const
3023+{
3024+ return extraMemory_.second ? (extraMemory_.first.length() + extraMemory_.first.spaceSize()) : readerSuppliedMemory_.length;
3025+}
3026+
3027+size_t
3028+Store::ParsingBuffer::contentSize() const
3029+{
3030+ return extraMemory_.second ? extraMemory_.first.length() : readerSuppliedMemoryContentSize_;
3031+}
3032+
3033+void
3034+Store::ParsingBuffer::appended(const char * const newBytes, const size_t newByteCount)
3035+{
3036+ // a positive newByteCount guarantees that, after the first assertion below
3037+ // succeeds, the second assertion will not increment a nil memory() pointer
3038+ if (!newByteCount)
3039+ return;
3040+
3041+ // these checks order guarantees that memory() is not nil in the second assertion
3042+ assert(newByteCount <= spaceSize()); // the new bytes end in our space
3043+ assert(memory() + contentSize() == newBytes); // the new bytes start in our space
3044+ // and now we know that newBytes is not nil either
3045+
3046+ if (extraMemory_.second)
3047+ extraMemory_.first.rawAppendFinish(newBytes, newByteCount);
3048+ else
3049+ readerSuppliedMemoryContentSize_ = IncreaseSum(readerSuppliedMemoryContentSize_, newByteCount).first;
3050+
3051+ assert(contentSize() <= capacity()); // paranoid
3052+}
3053+
3054+void
3055+Store::ParsingBuffer::consume(const size_t parsedBytes)
3056+{
3057+ Assure(contentSize() >= parsedBytes); // more conservative than extraMemory_->consume()
3058+ if (extraMemory_.second) {
3059+ extraMemory_.first.consume(parsedBytes);
3060+ } else {
3061+ readerSuppliedMemoryContentSize_ -= parsedBytes;
3062+ if (parsedBytes && readerSuppliedMemoryContentSize_)
3063+ memmove(readerSuppliedMemory_.data, memory() + parsedBytes, readerSuppliedMemoryContentSize_);
3064+ }
3065+}
3066+
3067+StoreIOBuffer
3068+Store::ParsingBuffer::space()
3069+{
3070+ const auto size = spaceSize();
3071+ const auto start = extraMemory_.second ?
3072+ extraMemory_.first.rawAppendStart(size) :
3073+ (readerSuppliedMemory_.data + readerSuppliedMemoryContentSize_);
3074+ return StoreIOBuffer(spaceSize(), 0, start);
3075+}
3076+
3077+StoreIOBuffer
3078+Store::ParsingBuffer::makeSpace(const size_t pageSize)
3079+{
3080+ growSpace(pageSize);
3081+ auto result = space();
3082+ Assure(result.length >= pageSize);
3083+ result.length = pageSize;
3084+ return result;
3085+}
3086+
3087+StoreIOBuffer
3088+Store::ParsingBuffer::content() const
3089+{
3090+ // This const_cast is a StoreIOBuffer API limitation: That class does not
3091+ // support a "constant content view", even though it is used as such a view.
3092+ return StoreIOBuffer(contentSize(), 0, const_cast<char*>(memory()));
3093+}
3094+
3095+/// makes sure we have the requested number of bytes, allocates enough memory if needed
3096+void
3097+Store::ParsingBuffer::growSpace(const size_t minimumSpaceSize)
3098+{
3099+ const auto capacityIncreaseAttempt = IncreaseSum(contentSize(), minimumSpaceSize);
3100+ if (!capacityIncreaseAttempt.second)
3101+ throw TextException(ToSBuf("no support for a single memory block of ", contentSize(), '+', minimumSpaceSize, " bytes"), Here());
3102+ const auto newCapacity = capacityIncreaseAttempt.first;
3103+
3104+ if (newCapacity <= capacity())
3105+ return; // already have enough space; no reallocation is needed
3106+
3107+ debugs(90, 7, "growing to provide " << minimumSpaceSize << " in " << *this);
3108+
3109+ if (extraMemory_.second) {
3110+ extraMemory_.first.reserveCapacity(newCapacity);
3111+ } else {
3112+ SBuf newStorage;
3113+ newStorage.reserveCapacity(newCapacity);
3114+ newStorage.append(readerSuppliedMemory_.data, readerSuppliedMemoryContentSize_);
3115+ extraMemory_.first = std::move(newStorage);
3116+ extraMemory_.second = true;
3117+ }
3118+ Assure(spaceSize() >= minimumSpaceSize);
3119+}
3120+
3121+SBuf
3122+Store::ParsingBuffer::toSBuf() const
3123+{
3124+ return extraMemory_.second ? extraMemory_.first : SBuf(content().data, content().length);
3125+}
3126+
3127+size_t
3128+Store::ParsingBuffer::spaceSize() const
3129+{
3130+ if (extraMemory_.second)
3131+ return extraMemory_.first.spaceSize();
3132+
3133+ assert(readerSuppliedMemoryContentSize_ <= readerSuppliedMemory_.length);
3134+ return readerSuppliedMemory_.length - readerSuppliedMemoryContentSize_;
3135+}
3136+
3137+/// 0-terminates stored byte sequence, allocating more memory if needed, but
3138+/// without increasing the number of stored content bytes
3139+void
3140+Store::ParsingBuffer::terminate()
3141+{
3142+ *makeSpace(1).data = 0;
3143+}
3144+
3145+StoreIOBuffer
3146+Store::ParsingBuffer::packBack()
3147+{
3148+ const auto bytesToPack = contentSize();
3149+ // until our callers do not have to work around legacy code expectations
3150+ Assure(bytesToPack);
3151+
3152+ // if we accumulated more bytes at some point, any extra metadata should
3153+ // have been consume()d by now, allowing readerSuppliedMemory_.data reuse
3154+ Assure(bytesToPack <= readerSuppliedMemory_.length);
3155+
3156+ auto result = readerSuppliedMemory_;
3157+ result.length = bytesToPack;
3158+ Assure(result.data);
3159+
3160+ if (!extraMemory_.second) {
3161+ // no accumulated bytes copying because they are in readerSuppliedMemory_
3162+ debugs(90, 7, "quickly exporting " << result.length << " bytes via " << readerSuppliedMemory_);
3163+ } else {
3164+ debugs(90, 7, "slowly exporting " << result.length << " bytes from " << extraMemory_.first.id << " back into " << readerSuppliedMemory_);
3165+ memmove(result.data, extraMemory_.first.rawContent(), result.length);
3166+ }
3167+
3168+ return result;
3169+}
3170+
3171+void
3172+Store::ParsingBuffer::print(std::ostream &os) const
3173+{
3174+ os << "size=" << contentSize();
3175+
3176+ if (extraMemory_.second) {
3177+ os << " capacity=" << capacity();
3178+ os << " extra=" << extraMemory_.first.id;
3179+ }
3180+
3181+ // report readerSuppliedMemory_ (if any) even if we are no longer using it
3182+ // for content storage; it affects packBack() and related parsing logic
3183+ if (readerSuppliedMemory_.length)
3184+ os << ' ' << readerSuppliedMemory_;
3185+}
3186+
3187diff --git a/src/store/ParsingBuffer.h b/src/store/ParsingBuffer.h
3188new file mode 100644
3189index 0000000..b473ac6
3190--- /dev/null
3191+++ b/src/store/ParsingBuffer.h
3192@@ -0,0 +1,128 @@
3193+/*
3194+ * Copyright (C) 1996-2023 The Squid Software Foundation and contributors
3195+ *
3196+ * Squid software is distributed under GPLv2+ license and includes
3197+ * contributions from numerous individuals and organizations.
3198+ * Please see the COPYING and CONTRIBUTORS files for details.
3199+ */
3200+
3201+#ifndef SQUID_SRC_STORE_PARSINGBUFFER_H
3202+#define SQUID_SRC_STORE_PARSINGBUFFER_H
3203+
3204+#include "sbuf/SBuf.h"
3205+#include "StoreIOBuffer.h"
3206+
3207+#include <optional>
3208+
3209+namespace Store
3210+{
3211+
3212+/// A continuous buffer for efficient accumulation and NUL-termination of
3213+/// Store-read bytes. The buffer accumulates two kinds of Store readers:
3214+///
3215+/// * Readers that do not have any external buffer to worry about but need to
3216+/// accumulate, terminate, and/or consume buffered content read by Store.
3217+/// These readers use the default constructor and then allocate the initial
3218+/// buffer space for their first read (if any).
3219+///
3220+/// * Readers that supply their StoreIOBuffer at construction time. That buffer
3221+/// is enough to handle the majority of use cases. However, the supplied
3222+/// StoreIOBuffer capacity may be exceeded when parsing requires accumulating
3223+/// multiple Store read results and/or NUL-termination of a full buffer.
3224+///
3225+/// This buffer seamlessly grows as needed, reducing memory over-allocation and,
3226+/// in case of StoreIOBuffer-seeded construction, memory copies.
3227+class ParsingBuffer
3228+{
3229+public:
3230+ /// creates buffer without any space or content
3231+ ParsingBuffer() = default;
3232+
3233+ /// seeds this buffer with the caller-supplied buffer space
3234+ explicit ParsingBuffer(StoreIOBuffer &);
3235+
3236+ /// a NUL-terminated version of content(); same lifetime as content()
3237+ const char *c_str() { terminate(); return memory(); }
3238+
3239+ /// export content() into SBuf, avoiding content copying when possible
3240+ SBuf toSBuf() const;
3241+
3242+ /// the total number of append()ed bytes that were not consume()d
3243+ size_t contentSize() const;
3244+
3245+ /// the number of bytes in the space() buffer
3246+ size_t spaceSize() const;
3247+
3248+ /// the maximum number of bytes we can store without allocating more space
3249+ size_t capacity() const;
3250+
3251+ /// Stored append()ed bytes that have not been consume()d. The returned
3252+ /// buffer offset is set to zero; the caller is responsible for adjusting
3253+ /// the offset if needed (TODO: Add/return a no-offset Mem::View instead).
3254+ /// The returned buffer is invalidated by calling a non-constant method or
3255+ /// by changing the StoreIOBuffer contents given to our constructor.
3256+ StoreIOBuffer content() const;
3257+
3258+ /// A (possibly empty) buffer for reading the next byte(s). The returned
3259+ /// buffer offset is set to zero; the caller is responsible for adjusting
3260+ /// the offset if needed (TODO: Add/return a no-offset Mem::Area instead).
3261+ /// The returned buffer is invalidated by calling a non-constant method or
3262+ /// by changing the StoreIOBuffer contents given to our constructor.
3263+ StoreIOBuffer space();
3264+
3265+ /// A buffer for reading the exact number of next byte(s). The method may
3266+ /// allocate new memory and copy previously appended() bytes as needed.
3267+ /// \param pageSize the exact number of bytes the caller wants to read
3268+ /// \returns space() after any necessary allocations
3269+ StoreIOBuffer makeSpace(size_t pageSize);
3270+
3271+ /// A buffer suitable for the first storeClientCopy() call. The method may
3272+ /// allocate new memory and copy previously appended() bytes as needed.
3273+ /// \returns space() after any necessary allocations
3274+ /// \deprecated New clients should call makeSpace() with client-specific
3275+ /// pageSize instead of this one-size-fits-all legacy method.
3276+ StoreIOBuffer makeInitialSpace() { return makeSpace(4096); }
3277+
3278+ /// remember the new bytes received into the previously provided space()
3279+ void appended(const char *, size_t);
3280+
3281+ /// get rid of previously appended() prefix of a given size
3282+ void consume(size_t);
3283+
3284+ /// Returns stored content, reusing the StoreIOBuffer given at the
3285+ /// construction time. Copying is avoided if we did not allocate extra
3286+ /// memory since construction. Not meant for default-constructed buffers.
3287+ /// \prec positive contentSize() (\sa store_client::finishCallback())
3288+ StoreIOBuffer packBack();
3289+
3290+ /// summarizes object state (for debugging)
3291+ void print(std::ostream &) const;
3292+
3293+private:
3294+ const char *memory() const;
3295+ void terminate();
3296+ void growSpace(size_t);
3297+
3298+private:
3299+ /// externally allocated buffer we were seeded with (or a zero-size one)
3300+ StoreIOBuffer readerSuppliedMemory_;
3301+
3302+ /// append()ed to readerSuppliedMemory_ bytes that were not consume()d
3303+ size_t readerSuppliedMemoryContentSize_ = 0;
3304+
3305+ /// our internal buffer that takes over readerSuppliedMemory_ when the
3306+ /// latter becomes full and more memory is needed
3307+ std::pair<SBuf, bool> extraMemory_ = std::make_pair(SBuf(), false);
3308+};
3309+
3310+inline std::ostream &
3311+operator <<(std::ostream &os, const ParsingBuffer &b)
3312+{
3313+ b.print(os);
3314+ return os;
3315+}
3316+
3317+} // namespace Store
3318+
3319+#endif /* SQUID_SRC_STORE_PARSINGBUFFER_H */
3320+
3321diff --git a/src/store/forward.h b/src/store/forward.h
3322index 1422a85..db5ee1c 100644
3323--- a/src/store/forward.h
3324+++ b/src/store/forward.h
3325@@ -46,6 +46,7 @@ class Disks;
3326 class Disk;
3327 class DiskConfig;
3328 class EntryGuard;
3329+class ParsingBuffer;
3330
3331 typedef ::StoreEntry Entry;
3332 typedef ::MemStore Memory;
3333diff --git a/src/store_client.cc b/src/store_client.cc
3334index 1b54f04..a5f2440 100644
3335--- a/src/store_client.cc
3336+++ b/src/store_client.cc
3337@@ -9,6 +9,7 @@
3338 /* DEBUG: section 90 Storage Manager Client-Side Interface */
3339
3340 #include "squid.h"
3341+#include "base/AsyncCbdataCalls.h"
3342 #include "event.h"
3343 #include "globals.h"
3344 #include "HttpReply.h"
3345@@ -16,8 +17,10 @@
3346 #include "MemBuf.h"
3347 #include "MemObject.h"
3348 #include "mime_header.h"
3349+#include "sbuf/Stream.h"
3350 #include "profiler/Profiler.h"
3351 #include "SquidConfig.h"
3352+#include "SquidMath.h"
3353 #include "StatCounters.h"
3354 #include "Store.h"
3355 #include "store_swapin.h"
3356@@ -39,17 +42,10 @@
3357 static StoreIOState::STRCB storeClientReadBody;
3358 static StoreIOState::STRCB storeClientReadHeader;
3359 static void storeClientCopy2(StoreEntry * e, store_client * sc);
3360-static EVH storeClientCopyEvent;
3361 static bool CheckQuickAbortIsReasonable(StoreEntry * entry);
3362
3363 CBDATA_CLASS_INIT(store_client);
3364
3365-bool
3366-store_client::memReaderHasLowerOffset(int64_t anOffset) const
3367-{
3368- return getType() == STORE_MEM_CLIENT && copyInto.offset < anOffset;
3369-}
3370-
3371 int
3372 store_client::getType() const
3373 {
3374@@ -105,25 +101,35 @@ storeClientListAdd(StoreEntry * e, void *data)
3375 }
3376
3377 void
3378-store_client::callback(ssize_t sz, bool error)
3379+store_client::FinishCallback(store_client * const sc)
3380 {
3381- size_t bSz = 0;
3382+ sc->finishCallback();
3383+}
3384
3385- if (sz >= 0 && !error)
3386- bSz = sz;
3387+void
3388+store_client::finishCallback()
3389+{
3390+ Assure(_callback.callback_handler);
3391+ Assure(_callback.notifier);
3392
3393- StoreIOBuffer result(bSz, 0 ,copyInto.data);
3394+ // XXX: Some legacy code relies on zero-length buffers having nil data
3395+ // pointers. Some other legacy code expects "correct" result.offset even
3396+ // when there is no body to return. Accommodate all those expectations.
3397+ auto result = StoreIOBuffer(0, copyInto.offset, nullptr);
3398+ if (object_ok && parsingBuffer.second && parsingBuffer.first.contentSize())
3399+ result = parsingBuffer.first.packBack();
3400+ result.flags.error = object_ok ? 0 : 1;
3401
3402- if (sz < 0 || error)
3403- result.flags.error = 1;
3404+ // no HTTP headers and no body bytes (but not because there was no space)
3405+ atEof_ = !sendingHttpHeaders() && !result.length && copyInto.length;
3406+
3407+ parsingBuffer.second = false;
3408+ ++answers;
3409
3410- result.offset = cmp_offset;
3411- assert(_callback.pending());
3412- cmp_offset = copyInto.offset + bSz;
3413 STCB *temphandler = _callback.callback_handler;
3414 void *cbdata = _callback.callback_data;
3415- _callback = Callback(NULL, NULL);
3416- copyInto.data = NULL;
3417+ _callback = Callback(nullptr, nullptr);
3418+ copyInto.data = nullptr;
3419
3420 if (cbdataReferenceValid(cbdata))
3421 temphandler(cbdata, result);
3422@@ -131,32 +137,18 @@ store_client::callback(ssize_t sz, bool error)
3423 cbdataReferenceDone(cbdata);
3424 }
3425
3426-static void
3427-storeClientCopyEvent(void *data)
3428-{
3429- store_client *sc = (store_client *)data;
3430- debugs(90, 3, "storeClientCopyEvent: Running");
3431- assert (sc->flags.copy_event_pending);
3432- sc->flags.copy_event_pending = false;
3433-
3434- if (!sc->_callback.pending())
3435- return;
3436-
3437- storeClientCopy2(sc->entry, sc);
3438-}
3439-
3440 store_client::store_client(StoreEntry *e) :
3441- cmp_offset(0),
3442 #if STORE_CLIENT_LIST_DEBUG
3443 owner(cbdataReference(data)),
3444 #endif
3445 entry(e),
3446 type(e->storeClientType()),
3447- object_ok(true)
3448+ object_ok(true),
3449+ atEof_(false),
3450+ answers(0)
3451 {
3452 flags.disk_io_pending = false;
3453 flags.store_copying = false;
3454- flags.copy_event_pending = false;
3455 ++ entry->refcount;
3456
3457 if (getType() == STORE_DISK_CLIENT) {
3458@@ -202,16 +194,33 @@ store_client::copy(StoreEntry * anEntry,
3459 #endif
3460
3461 assert(!_callback.pending());
3462-#if ONLYCONTIGUOUSREQUESTS
3463-
3464- assert(cmp_offset == copyRequest.offset);
3465-#endif
3466- /* range requests will skip into the body */
3467- cmp_offset = copyRequest.offset;
3468 _callback = Callback (callback_fn, cbdataReference(data));
3469 copyInto.data = copyRequest.data;
3470 copyInto.length = copyRequest.length;
3471 copyInto.offset = copyRequest.offset;
3472+ Assure(copyInto.offset >= 0);
3473+
3474+ if (!copyInto.length) {
3475+ // During the first storeClientCopy() call, a zero-size buffer means
3476+ // that we will have to drop any HTTP response body bytes we read (with
3477+ // the HTTP headers from disk). After that, it means we cannot return
3478+ // anything to the caller at all.
3479+ debugs(90, 2, "WARNING: zero-size storeClientCopy() buffer: " << copyInto);
3480+ // keep going; moreToRead() should prevent any from-Store reading
3481+ }
3482+
3483+ // Our nextHttpReadOffset() expects the first copy() call to have zero
3484+ // offset. More complex code could handle a positive first offset, but it
3485+ // would only be useful when reading responses from memory: We would not
3486+ // _delay_ the response (to read the requested HTTP body bytes from disk)
3487+ // when we already can respond with HTTP headers.
3488+ Assure(!copyInto.offset || answeredOnce());
3489+
3490+ parsingBuffer.first = Store::ParsingBuffer(copyInto);
3491+ parsingBuffer.second = true;
3492+
3493+ discardableHttpEnd_ = nextHttpReadOffset();
3494+ debugs(90, 7, "discardableHttpEnd_=" << discardableHttpEnd_);
3495
3496 static bool copying (false);
3497 assert (!copying);
3498@@ -239,50 +248,41 @@ store_client::copy(StoreEntry * anEntry,
3499 // Add no code here. This object may no longer exist.
3500 }
3501
3502-/// Whether there is (or will be) more entry data for us.
3503+/// Whether Store has (or possibly will have) more entry data for us.
3504 bool
3505-store_client::moreToSend() const
3506+store_client::moreToRead() const
3507 {
3508+ if (!copyInto.length)
3509+ return false; // the client supplied a zero-size buffer
3510+
3511 if (entry->store_status == STORE_PENDING)
3512 return true; // there may be more coming
3513
3514 /* STORE_OK, including aborted entries: no more data is coming */
3515
3516- const int64_t len = entry->objectLen();
3517+ if (canReadFromMemory())
3518+ return true; // memory has the first byte wanted by the client
3519
3520- // If we do not know the entry length, then we have to open the swap file.
3521- const bool canSwapIn = entry->hasDisk();
3522- if (len < 0)
3523- return canSwapIn;
3524+ if (!entry->hasDisk())
3525+ return false; // cannot read anything from disk either
3526
3527- if (copyInto.offset >= len)
3528- return false; // sent everything there is
3529+ if (entry->objectLen() >= 0 && copyInto.offset >= entry->contentLen())
3530+ return false; // the disk cannot have byte(s) wanted by the client
3531
3532- if (canSwapIn)
3533- return true; // if we lack prefix, we can swap it in
3534-
3535- // If we cannot swap in, make sure we have what we want in RAM. Otherwise,
3536- // scheduleRead calls scheduleDiskRead which asserts without a swap file.
3537- const MemObject *mem = entry->mem_obj;
3538- return mem &&
3539- mem->inmem_lo <= copyInto.offset && copyInto.offset < mem->endOffset();
3540+ // we cannot be sure until we swap in metadata and learn contentLen(),
3541+ // but the disk may have the byte(s) wanted by the client
3542+ return true;
3543 }
3544
3545 static void
3546 storeClientCopy2(StoreEntry * e, store_client * sc)
3547 {
3548 /* reentrancy not allowed - note this could lead to
3549- * dropped events
3550+ * dropped notifications about response data availability
3551 */
3552
3553- if (sc->flags.copy_event_pending) {
3554- return;
3555- }
3556-
3557 if (sc->flags.store_copying) {
3558- sc->flags.copy_event_pending = true;
3559- debugs(90, 3, "storeClientCopy2: Queueing storeClientCopyEvent()");
3560- eventAdd("storeClientCopyEvent", storeClientCopyEvent, sc, 0.0, 0);
3561+ debugs(90, 3, "prevented recursive copying for " << *e);
3562 return;
3563 }
3564
3565@@ -295,39 +295,44 @@ storeClientCopy2(StoreEntry * e, store_client * sc)
3566 * if the peer aborts, we want to give the client(s)
3567 * everything we got before the abort condition occurred.
3568 */
3569- /* Warning: doCopy may indirectly free itself in callbacks,
3570- * hence the lock to keep it active for the duration of
3571- * this function
3572- * XXX: Locking does not prevent calling sc destructor (it only prevents
3573- * freeing sc memory) so sc may become invalid from C++ p.o.v.
3574- */
3575- CbcPointer<store_client> tmpLock = sc;
3576- assert (!sc->flags.store_copying);
3577 sc->doCopy(e);
3578- assert(!sc->flags.store_copying);
3579+}
3580+
3581+/// Whether our answer, if sent right now, will announce the availability of
3582+/// HTTP response headers (to the STCB callback) for the first time.
3583+bool
3584+store_client::sendingHttpHeaders() const
3585+{
3586+ return !answeredOnce() && entry->mem().baseReply().hdr_sz > 0;
3587 }
3588
3589 void
3590 store_client::doCopy(StoreEntry *anEntry)
3591 {
3592+ Assure(_callback.pending());
3593+ Assure(!flags.disk_io_pending);
3594+ Assure(!flags.store_copying);
3595+
3596 assert (anEntry == entry);
3597 flags.store_copying = true;
3598 MemObject *mem = entry->mem_obj;
3599
3600- debugs(33, 5, "store_client::doCopy: co: " <<
3601- copyInto.offset << ", hi: " <<
3602- mem->endOffset());
3603+ debugs(33, 5, this << " into " << copyInto <<
3604+ " hi: " << mem->endOffset() <<
3605+ " objectLen: " << entry->objectLen() <<
3606+ " past_answers: " << answers);
3607
3608- if (!moreToSend()) {
3609+ const auto sendHttpHeaders = sendingHttpHeaders();
3610+
3611+ if (!sendHttpHeaders && !moreToRead()) {
3612 /* There is no more to send! */
3613 debugs(33, 3, HERE << "There is no more to send!");
3614- callback(0);
3615+ noteNews();
3616 flags.store_copying = false;
3617 return;
3618 }
3619
3620- /* Check that we actually have data */
3621- if (anEntry->store_status == STORE_PENDING && copyInto.offset >= mem->endOffset()) {
3622+ if (!sendHttpHeaders && anEntry->store_status == STORE_PENDING && nextHttpReadOffset() >= mem->endOffset()) {
3623 debugs(90, 3, "store_client::doCopy: Waiting for more");
3624 flags.store_copying = false;
3625 return;
3626@@ -349,7 +354,24 @@ store_client::doCopy(StoreEntry *anEntry)
3627 if (!startSwapin())
3628 return; // failure
3629 }
3630- scheduleRead();
3631+
3632+ // send any immediately available body bytes even if we also sendHttpHeaders
3633+ if (canReadFromMemory()) {
3634+ readFromMemory();
3635+ noteNews(); // will sendHttpHeaders (if needed) as well
3636+ flags.store_copying = false;
3637+ return;
3638+ }
3639+
3640+ if (sendHttpHeaders) {
3641+ debugs(33, 5, "just send HTTP headers: " << mem->baseReply().hdr_sz);
3642+ noteNews();
3643+ flags.store_copying = false;
3644+ return;
3645+ }
3646+
3647+ // no information that the client needs is available immediately
3648+ scheduleDiskRead();
3649 }
3650
3651 /// opens the swapin "file" if possible; otherwise, fail()s and returns false
3652@@ -383,14 +405,13 @@ store_client::startSwapin()
3653 }
3654
3655 void
3656-store_client::scheduleRead()
3657+store_client::noteSwapInDone(const bool error)
3658 {
3659- MemObject *mem = entry->mem_obj;
3660-
3661- if (copyInto.offset >= mem->inmem_lo && copyInto.offset < mem->endOffset())
3662- scheduleMemRead();
3663+ Assure(_callback.pending());
3664+ if (error)
3665+ fail();
3666 else
3667- scheduleDiskRead();
3668+ noteNews();
3669 }
3670
3671 void
3672@@ -415,15 +436,44 @@ store_client::scheduleDiskRead()
3673 flags.store_copying = false;
3674 }
3675
3676+/// whether at least one byte wanted by the client is in memory
3677+bool
3678+store_client::canReadFromMemory() const
3679+{
3680+ const auto &mem = entry->mem();
3681+ const auto memReadOffset = nextHttpReadOffset();
3682+ return mem.inmem_lo <= memReadOffset && memReadOffset < mem.endOffset() &&
3683+ parsingBuffer.first.spaceSize();
3684+}
3685+
3686+/// The offset of the next stored HTTP response byte wanted by the client.
3687+int64_t
3688+store_client::nextHttpReadOffset() const
3689+{
3690+ Assure(parsingBuffer.second);
3691+ const auto &mem = entry->mem();
3692+ const auto hdr_sz = mem.baseReply().hdr_sz;
3693+ // Certain SMP cache manager transactions do not store HTTP headers in
3694+ // mem_hdr; they store just a kid-specific piece of the future report body.
3695+ // In such cases, hdr_sz ought to be zero. In all other (known) cases,
3696+ // mem_hdr contains HTTP response headers (positive hdr_sz if parsed)
3697+ // followed by HTTP response body. This code math accommodates all cases.
3698+ return NaturalSum<int64_t>(hdr_sz, copyInto.offset, parsingBuffer.first.contentSize()).first;
3699+}
3700+
3701+/// Copies at least some of the requested body bytes from MemObject memory,
3702+/// satisfying the copy() request.
3703+/// \pre canReadFromMemory() is true
3704 void
3705-store_client::scheduleMemRead()
3706+store_client::readFromMemory()
3707 {
3708- /* What the client wants is in memory */
3709- /* Old style */
3710- debugs(90, 3, "store_client::doCopy: Copying normal from memory");
3711- size_t sz = entry->mem_obj->data_hdr.copy(copyInto);
3712- callback(sz);
3713- flags.store_copying = false;
3714+ Assure(parsingBuffer.second);
3715+ const auto readInto = parsingBuffer.first.space().positionAt(nextHttpReadOffset());
3716+
3717+ debugs(90, 3, "copying HTTP body bytes from memory into " << readInto);
3718+ const auto sz = entry->mem_obj->data_hdr.copy(readInto);
3719+ Assure(sz > 0); // our canReadFromMemory() precondition guarantees that
3720+ parsingBuffer.first.appended(readInto.data, sz);
3721 }
3722
3723 void
3724@@ -435,65 +485,150 @@ store_client::fileRead()
3725 assert(!flags.disk_io_pending);
3726 flags.disk_io_pending = true;
3727
3728+ // mem->swap_hdr_sz is zero here during initial read(s)
3729+ const auto nextStoreReadOffset = NaturalSum<int64_t>(mem->swap_hdr_sz, nextHttpReadOffset()).first;
3730+
3731+ // XXX: If fileRead() is called when we do not yet know mem->swap_hdr_sz,
3732+ // then we must start reading from disk offset zero to learn it: we cannot
3733+ // compute correct HTTP response start offset on disk without it. However,
3734+ // late startSwapin() calls imply that the assertion below might fail.
3735+ Assure(mem->swap_hdr_sz > 0 || !nextStoreReadOffset);
3736+
3737+ // TODO: Remove this assertion. Introduced in 1998 commit 3157c72, it
3738+ // assumes that swapped out memory is freed unconditionally, but we no
3739+ // longer do that because trimMemory() path checks lowestMemReaderOffset().
3740+ // It is also misplaced: We are not swapping out anything here and should
3741+ // not care about any swapout invariants.
3742 if (mem->swap_hdr_sz != 0)
3743 if (entry->swappingOut())
3744- assert(mem->swapout.sio->offset() > copyInto.offset + (int64_t)mem->swap_hdr_sz);
3745+ assert(mem->swapout.sio->offset() > nextStoreReadOffset);
3746+
3747+ // XXX: We should let individual cache_dirs limit the read size instead, but
3748+ // we cannot do that without more fixes and research because:
3749+ // * larger reads corrupt responses when cache_dir uses SharedMemory::get();
3750+ // * we do not know how to find all I/O code that assumes this limit;
3751+ // * performance effects of larger disk reads may be negative somewhere.
3752+ const decltype(StoreIOBuffer::length) maxReadSize = SM_PAGE_SIZE;
3753+
3754+ Assure(parsingBuffer.second);
3755+ // also, do not read more than we can return (via a copyInto.length buffer)
3756+ const auto readSize = std::min(copyInto.length, maxReadSize);
3757+ lastDiskRead = parsingBuffer.first.makeSpace(readSize).positionAt(nextStoreReadOffset);
3758+ debugs(90, 5, "into " << lastDiskRead);
3759
3760 storeRead(swapin_sio,
3761- copyInto.data,
3762- copyInto.length,
3763- copyInto.offset + mem->swap_hdr_sz,
3764+ lastDiskRead.data,
3765+ lastDiskRead.length,
3766+ lastDiskRead.offset,
3767 mem->swap_hdr_sz == 0 ? storeClientReadHeader
3768 : storeClientReadBody,
3769 this);
3770 }
3771
3772 void
3773-store_client::readBody(const char *, ssize_t len)
3774+store_client::readBody(const char * const buf, const ssize_t lastIoResult)
3775 {
3776- int parsed_header = 0;
3777-
3778- // Don't assert disk_io_pending here.. may be called by read_header
3779+ Assure(flags.disk_io_pending);
3780 flags.disk_io_pending = false;
3781 assert(_callback.pending());
3782- debugs(90, 3, "storeClientReadBody: len " << len << "");
3783+ Assure(parsingBuffer.second);
3784+ debugs(90, 3, "got " << lastIoResult << " using " << parsingBuffer.first);
3785
3786- if (len < 0)
3787+ if (lastIoResult < 0)
3788 return fail();
3789
3790- if (copyInto.offset == 0 && len > 0 && entry->getReply()->sline.status() == Http::scNone) {
3791- /* Our structure ! */
3792- HttpReply *rep = (HttpReply *) entry->getReply(); // bypass const
3793+ if (!lastIoResult) {
3794+ if (answeredOnce())
3795+ return noteNews();
3796
3797- if (!rep->parseCharBuf(copyInto.data, headersEnd(copyInto.data, len))) {
3798- debugs(90, DBG_CRITICAL, "Could not parse headers from on disk object");
3799- } else {
3800- parsed_header = 1;
3801- }
3802+ debugs(90, DBG_CRITICAL, "ERROR: Truncated HTTP headers in on-disk object");
3803+ return fail();
3804 }
3805
3806- const HttpReply *rep = entry->getReply();
3807- if (len > 0 && rep && entry->mem_obj->inmem_lo == 0 && entry->objectLen() <= (int64_t)Config.Store.maxInMemObjSize && Config.onoff.memory_cache_disk) {
3808- storeGetMemSpace(len);
3809- // The above may start to free our object so we need to check again
3810+ assert(lastDiskRead.data == buf);
3811+ lastDiskRead.length = lastIoResult;
3812+
3813+ parsingBuffer.first.appended(buf, lastIoResult);
3814+
3815+ // we know swap_hdr_sz by now and were reading beyond swap metadata because
3816+ // readHead() would have been called otherwise (to read swap metadata)
3817+ const auto swap_hdr_sz = entry->mem().swap_hdr_sz;
3818+ Assure(swap_hdr_sz > 0);
3819+ Assure(!Less(lastDiskRead.offset, swap_hdr_sz));
3820+
3821+ // Map lastDiskRead (i.e. the disk area we just read) to an HTTP reply part.
3822+ // The bytes are the same, but disk and HTTP offsets differ by swap_hdr_sz.
3823+ const auto httpOffset = lastDiskRead.offset - swap_hdr_sz;
3824+ const auto httpPart = StoreIOBuffer(lastDiskRead).positionAt(httpOffset);
3825+
3826+ maybeWriteFromDiskToMemory(httpPart);
3827+ handleBodyFromDisk();
3828+}
3829+
3830+/// de-serializes HTTP response (partially) read from disk storage
3831+void
3832+store_client::handleBodyFromDisk()
3833+{
3834+ // We cannot de-serialize on-disk HTTP response without MemObject because
3835+ // without MemObject::swap_hdr_sz we cannot know where that response starts.
3836+ Assure(entry->mem_obj);
3837+ Assure(entry->mem_obj->swap_hdr_sz > 0);
3838+
3839+ if (!answeredOnce()) {
3840+ // All on-disk responses have HTTP headers. First disk body read(s)
3841+ // include HTTP headers that we must parse (if needed) and skip.
3842+ const auto haveHttpHeaders = entry->mem_obj->baseReply().pstate == psParsed;
3843+ if (!haveHttpHeaders && !parseHttpHeadersFromDisk())
3844+ return;
3845+ skipHttpHeadersFromDisk();
3846+ }
3847+
3848+ noteNews();
3849+}
3850+
3851+/// Adds HTTP response data loaded from disk to the memory cache (if
3852+/// needed/possible). The given part may contain portions of HTTP response
3853+/// headers and/or HTTP response body.
3854+void
3855+store_client::maybeWriteFromDiskToMemory(const StoreIOBuffer &httpResponsePart)
3856+{
3857+ // XXX: Reject [memory-]uncachable/unshareable responses instead of assuming
3858+ // that an HTTP response should be written to MemObject's data_hdr (and that
3859+ // it may purge already cached entries) just because it "fits" and was
3860+ // loaded from disk. For example, this response may already be marked for
3861+ // release. The (complex) cachability decision(s) should be made outside
3862+ // (and obeyed by) this low-level code.
3863+ if (httpResponsePart.length && entry->mem_obj->inmem_lo == 0 && entry->objectLen() <= (int64_t)Config.Store.maxInMemObjSize && Config.onoff.memory_cache_disk) {
3864+ storeGetMemSpace(httpResponsePart.length);
3865+ // XXX: This "recheck" is not needed because storeGetMemSpace() cannot
3866+ // purge mem_hdr bytes of a locked entry, and we do lock ours. And
3867+ // inmem_lo offset itself should not be relevant to appending new bytes.
3868+ //
3869+ // recheck for the above call may purge entry's data from the memory cache
3870 if (entry->mem_obj->inmem_lo == 0) {
3871- /* Copy read data back into memory.
3872- * copyInto.offset includes headers, which is what mem cache needs
3873- */
3874- int64_t mem_offset = entry->mem_obj->endOffset();
3875- if ((copyInto.offset == mem_offset) || (parsed_header && mem_offset == rep->hdr_sz)) {
3876- entry->mem_obj->write(StoreIOBuffer(len, copyInto.offset, copyInto.data));
3877- }
3878+ // XXX: This code assumes a non-shared memory cache.
3879+ if (httpResponsePart.offset == entry->mem_obj->endOffset())
3880+ entry->mem_obj->write(httpResponsePart);
3881 }
3882 }
3883-
3884- callback(len);
3885 }
3886
3887 void
3888 store_client::fail()
3889 {
3890+ debugs(90, 3, (object_ok ? "once" : "again"));
3891+ if (!object_ok)
3892+ return; // we failed earlier; nothing to do now
3893+
3894 object_ok = false;
3895+
3896+ noteNews();
3897+}
3898+
3899+/// if necessary and possible, informs the Store reader about copy() result
3900+void
3901+store_client::noteNews()
3902+{
3903 /* synchronous open failures callback from the store,
3904 * before startSwapin detects the failure.
3905 * TODO: fix this inconsistent behaviour - probably by
3906@@ -501,8 +636,20 @@ store_client::fail()
3907 * not synchronous
3908 */
3909
3910- if (_callback.pending())
3911- callback(0, true);
3912+ if (!_callback.callback_handler) {
3913+ debugs(90, 5, "client lost interest");
3914+ return;
3915+ }
3916+
3917+ if (_callback.notifier) {
3918+ debugs(90, 5, "earlier news is being delivered by " << _callback.notifier);
3919+ return;
3920+ }
3921+
3922+ _callback.notifier = asyncCall(90, 4, "store_client::FinishCallback", cbdataDialer(store_client::FinishCallback, this));
3923+ ScheduleCallHere(_callback.notifier);
3924+
3925+ Assure(!_callback.pending());
3926 }
3927
3928 static void
3929@@ -573,38 +720,22 @@ store_client::readHeader(char const *buf, ssize_t len)
3930 if (!object_ok)
3931 return;
3932
3933+ Assure(parsingBuffer.second);
3934+ debugs(90, 3, "got " << len << " using " << parsingBuffer.first);
3935+
3936 if (len < 0)
3937 return fail();
3938
3939+ Assure(!parsingBuffer.first.contentSize());
3940+ parsingBuffer.first.appended(buf, len);
3941 if (!unpackHeader(buf, len)) {
3942 fail();
3943 return;
3944 }
3945+ parsingBuffer.first.consume(mem->swap_hdr_sz);
3946
3947- /*
3948- * If our last read got some data the client wants, then give
3949- * it to them, otherwise schedule another read.
3950- */
3951- size_t body_sz = len - mem->swap_hdr_sz;
3952-
3953- if (copyInto.offset < static_cast<int64_t>(body_sz)) {
3954- /*
3955- * we have (part of) what they want
3956- */
3957- size_t copy_sz = min(copyInto.length, body_sz);
3958- debugs(90, 3, "storeClientReadHeader: copying " << copy_sz << " bytes of body");
3959- memmove(copyInto.data, copyInto.data + mem->swap_hdr_sz, copy_sz);
3960-
3961- readBody(copyInto.data, copy_sz);
3962-
3963- return;
3964- }
3965-
3966- /*
3967- * we don't have what the client wants, but at least we now
3968- * know the swap header size.
3969- */
3970- fileRead();
3971+ maybeWriteFromDiskToMemory(parsingBuffer.first.content());
3972+ handleBodyFromDisk();
3973 }
3974
3975 int
3976@@ -673,10 +804,12 @@ storeUnregister(store_client * sc, StoreEntry * e, void *data)
3977 ++statCounter.swap.ins;
3978 }
3979
3980- if (sc->_callback.pending()) {
3981- /* callback with ssize = -1 to indicate unexpected termination */
3982- debugs(90, 3, "store_client for " << *e << " has a callback");
3983- sc->fail();
3984+ if (sc->_callback.callback_handler || sc->_callback.notifier) {
3985+ debugs(90, 3, "forgetting store_client callback for " << *e);
3986+ // Do not notify: Callers want to stop copying and forget about this
3987+ // pending copy request. Some would mishandle a notification from here.
3988+ if (sc->_callback.notifier)
3989+ sc->_callback.notifier->cancel("storeUnregister");
3990 }
3991
3992 #if STORE_CLIENT_LIST_DEBUG
3993@@ -684,6 +817,8 @@ storeUnregister(store_client * sc, StoreEntry * e, void *data)
3994
3995 #endif
3996
3997+ // XXX: We might be inside sc store_client method somewhere up the call
3998+ // stack. TODO: Convert store_client to AsyncJob to make destruction async.
3999 delete sc;
4000
4001 assert(e->locked());
4002@@ -740,6 +875,9 @@ StoreEntry::invokeHandlers()
4003
4004 if (sc->flags.disk_io_pending)
4005 continue;
4006+
4007+ if (sc->flags.store_copying)
4008+ continue;
4009
4010 storeClientCopy2(this, sc);
4011 }
4012@@ -847,6 +985,63 @@ CheckQuickAbortIsReasonable(StoreEntry * entry)
4013 return true;
4014 }
4015
4016+/// parses HTTP header bytes loaded from disk
4017+/// \returns false if fail() or scheduleDiskRead() has been called and, hence,
4018+/// the caller should just quit without any further action
4019+bool
4020+store_client::parseHttpHeadersFromDisk()
4021+{
4022+ try {
4023+ return tryParsingHttpHeaders();
4024+ } catch (...) {
4025+ // XXX: Our parser enforces Config.maxReplyHeaderSize limit, but our
4026+ // packer does not. Since packing might increase header size, we may
4027+ // cache a header that we cannot parse and get here. Same for MemStore.
4028+ debugs(90, DBG_CRITICAL, "ERROR: Cannot parse on-disk HTTP headers" <<
4029+ Debug::Extra << "exception: " << CurrentException <<
4030+ Debug::Extra << "raw input size: " << parsingBuffer.first.contentSize() << " bytes" <<
4031+ Debug::Extra << "current buffer capacity: " << parsingBuffer.first.capacity() << " bytes");
4032+ fail();
4033+ return false;
4034+ }
4035+}
4036+
4037+/// parseHttpHeadersFromDisk() helper
4038+/// \copydoc parseHttpHeaders()
4039+bool
4040+store_client::tryParsingHttpHeaders()
4041+{
4042+ Assure(parsingBuffer.second);
4043+ Assure(!copyInto.offset); // otherwise, parsingBuffer cannot have HTTP response headers
4044+ auto &adjustableReply = entry->mem().baseReply();
4045+ if (adjustableReply.parseTerminatedPrefix(parsingBuffer.first.c_str(), parsingBuffer.first.contentSize()))
4046+ return true;
4047+
4048+ // TODO: Optimize by checking memory as well. For simplicity sake, we
4049+ // continue on the disk-reading path, but readFromMemory() can give us the
4050+ // missing header bytes immediately if a concurrent request put those bytes
4051+ // into memory while we were waiting for our disk response.
4052+ scheduleDiskRead();
4053+ return false;
4054+}
4055+
4056+/// skips HTTP header bytes previously loaded from disk
4057+void
4058+store_client::skipHttpHeadersFromDisk()
4059+{
4060+ const auto hdr_sz = entry->mem_obj->baseReply().hdr_sz;
4061+ Assure(hdr_sz > 0); // all on-disk responses have HTTP headers
4062+ if (Less(parsingBuffer.first.contentSize(), hdr_sz)) {
4063+ debugs(90, 5, "discovered " << hdr_sz << "-byte HTTP headers in memory after reading some of them from disk: " << parsingBuffer.first);
4064+ parsingBuffer.first.consume(parsingBuffer.first.contentSize()); // skip loaded HTTP header prefix
4065+ } else {
4066+ parsingBuffer.first.consume(hdr_sz); // skip loaded HTTP headers
4067+ const auto httpBodyBytesAfterHeader = parsingBuffer.first.contentSize(); // may be zero
4068+ Assure(httpBodyBytesAfterHeader <= copyInto.length);
4069+ debugs(90, 5, "read HTTP body prefix: " << httpBodyBytesAfterHeader);
4070+ }
4071+}
4072+
4073 void
4074 store_client::dumpStats(MemBuf * output, int clientNumber) const
4075 {
4076@@ -864,8 +1059,8 @@ store_client::dumpStats(MemBuf * output, int clientNumber) const
4077 if (flags.store_copying)
4078 output->append(" store_copying", 14);
4079
4080- if (flags.copy_event_pending)
4081- output->append(" copy_event_pending", 19);
4082+ if (_callback.notifier)
4083+ output->append(" notifying", 10);
4084
4085 output->append("\n",1);
4086 }
4087@@ -873,12 +1068,19 @@ store_client::dumpStats(MemBuf * output, int clientNumber) const
4088 bool
4089 store_client::Callback::pending() const
4090 {
4091- return callback_handler && callback_data;
4092+ return callback_handler && !notifier;
4093 }
4094
4095 store_client::Callback::Callback(STCB *function, void *data) : callback_handler(function), callback_data (data) {}
4096
4097 #if USE_DELAY_POOLS
4098+int
4099+store_client::bytesWanted() const
4100+{
4101+ // TODO: To avoid using stale copyInto, return zero if !_callback.pending()?
4102+ return delayId.bytesWanted(0, copyInto.length);
4103+}
4104+
4105 void
4106 store_client::setDelayId(DelayId delay_id)
4107 {
4108diff --git a/src/store_swapin.cc b/src/store_swapin.cc
4109index a05d7e3..cd32e94 100644
4110--- a/src/store_swapin.cc
4111+++ b/src/store_swapin.cc
4112@@ -56,7 +56,7 @@ storeSwapInFileClosed(void *data, int errflag, StoreIOState::Pointer)
4113
4114 if (sc->_callback.pending()) {
4115 assert (errflag <= 0);
4116- sc->callback(0, errflag ? true : false);
4117+ sc->noteSwapInDone(errflag);
4118 }
4119
4120 ++statCounter.swap.ins;
4121diff --git a/src/tests/stub_HttpReply.cc b/src/tests/stub_HttpReply.cc
4122index 8ca7f9e..5cde8e6 100644
4123--- a/src/tests/stub_HttpReply.cc
4124+++ b/src/tests/stub_HttpReply.cc
4125@@ -25,6 +25,7 @@ void httpBodyPackInto(const HttpBody *, Packable *) STUB
4126 bool HttpReply::sanityCheckStartLine(const char *buf, const size_t hdr_len, Http::StatusCode *error) STUB_RETVAL(false)
4127 int HttpReply::httpMsgParseError() STUB_RETVAL(0)
4128 bool HttpReply::expectingBody(const HttpRequestMethod&, int64_t&) const STUB_RETVAL(false)
4129+size_t HttpReply::parseTerminatedPrefix(const char *, size_t) STUB_RETVAL(0)
4130 bool HttpReply::parseFirstLine(const char *start, const char *end) STUB_RETVAL(false)
4131 void HttpReply::hdrCacheInit() STUB
4132 HttpReply * HttpReply::clone() const STUB_RETVAL(NULL)
4133diff --git a/src/tests/stub_store_client.cc b/src/tests/stub_store_client.cc
4134index 2a13874..debe24e 100644
4135--- a/src/tests/stub_store_client.cc
4136+++ b/src/tests/stub_store_client.cc
4137@@ -34,7 +34,12 @@ void storeLogOpen(void) STUB
4138 void storeDigestInit(void) STUB
4139 void storeRebuildStart(void) STUB
4140 void storeReplSetup(void) STUB
4141-bool store_client::memReaderHasLowerOffset(int64_t anOffset) const STUB_RETVAL(false)
4142 void store_client::dumpStats(MemBuf * output, int clientNumber) const STUB
4143 int store_client::getType() const STUB_RETVAL(0)
4144+void store_client::noteSwapInDone(bool) STUB
4145+#if USE_DELAY_POOLS
4146+int store_client::bytesWanted() const STUB_RETVAL(0)
4147+#endif
4148+
4149+
4150
4151diff --git a/src/urn.cc b/src/urn.cc
4152index 74453e1..6efdec1 100644
4153--- a/src/urn.cc
4154+++ b/src/urn.cc
4155@@ -26,8 +26,6 @@
4156 #include "tools.h"
4157 #include "urn.h"
4158
4159-#define URN_REQBUF_SZ 4096
4160-
4161 class UrnState : public StoreClient
4162 {
4163 CBDATA_CLASS(UrnState);
4164@@ -45,8 +43,8 @@ public:
4165 HttpRequest::Pointer request;
4166 HttpRequest::Pointer urlres_r;
4167
4168- char reqbuf[URN_REQBUF_SZ] = { '\0' };
4169- int reqofs = 0;
4170+ /// for receiving a URN resolver reply body from Store and interpreting it
4171+ Store::ParsingBuffer parsingBuffer;
4172
4173 private:
4174 char *urlres;
4175@@ -63,7 +61,7 @@ typedef struct {
4176 } url_entry;
4177
4178 static STCB urnHandleReply;
4179-static url_entry *urnParseReply(const char *inbuf, const HttpRequestMethod&);
4180+static url_entry *urnParseReply(const SBuf &, const HttpRequestMethod &);
4181 static const char *const crlf = "\r\n";
4182
4183 CBDATA_CLASS_INIT(UrnState);
4184@@ -183,13 +181,8 @@ UrnState::created(StoreEntry *newEntry)
4185 sc = storeClientListAdd(urlres_e, this);
4186 }
4187
4188- reqofs = 0;
4189- StoreIOBuffer tempBuffer;
4190- tempBuffer.offset = reqofs;
4191- tempBuffer.length = URN_REQBUF_SZ;
4192- tempBuffer.data = reqbuf;
4193 storeClientCopy(sc, urlres_e,
4194- tempBuffer,
4195+ parsingBuffer.makeInitialSpace(),
4196 urnHandleReply,
4197 this);
4198 }
4199@@ -224,9 +217,6 @@ urnHandleReply(void *data, StoreIOBuffer result)
4200 UrnState *urnState = static_cast<UrnState *>(data);
4201 StoreEntry *e = urnState->entry;
4202 StoreEntry *urlres_e = urnState->urlres_e;
4203- char *s = NULL;
4204- size_t k;
4205- HttpReply *rep;
4206 url_entry *urls;
4207 url_entry *u;
4208 url_entry *min_u;
4209@@ -234,10 +224,7 @@ urnHandleReply(void *data, StoreIOBuffer result)
4210 ErrorState *err;
4211 int i;
4212 int urlcnt = 0;
4213- char *buf = urnState->reqbuf;
4214- StoreIOBuffer tempBuffer;
4215-
4216- debugs(52, 3, "urnHandleReply: Called with size=" << result.length << ".");
4217+ debugs(52, 3, result << " with " << *e);
4218
4219 if (EBIT_TEST(urlres_e->flags, ENTRY_ABORTED) || result.flags.error) {
4220 delete urnState;
4221@@ -250,59 +237,39 @@ urnHandleReply(void *data, StoreIOBuffer result)
4222 return;
4223 }
4224
4225- /* Update reqofs to point to where in the buffer we'd be */
4226- urnState->reqofs += result.length;
4227-
4228- /* Handle reqofs being bigger than normal */
4229- if (urnState->reqofs >= URN_REQBUF_SZ) {
4230- delete urnState;
4231- return;
4232- }
4233+ urnState->parsingBuffer.appended(result.data, result.length);
4234
4235 /* If we haven't received the entire object (urn), copy more */
4236- if (urlres_e->store_status == STORE_PENDING) {
4237- Must(result.length > 0); // zero length ought to imply STORE_OK
4238- tempBuffer.offset = urnState->reqofs;
4239- tempBuffer.length = URN_REQBUF_SZ - urnState->reqofs;
4240- tempBuffer.data = urnState->reqbuf + urnState->reqofs;
4241+ if (!urnState->sc->atEof()) {
4242+ const auto bufferedBytes = urnState->parsingBuffer.contentSize();
4243+ const auto remainingSpace = urnState->parsingBuffer.space().positionAt(bufferedBytes);
4244+
4245+ if (!remainingSpace.length) {
4246+ debugs(52, 3, "ran out of buffer space after " << bufferedBytes << " bytes");
4247+ // TODO: Here and in other error cases, send ERR_URN_RESOLVE to client.
4248+ delete urnState;
4249+ return;
4250+ }
4251+
4252 storeClientCopy(urnState->sc, urlres_e,
4253- tempBuffer,
4254+ remainingSpace,
4255 urnHandleReply,
4256 urnState);
4257 return;
4258 }
4259
4260- /* we know its STORE_OK */
4261- k = headersEnd(buf, urnState->reqofs);
4262-
4263- if (0 == k) {
4264- debugs(52, DBG_IMPORTANT, "urnHandleReply: didn't find end-of-headers for " << e->url() );
4265- delete urnState;
4266- return;
4267- }
4268-
4269- s = buf + k;
4270- assert(urlres_e->getReply());
4271- rep = new HttpReply;
4272- rep->parseCharBuf(buf, k);
4273- debugs(52, 3, "reply exists, code=" << rep->sline.status() << ".");
4274-
4275- if (rep->sline.status() != Http::scOkay) {
4276+ const auto &peerReply = urlres_e->mem().baseReply();
4277+ debugs(52, 3, "got reply, code=" << peerReply.sline.status());
4278+ if (peerReply.sline.status() != Http::scOkay) {
4279 debugs(52, 3, "urnHandleReply: failed.");
4280 err = new ErrorState(ERR_URN_RESOLVE, Http::scNotFound, urnState->request.getRaw());
4281 err->url = xstrdup(e->url());
4282 errorAppendEntry(e, err);
4283- delete rep;
4284 delete urnState;
4285 return;
4286 }
4287
4288- delete rep;
4289-
4290- while (xisspace(*s))
4291- ++s;
4292-
4293- urls = urnParseReply(s, urnState->request->method);
4294+ urls = urnParseReply(urnState->parsingBuffer.toSBuf(), urnState->request->method);
4295
4296 if (!urls) { /* unknown URN error */
4297 debugs(52, 3, "urnTranslateDone: unknown URN " << e->url());
4298@@ -350,7 +317,7 @@ urnHandleReply(void *data, StoreIOBuffer result)
4299 "Generated by %s@%s\n"
4300 "</ADDRESS>\n",
4301 APP_FULLNAME, getMyHostname());
4302- rep = new HttpReply;
4303+ const auto rep = new HttpReply;
4304 rep->setHeaders(Http::scFound, NULL, "text/html", mb->contentSize(), 0, squid_curtime);
4305
4306 if (min_u) {
4307@@ -372,9 +339,8 @@ urnHandleReply(void *data, StoreIOBuffer result)
4308 }
4309
4310 static url_entry *
4311-urnParseReply(const char *inbuf, const HttpRequestMethod& m)
4312+urnParseReply(const SBuf &inBuf, const HttpRequestMethod &m)
4313 {
4314- char *buf = xstrdup(inbuf);
4315 char *token;
4316 url_entry *list;
4317 url_entry *old;
4318@@ -383,6 +349,13 @@ urnParseReply(const char *inbuf, const HttpRequestMethod& m)
4319 debugs(52, 3, "urnParseReply");
4320 list = (url_entry *)xcalloc(n + 1, sizeof(*list));
4321
4322+ // XXX: Switch to tokenizer-based parsing.
4323+ const auto allocated = SBufToCstring(inBuf);
4324+
4325+ auto buf = allocated;
4326+ while (xisspace(*buf))
4327+ ++buf;
4328+
4329 for (token = strtok(buf, crlf); token; token = strtok(NULL, crlf)) {
4330 debugs(52, 3, "urnParseReply: got '" << token << "'");
4331
4332@@ -418,7 +391,7 @@ urnParseReply(const char *inbuf, const HttpRequestMethod& m)
4333 }
4334
4335 debugs(52, 3, "urnParseReply: Found " << i << " URLs");
4336- xfree(buf);
4337+ xfree(allocated);
4338 return list;
4339 }
4340
diff --git a/meta-networking/recipes-daemons/squid/squid_4.15.bb b/meta-networking/recipes-daemons/squid/squid_4.15.bb
index 69b62aa5a5..a042f57166 100644
--- a/meta-networking/recipes-daemons/squid/squid_4.15.bb
+++ b/meta-networking/recipes-daemons/squid/squid_4.15.bb
@@ -32,6 +32,7 @@ SRC_URI = "http://www.squid-cache.org/Versions/v${MAJ_VER}/${BPN}-${PV}.tar.bz2
32 file://CVE-2023-46846.patch \ 32 file://CVE-2023-46846.patch \
33 file://CVE-2023-49286.patch \ 33 file://CVE-2023-49286.patch \
34 file://CVE-2023-50269.patch \ 34 file://CVE-2023-50269.patch \
35 file://CVE-2023-5824.patch \
35 " 36 "
36 37
37SRC_URI:remove:toolchain-clang = "file://0001-configure-Check-for-Wno-error-format-truncation-comp.patch" 38SRC_URI:remove:toolchain-clang = "file://0001-configure-Check-for-Wno-error-format-truncation-comp.patch"
generated by cgit v1.2.3-54-g00ecf (git 2.39.0) at 2025-07-26 00:29:33 +0000