From 91a2b4550ae60f8f23b748f6f305d32e946d5732 Mon Sep 17 00:00:00 2001 From: Cedric Vandendriessche Date: Thu, 28 Mar 2024 11:48:25 +0100 Subject: [PATCH 1/3] Parse entire HTTP chunk size #396 Continue parsing HTTP chunk size up until Int.MaxValue so the actual size of the chunk can be logged if it exceeds the configured max chunk size. --- .../impl/engine/parsing/HttpMessageParser.scala | 9 ++++++--- .../impl/engine/parsing/RequestParserSpec.scala | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala index ea7edccaf..6d919b2de 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala @@ -307,17 +307,20 @@ private[http] trait HttpMessageParser[Output >: MessageOutput <: ParserOutput] { s"HTTP chunk extension length exceeds configured limit of ${settings.maxChunkExtLength} characters") @tailrec def parseSize(cursor: Int, size: Long): StateResult = - if (size <= settings.maxChunkSize) { + if (size <= Int.MaxValue) { byteChar(input, cursor) match { case c if CharacterClasses.HEXDIG(c) => parseSize(cursor + 1, size * 16 + CharUtils.hexValue(c)) - case ';' if cursor > offset => parseChunkExtensions(size.toInt, cursor + 1)() + case c if size > settings.maxChunkSize => + failEntityStream( + s"HTTP chunk of $size bytes exceeds the configured limit of ${settings.maxChunkSize} bytes") + case ';' if cursor > offset => parseChunkExtensions(size.toInt, cursor + 1)() case '\r' if cursor > offset && byteChar(input, cursor + 1) == '\n' => parseChunkBody(size.toInt, "", cursor + 2) case '\n' if cursor > offset => parseChunkBody(size.toInt, "", cursor + 1) case c if CharacterClasses.WSP(c) => parseSize(cursor + 1, size) // illegal according to the spec but can happen, see issue #1812 case c => failEntityStream(s"Illegal character '${escape(c)}' in chunk start") } - } else failEntityStream(s"HTTP chunk size exceeds the configured limit of ${settings.maxChunkSize} bytes") + } else failEntityStream(s"HTTP chunk size exceeds ${Int.MaxValue} bytes") try parseSize(offset, 0) catch { diff --git a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala index 7b6d8079c..dcfde9edd 100644 --- a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala +++ b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala @@ -531,13 +531,23 @@ abstract class RequestParserSpec(mode: String, newLine: String) extends AnyFreeS closeAfterResponseCompletion shouldEqual Seq(false) } - "too-large chunk size" in new Test { + "too-large chunk size (> Int.MaxValue)" in new Test { Seq( start, """1a2b3c4d5e |""") should generalMultiParseTo( Right(baseRequest), - Left(EntityStreamError(ErrorInfo("HTTP chunk size exceeds the configured limit of 1048576 bytes")))) + Left(EntityStreamError(ErrorInfo("HTTP chunk size exceeds 2147483647 bytes")))) + closeAfterResponseCompletion shouldEqual Seq(false) + } + + "too-large chunk size" in new Test { + Seq( + start, + """400000 + |""") should generalMultiParseTo( + Right(baseRequest), + Left(EntityStreamError(ErrorInfo("HTTP chunk of 4194304 bytes exceeds the configured limit of 1048576 bytes")))) closeAfterResponseCompletion shouldEqual Seq(false) } From 8c1c22037e5fe7918af9004ef92195e335996aca Mon Sep 17 00:00:00 2001 From: Cedric Vandendriessche Date: Thu, 28 Mar 2024 15:50:35 +0100 Subject: [PATCH 2/3] Add reference to Integer.MAX_VALUE in the error message --- .../pekko/http/impl/engine/parsing/HttpMessageParser.scala | 2 +- .../pekko/http/impl/engine/parsing/RequestParserSpec.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala index 6d919b2de..374f3c3e2 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala @@ -320,7 +320,7 @@ private[http] trait HttpMessageParser[Output >: MessageOutput <: ParserOutput] { case c if CharacterClasses.WSP(c) => parseSize(cursor + 1, size) // illegal according to the spec but can happen, see issue #1812 case c => failEntityStream(s"Illegal character '${escape(c)}' in chunk start") } - } else failEntityStream(s"HTTP chunk size exceeds ${Int.MaxValue} bytes") + } else failEntityStream(s"HTTP chunk size exceeds Integer.MAX_VALUE (${Int.MaxValue}) bytes") try parseSize(offset, 0) catch { diff --git a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala index dcfde9edd..7f750f7c8 100644 --- a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala +++ b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala @@ -537,7 +537,7 @@ abstract class RequestParserSpec(mode: String, newLine: String) extends AnyFreeS """1a2b3c4d5e |""") should generalMultiParseTo( Right(baseRequest), - Left(EntityStreamError(ErrorInfo("HTTP chunk size exceeds 2147483647 bytes")))) + Left(EntityStreamError(ErrorInfo("HTTP chunk size exceeds Integer.MAX_VALUE (2147483647) bytes")))) closeAfterResponseCompletion shouldEqual Seq(false) } From c0d4775da2c887a69221a132a3eef581fe507ea6 Mon Sep 17 00:00:00 2001 From: Cedric Vandendriessche Date: Thu, 28 Mar 2024 16:04:01 +0100 Subject: [PATCH 3/3] Fix formatting --- .../pekko/http/impl/engine/parsing/RequestParserSpec.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala index 7f750f7c8..fbbbec7ba 100644 --- a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala +++ b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala @@ -547,7 +547,8 @@ abstract class RequestParserSpec(mode: String, newLine: String) extends AnyFreeS """400000 |""") should generalMultiParseTo( Right(baseRequest), - Left(EntityStreamError(ErrorInfo("HTTP chunk of 4194304 bytes exceeds the configured limit of 1048576 bytes")))) + Left( + EntityStreamError(ErrorInfo("HTTP chunk of 4194304 bytes exceeds the configured limit of 1048576 bytes")))) closeAfterResponseCompletion shouldEqual Seq(false) }