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

Prior to 2023 commit 951013d0, 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
26f0a359). This change restores that fix functionality. FWIW, our
research shows that nginx and httpd also accept similar input.

diff --git a/src/http/one/Parser.cc b/src/http/one/Parser.cc
index b1908316a0beaea3570ad63b6318868a9a1d96cb..7403a9163a27ae851f914a64fc6273b12ea59845 100644 (file)
--- a/src/http/one/Parser.cc
+++ b/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);
+}
 

diff --git a/src/http/one/Parser.h b/src/http/one/Parser.h
index d9a0ac8c27331396913405f764604f4689a736a4..49e399de5467a8a0ee535ef2797f277b2dfa9c83 100644 (file)
--- a/src/http/one/Parser.h
+++ b/src/http/one/Parser.h
@@ -164,8 +164,15 @@ private:
 
 /// 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();
 

diff --git a/src/http/one/TeChunkedParser.cc b/src/http/one/TeChunkedParser.cc
index 9cce10fdc916988f7c1238c7915196cfb8c7b1c9..859471b8c777b9b3bd0f32dc4765a102df7a27a9 100644 (file)
--- a/src/http/one/TeChunkedParser.cc
+++ b/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)