Bug 5449: Ignore SP and HTAB chars after chunk-size (#1914)

Prior to 2023 commit 951013d, Squid accepted Transfer-Encoding chunks
with chunk-size followed by spaces or tabs (before CRLF). This HTTP
syntax violation was allowed to address Bug 4492 (fixed in 2017 commit
26f0a35). This change restores that fix functionality. FWIW, our
research shows that nginx and httpd also accept similar input.

src/http/one/Parser.cc

@@ -271,11 +271,12 @@ Http::One::ErrorLevel()
     return Config.onoff.relaxed_header_parser < 0 ? DBG_IMPORTANT : 5;
 }
 
-// BWS = *( SP / HTAB ) ; WhitespaceCharacters() may relax this RFC 7230 rule
-void
-Http::One::ParseBws(Parser::Tokenizer &tok)
+/// common part of ParseBws() and ParseStrctBws()
+namespace Http::One {
+static void
+ParseBws_(Parser::Tokenizer &tok, const CharacterSet &bwsChars)
 {
-    const auto count = tok.skipAll(Parser::WhitespaceCharacters());
+    const auto count = tok.skipAll(bwsChars);
 
     if (tok.atEnd())
         throw InsufficientInput(); // even if count is positive
@@ -290,4 +291,17 @@ Http::One::ParseBws(Parser::Tokenizer &tok)
 
     // success: no more BWS characters expected
 }
+} // namespace Http::One
+
+void
+Http::One::ParseBws(Parser::Tokenizer &tok)
+{
+    ParseBws_(tok, Parser::WhitespaceCharacters());
+}
+
+void
+Http::One::ParseStrictBws(Parser::Tokenizer &tok)
+{
+    ParseBws_(tok, CharacterSet::WSP);
+}
 

src/http/one/Parser.h

@@ -164,8 +164,15 @@ class Parser : public RefCountable
 
 /// skips and, if needed, warns about RFC 7230 BWS ("bad" whitespace)
 /// \throws InsufficientInput when the end of BWS cannot be confirmed
+/// \sa WhitespaceCharacters() for the definition of BWS characters
+/// \sa ParseStrictBws() that avoids WhitespaceCharacters() uncertainties
 void ParseBws(Parser::Tokenizer &);
 
+/// Like ParseBws() but only skips CharacterSet::WSP characters. This variation
+/// must be used if the next element may start with CR or any other character
+/// from RelaxedDelimiterCharacters().
+void ParseStrictBws(Parser::Tokenizer &);
+
 /// the right debugs() level for logging HTTP violation messages
 int ErrorLevel();
 

src/http/one/TeChunkedParser.cc

@@ -125,6 +125,10 @@ Http::One::TeChunkedParser::parseChunkMetadataSuffix(Tokenizer &tok)
     // Code becomes much simpler when incremental parsing functions throw on
     // bad or insufficient input, like in the code below. TODO: Expand up.
     try {
+        // Bug 4492: IBM_HTTP_Server sends SP after chunk-size.
+        // No ParseBws() here because it may consume CR required further below.
+        ParseStrictBws(tok);
+
         parseChunkExtensions(tok); // a possibly empty chunk-ext list
         tok.skipRequired("CRLF after [chunk-ext]", Http1::CrLf());
         buf_ = tok.remaining();
@@ -145,7 +149,7 @@ Http::One::TeChunkedParser::parseChunkExtensions(Tokenizer &callerTok)
     do {
         auto tok = callerTok;
 
-        ParseBws(tok); // Bug 4492: IBM_HTTP_Server sends SP after chunk-size
+        ParseBws(tok);
 
         if (!tok.skip(';'))
             return; // reached the end of extensions (if any)