Lack of data at EOF is a parsing error, not a "need more" condition.

Branch changes did not distinguish "need more and can expect more" from
"need more but got everything" situations. When one of the inner parsers
that deal with "complete" buffers failed, it would throw
InsufficientInput exception and the higher level parsing code would
interpret that as a sign that more SSL records/fragments are needed,
resulting in stuck transactions instead of parsing errors.

Unlike parsing errors, stuck transactions cannot be bypassed or ignored
so the difference is important.

diff --git a/src/parser/BinaryTokenizer.cc b/src/parser/BinaryTokenizer.cc
index 8ee081657d23c7e0f157ec31f688b1567c269aa5..f6589a81a12caf4178e2f2ab4d28f786bed5b6c4 100644 (file)
--- a/src/parser/BinaryTokenizer.cc
+++ b/src/parser/BinaryTokenizer.cc
@@ -15,17 +15,19 @@ BinaryTokenizer::BinaryTokenizer(): BinaryTokenizer(SBuf())
 {
 }
 
-BinaryTokenizer::BinaryTokenizer(const SBuf &data):
+BinaryTokenizer::BinaryTokenizer(const SBuf &data, const bool expectMore):
     context(""),
     data_(data),
     parsed_(0),
-    syncPoint_(0)
+    syncPoint_(0),
+    expectMore_(expectMore)
 {
 }
 
 /// debugging helper that prints a "standard" debugs() trailer
 #define BinaryTokenizer_tail(size, start) \
-    " occupying " << (size) << " bytes @" << (start) << " in " << this;
+    " occupying " << (size) << " bytes @" << (start) << " in " << this << \
+    (expectMore_ ? ';' : '.');
 
 /// logs and throws if fewer than size octets remain; no other side effects
 void
@@ -34,6 +36,7 @@ BinaryTokenizer::want(uint64_t size, const char *description) const
     if (parsed_ + size > data_.length()) {
         debugs(24, 5, (parsed_ + size - data_.length()) << " more bytes for " <<
                context << description << BinaryTokenizer_tail(size, parsed_));
+        Must(expectMore_); // throw an error on premature input termination
         throw InsufficientInput();
     }
 }
@@ -76,9 +79,9 @@ BinaryTokenizer::octet()
 }
 
 void
-BinaryTokenizer::reset(const SBuf &data)
+BinaryTokenizer::reset(const SBuf &data, const bool expectMore)
 {
-    *this = BinaryTokenizer(data);
+    *this = BinaryTokenizer(data, expectMore);
 }
 
 void

diff --git a/src/parser/BinaryTokenizer.h b/src/parser/BinaryTokenizer.h
index 08af24607ae34cba27e558f20f79c282218d4734..58016767fc3756b8f7f26ffbdec4b53ba8887867 100644 (file)
--- a/src/parser/BinaryTokenizer.h
+++ b/src/parser/BinaryTokenizer.h
@@ -22,15 +22,15 @@ public:
     typedef uint64_t size_type; // enough for the largest supported offset
 
     BinaryTokenizer();
-    explicit BinaryTokenizer(const SBuf &data);
+    explicit BinaryTokenizer(const SBuf &data, const bool expectMore = false);
 
     /// restart parsing from the very beginning
     /// this method is for using one BinaryTokenizer to parse independent inputs
-    void reset(const SBuf &data);
+    void reset(const SBuf &data, const bool expectMore);
 
-    /// change input without changing parsing state
+    /// change input state without changing parsing state
     /// this method avoids append overheads during incremental parsing
-    void reinput(const SBuf &data) { data_ = data; }
+    void reinput(const SBuf &data, const bool expectMore) { data_ = data; expectMore_ = expectMore; }
 
     /// make progress: future parsing failures will not rollback beyond this point
     void commit();
@@ -75,6 +75,7 @@ private:
     SBuf data_;
     uint64_t parsed_; ///< number of data bytes parsed or skipped
     uint64_t syncPoint_; ///< where to re-start the next parsing attempt
+    bool expectMore_; ///< whether more data bytes may arrive in the future
 };
 
 #endif // SQUID_BINARY_TOKENIZER_H

diff --git a/src/security/Handshake.cc b/src/security/Handshake.cc
index 34487b0aeaf38be263122c32e30a01fc53b99c33..b493078b323ea19b08be28f16190c95e0d2e58f0 100644 (file)
--- a/src/security/Handshake.cc
+++ b/src/security/Handshake.cc
@@ -197,11 +197,11 @@ Security::HandshakeParser::parseModernRecord()
     if (currentContentType != record.type) {
         Must(tkMessages.atEnd()); // no currentContentType leftovers
         fragments = record.fragment;
-        tkMessages.reset(fragments);
+        tkMessages.reset(fragments, true); // true because more fragments may come
         currentContentType = record.type;
     } else {
         fragments.append(record.fragment);
-        tkMessages.reinput(fragments);
+        tkMessages.reinput(fragments, true); // true because more fragments may come
         tkMessages.rollback();
     }
     parseMessages();
@@ -437,7 +437,8 @@ bool
 Security::HandshakeParser::parseServerHello(const SBuf &data)
 {
     try {
-        tkRecords.reinput(data); // data contains _everything_ read so far
+        // data contains everything read so far, but we may read more later
+        tkRecords.reinput(data, true);
         tkRecords.rollback();
         while (!tkRecords.atEnd() && !parseDone)
             parseRecord();
@@ -459,7 +460,8 @@ bool
 Security::HandshakeParser::parseClientHello(const SBuf &data)
 {
     try {
-        tkRecords.reinput(data); // data contains _everything_ read so far
+        // data contains everything read so far, but we may read more later
+        tkRecords.reinput(data, true);
         tkRecords.rollback();
         while (!tkRecords.atEnd() && !parseDone)
             parseRecord();