This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko-http.git
The following commit(s) were added to refs/heads/main by this push:
new 9bbb67cb8 give ParsingErrorHandler access to the request that was
rejected (#1245) (#1246)
9bbb67cb8 is described below
commit 9bbb67cb8a8b9d395308b9c54b060c8da4e15624
Author: PJ Fanning <[email protected]>
AuthorDate: Fri Sep 11 09:57:16 2026 +0100
give ParsingErrorHandler access to the request that was rejected (#1245)
(#1246)
`ParsingErrorHandler` could not see the request it was rejecting: none of
its four arguments describe the method or the request target, and
`ParserOutput.MessageStartError` discarded both at the point where the
parser gave up.
Add `IllegalRequestContext` and a five-argument `handle` overload that
receives it. The overload defaults to the existing four-argument method,
so existing handlers, `DefaultParsingErrorHandler` included, keep working
unchanged.
The parser populates the context at the failure site: its `method`,
`uri` and `uriBytes` fields are reused across a keep-alive connection, so
they are cleared for every message to stop a rejection from reporting the
previous request's values.
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../illegal-request-context.excludes | 25 ++++++
.../apache/pekko/http/ParsingErrorHandler.scala | 98 +++++++++++++++++++++-
.../client/OutgoingConnectionBlueprint.scala | 2 +-
.../impl/engine/parsing/HttpMessageParser.scala | 26 +++++-
.../impl/engine/parsing/HttpRequestParser.scala | 17 ++++
.../http/impl/engine/parsing/ParserOutput.scala | 6 +-
.../impl/engine/server/HttpServerBluePrint.scala | 18 ++--
.../impl/engine/ws/WebSocketClientBlueprint.scala | 2 +-
.../impl/engine/parsing/RequestParserSpec.scala | 49 ++++++++++-
.../impl/engine/parsing/ResponseParserSpec.scala | 2 +-
.../http/impl/engine/server/HttpServerSpec.scala | 69 ++++++++++++++-
11 files changed, 295 insertions(+), 19 deletions(-)
diff --git
a/http-core/src/main/mima-filters/2.0.x.backwards.excludes/illegal-request-context.excludes
b/http-core/src/main/mima-filters/2.0.x.backwards.excludes/illegal-request-context.excludes
new file mode 100644
index 000000000..a7049a8bf
--- /dev/null
+++
b/http-core/src/main/mima-filters/2.0.x.backwards.excludes/illegal-request-context.excludes
@@ -0,0 +1,25 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# internal API: MessageStartError carries what is known about the request that
was rejected
+ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.parsing.ParserOutput#MessageStartError.copy")
+ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.parsing.ParserOutput#MessageStartError.this")
+ProblemFilters.exclude[MissingTypesProblem]("org.apache.pekko.http.impl.engine.parsing.ParserOutput$MessageStartError$")
+ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.parsing.ParserOutput#MessageStartError.apply")
+ProblemFilters.exclude[IncompatibleSignatureProblem]("org.apache.pekko.http.impl.engine.parsing.ParserOutput#MessageStartError.unapply")
+
+# internal API: replaced by an instance level completion handling that can
report the same context
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/ParsingErrorHandler.scala
b/http-core/src/main/scala/org/apache/pekko/http/ParsingErrorHandler.scala
index 6bd426cc0..472519d29 100644
--- a/http-core/src/main/scala/org/apache/pekko/http/ParsingErrorHandler.scala
+++ b/http-core/src/main/scala/org/apache/pekko/http/ParsingErrorHandler.scala
@@ -13,19 +13,115 @@
package org.apache.pekko.http
+import java.util.Optional
+
+import scala.jdk.OptionConverters._
+
import org.apache.pekko
import pekko.event.LoggingAdapter
import pekko.http.javadsl.{ model => jm }
-import pekko.http.scaladsl.model.{ ErrorInfo, HttpResponse, StatusCode }
+import pekko.http.scaladsl.model.{ ErrorInfo, HttpMethod, HttpProtocol,
HttpResponse, StatusCode }
import pekko.http.scaladsl.settings.ServerSettings
+/**
+ * What is known about a request that failed to parse, at the point where
parsing gave up.
+ *
+ * Every field is optional because a request can be rejected before that part
of it has been read:
+ * a request with an unsupported method fails before the request target is
seen, and one with an
+ * unparsable request target fails before the protocol is seen.
+ *
+ * Note that `rawRequestTarget` is unvalidated, attacker-controlled input, by
definition malformed
+ * whenever the rejection was caused by the request target itself. Anything
that logs or echoes it
+ * has to escape it.
+ *
+ * @since 2.0.0
+ */
+final class IllegalRequestContext private[http] (
+ val method: Option[HttpMethod],
+ val rawRequestTarget: Option[String],
+ val protocol: Option[HttpProtocol]) {
+
+ /**
+ * Java API
+ *
+ * @since 2.0.0
+ */
+ def getMethod: Optional[jm.HttpMethod] = method.map(m => m:
jm.HttpMethod).toJava
+
+ /**
+ * Java API
+ *
+ * @since 2.0.0
+ */
+ def getRawRequestTarget: Optional[String] = rawRequestTarget.toJava
+
+ /**
+ * Java API
+ *
+ * @since 2.0.0
+ */
+ def getProtocol: Optional[jm.HttpProtocol] = protocol.map(p => p:
jm.HttpProtocol).toJava
+
+ override def toString: String =
+ s"IllegalRequestContext(${method.map(_.value).getOrElse("-")}," +
+
s"${rawRequestTarget.getOrElse("-")},${protocol.map(_.value).getOrElse("-")})"
+}
+
+object IllegalRequestContext {
+
+ /**
+ * A context that knows nothing about the request, used when no information
could be recovered.
+ *
+ * @since 2.0.0
+ */
+ val empty: IllegalRequestContext = new IllegalRequestContext(None, None,
None)
+
+ private[http] def apply(
+ method: Option[HttpMethod],
+ rawRequestTarget: Option[String],
+ protocol: Option[HttpProtocol]): IllegalRequestContext =
+ if (method.isEmpty && rawRequestTarget.isEmpty && protocol.isEmpty) empty
+ else new IllegalRequestContext(method, rawRequestTarget, protocol)
+}
+
+/**
+ * Produces the response to a request that failed to parse. Selected by the
+ * `pekko.http.server.parsing-error-handler` setting.
+ *
+ * This is also the earliest public symbol that observes a rejected request,
so it is what
+ * observability tooling attaches to: the OpenTelemetry Java agent instruments
`handle` to emit a
+ * span for a request that never reaches the route handler
+ * (open-telemetry/opentelemetry-java-instrumentation#5139).
+ */
abstract class ParsingErrorHandler {
def handle(status: StatusCode, error: ErrorInfo, log: LoggingAdapter,
settings: ServerSettings): jm.HttpResponse
+
+ /**
+ * Called by the server for a request that failed to parse, with what is
known about that request.
+ *
+ * The default implementation ignores `context` and delegates to the
four-argument `handle`, so
+ * existing handlers keep working unchanged; override this method instead to
make use of the
+ * context. The parameter is still worth passing for a handler that does not
read it, because the
+ * arguments of this method are visible to anything instrumenting it: the
OpenTelemetry Java agent
+ * has to name the span it emits for a rejected request `HTTP` and report
neither
+ * `http.request.method` nor `url.path`, since the four-argument signature
describes only the
+ * failure and never the request that caused it. See
+ * [[https://github.com/apache/pekko-http/issues/1245]].
+ *
+ * Note that `DefaultParsingErrorHandler` deliberately keeps implementing
the four-argument method
+ * rather than this one, so that advice matching that signature keeps firing.
+ *
+ * @since 2.0.0
+ */
+ def handle(status: StatusCode, error: ErrorInfo, log: LoggingAdapter,
settings: ServerSettings,
+ context: IllegalRequestContext): jm.HttpResponse =
+ handle(status, error, log, settings)
}
object DefaultParsingErrorHandler extends ParsingErrorHandler {
import pekko.http.impl.engine.parsing.logParsingError
+ // implements the four-argument method on purpose, see the scaladoc of the
five-argument one
override def handle(
status: StatusCode, info: ErrorInfo, log: LoggingAdapter, settings:
ServerSettings): HttpResponse = {
logParsingError(
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/client/OutgoingConnectionBlueprint.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/client/OutgoingConnectionBlueprint.scala
index 6672d7446..b0839294d 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/client/OutgoingConnectionBlueprint.scala
+++
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/client/OutgoingConnectionBlueprint.scala
@@ -201,7 +201,7 @@ private[http] object OutgoingConnectionBlueprint {
push(httpResponseOut, new HttpResponse(statusCode, headers,
attributes, entity, protocol))
completeOnMessageEnd = closeRequested
- case MessageStartError(_, info) =>
+ case MessageStartError(_, info, _) =>
throw IllegalResponseException(info)
case other =>
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 ac468de2b..fa9961286 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
@@ -22,6 +22,7 @@ import org.parboiled2.CharUtils
import org.apache.pekko
import pekko.annotation.InternalApi
+import pekko.http.IllegalRequestContext
import pekko.http.impl.model.parser.CharacterClasses
import pekko.http.impl.util.HttpConstants._
import pekko.http.scaladsl.model.{ ParsingException => _, _ }
@@ -62,6 +63,25 @@ private[http] trait HttpMessageParser[Output >:
MessageOutput <: ParserOutput] {
/** invoked if the specified protocol is unknown */
protected def onBadProtocol(input: ByteString): Nothing
+
+ /**
+ * What is known about the message that is currently being parsed, at the
point where parsing failed.
+ * Only the request parser has anything to report here.
+ */
+ protected def illegalRequestContext: IllegalRequestContext =
IllegalRequestContext.empty
+
+ /** The protocol of the message that is currently being parsed */
+ protected final def currentProtocol: HttpProtocol = protocol
+
+ /**
+ * Completion handling for a message start that was truncated by the
connection closing, reporting
+ * what the parser had already read of that message.
+ */
+ protected final val completionIsMessageStartError: CompletionHandling =
+ () =>
+ Some(MessageStartError(StatusCodes.BadRequest, ErrorInfo("Illegal HTTP
message start"),
+ illegalRequestContext))
+
protected def parseMessage(input: ByteString, offset: Int):
HttpMessageParser.StateResult
protected def parseEntity(headers: List[HttpHeader], protocol: HttpProtocol,
input: ByteString, bodyStart: Int,
clh: Option[`Content-Length`], cth: Option[`Content-Type`], isChunked:
Boolean,
@@ -122,7 +142,7 @@ private[http] trait HttpMessageParser[Output >:
MessageOutput <: ParserOutput] {
}
protected final def startNewMessage(input: ByteString, offset: Int):
StateResult = {
- if (offset < input.length)
setCompletionHandling(CompletionIsMessageStartError)
+ if (offset < input.length)
setCompletionHandling(completionIsMessageStartError)
try parseMessage(input, offset)
catch { case NotEnoughDataException => continue(input,
offset)(startNewMessage) }
}
@@ -382,7 +402,7 @@ private[http] trait HttpMessageParser[Output >:
MessageOutput <: ParserOutput] {
protected final def failMessageStart(status: StatusCode, summary: String,
detail: String = ""): StateResult =
failMessageStart(status, ErrorInfo(summary, detail))
protected final def failMessageStart(status: StatusCode, info: ErrorInfo):
StateResult = {
- emit(MessageStartError(status, info))
+ emit(MessageStartError(status, info, illegalRequestContext))
setCompletionHandling(CompletionOk)
terminate()
}
@@ -452,8 +472,6 @@ private[http] object HttpMessageParser {
type CompletionHandling = () => Option[ErrorOutput]
val CompletionOk: CompletionHandling = () => None
- val CompletionIsMessageStartError: CompletionHandling =
- () => Some(ParserOutput.MessageStartError(StatusCodes.BadRequest,
ErrorInfo("Illegal HTTP message start")))
val CompletionIsEntityStreamError: CompletionHandling =
() =>
Some(ParserOutput.EntityStreamError(ErrorInfo(
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpRequestParser.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpRequestParser.scala
index a826738cd..e43fe3954 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpRequestParser.scala
+++
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpRequestParser.scala
@@ -14,12 +14,14 @@
package org.apache.pekko.http.impl.engine.parsing
import java.lang.{ StringBuilder => JStringBuilder }
+import java.nio.charset.StandardCharsets
import javax.net.ssl.SSLSession
import scala.annotation.{ switch, tailrec }
import org.apache.pekko
import pekko.annotation.InternalApi
+import pekko.http.IllegalRequestContext
import pekko.http.impl.engine.server.HttpAttributes
import pekko.http.impl.util.ByteStringParserInput
import pekko.http.impl.util.HttpConstants._
@@ -69,6 +71,8 @@ private[http] final class HttpRequestParser(
private var method: HttpMethod = null
private var uri: Uri = null
private var uriBytes: ByteString = null
+ // whether `protocol` of the underlying parser belongs to the message
currently being parsed
+ private var protocolParsed: Boolean = false
override def onPush(): Unit =
handleParserOutput(parseSessionBytes(grab(in)))
override def onPull(): Unit = handleParserOutput(doPull())
@@ -89,9 +93,16 @@ private[http] final class HttpRequestParser(
override def parseMessage(input: ByteString, offset: Int): StateResult =
if (offset < input.length) {
+ // the fields below are reused for every message on a connection,
forget what the previous
+ // one left behind so that a failure cannot report values belonging
to another request
+ method = null
+ uri = null
+ uriBytes = null
+ protocolParsed = false
var cursor = parseMethod(input, offset)
cursor = parseRequestTarget(input, cursor)
cursor = parseProtocol(input, cursor)
+ protocolParsed = true
if (byteAt(input, cursor) == CR_BYTE && byteAt(input, cursor + 1) ==
LF_BYTE)
parseHeaderLines(input, cursor + 2)
else if (byteAt(input, cursor) == LF_BYTE)
@@ -252,6 +263,12 @@ private[http] final class HttpRequestParser(
}
} else failMessageStart("Request is missing required `Host` header")
+ override protected def illegalRequestContext: IllegalRequestContext =
+ IllegalRequestContext(
+ Option(method),
+ Option.unless(uriBytes eq
null)(uriBytes.decodeString(StandardCharsets.US_ASCII)),
+ Option.when(protocolParsed)(currentProtocol))
+
private def remoteAddressStr: String =
inheritedAttributes.get[HttpAttributes.RemoteAddress].map(_.address)
match {
case Some(addr) => s" from ${addr.getHostString}:${addr.getPort}"
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/ParserOutput.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/ParserOutput.scala
index 6582407de..60ceeb6d3 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/ParserOutput.scala
+++
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/ParserOutput.scala
@@ -16,6 +16,7 @@ package org.apache.pekko.http.impl.engine.parsing
import org.apache.pekko
import pekko.NotUsed
import pekko.annotation.InternalApi
+import pekko.http.IllegalRequestContext
import pekko.http.impl.util.StreamUtils
import pekko.http.scaladsl.model._
import pekko.stream.scaladsl.Source
@@ -62,7 +63,10 @@ private[http] object ParserOutput {
final case class EntityChunk(chunk: HttpEntity.ChunkStreamPart) extends
MessageOutput
- final case class MessageStartError(status: StatusCode, info: ErrorInfo)
extends MessageStart with ErrorOutput
+ final case class MessageStartError(
+ status: StatusCode,
+ info: ErrorInfo,
+ context: IllegalRequestContext = IllegalRequestContext.empty) extends
MessageStart with ErrorOutput
final case class EntityStreamError(info: ErrorInfo) extends ErrorOutput
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/server/HttpServerBluePrint.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/server/HttpServerBluePrint.scala
index 4c20f5bd1..9a8c48795 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/server/HttpServerBluePrint.scala
+++
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/server/HttpServerBluePrint.scala
@@ -26,7 +26,7 @@ import pekko.stream._
import pekko.stream.TLSProtocol._
import pekko.stream.scaladsl._
import pekko.stream.stage._
-import pekko.http.ParsingErrorHandler
+import pekko.http.{ IllegalRequestContext, ParsingErrorHandler }
import pekko.http.scaladsl.settings.ServerSettings
import pekko.http.impl.engine.parsing.ParserOutput._
import pekko.http.impl.engine.parsing._
@@ -268,14 +268,15 @@ private[http] object HttpServerBluePrint {
def establishAbsoluteUri(requestOutput: RequestOutput): RequestOutput =
requestOutput match {
case connect: RequestStart if connect.method == HttpMethods.CONNECT =>
MessageStartError(StatusCodes.BadRequest,
- ErrorInfo(s"CONNECT requests are not supported", s"Rejecting CONNECT
request to '${connect.uri}'"))
+ ErrorInfo(s"CONNECT requests are not supported", s"Rejecting CONNECT
request to '${connect.uri}'"),
+ contextOf(connect))
case start: RequestStart =>
try {
val effectiveUri = HttpRequest.effectiveUri(start.uri,
start.headers, isSecureConnection, defaultHostHeader)
start.copy(uri = effectiveUri)
} catch {
case e: IllegalUriException =>
- MessageStartError(StatusCodes.BadRequest, e.info)
+ MessageStartError(StatusCodes.BadRequest, e.info, contextOf(start))
}
case x => x
}
@@ -283,6 +284,9 @@ private[http] object HttpServerBluePrint {
Flow[SessionBytes].via(rootParser).map(establishAbsoluteUri)
}
+ private def contextOf(start: RequestStart): IllegalRequestContext =
+ IllegalRequestContext(Some(start.method), Some(start.uri.toString),
Some(start.protocol))
+
def rendering(settings: ServerSettings, log: LoggingAdapter,
dateHeaderRendering: DateHeaderRendering)
: Flow[ResponseRenderingContext, ResponseRenderingOutput, NotUsed] = {
import settings._
@@ -482,7 +486,8 @@ private[http] object HttpServerBluePrint {
case MessageEnd =>
messageEndPending = false
push(requestPrepOut, MessageEnd)
- case MessageStartError(status, info)
=> finishWithIllegalRequestError(status, info)
+ case MessageStartError(status, info, context) =>
+ finishWithIllegalRequestError(status, info, context)
case x: EntityStreamError if messageEndPending &&
openRequests.isEmpty =>
// client terminated the connection after receiving an early
response to 100-continue
completeStage()
@@ -587,8 +592,9 @@ private[http] object HttpServerBluePrint {
}
})
- def finishWithIllegalRequestError(status: StatusCode, info:
ErrorInfo): Unit = {
- val errorResponse =
JavaMapping.toScala(parsingErrorHandler.handle(status, info, log, settings))
+ def finishWithIllegalRequestError(status: StatusCode, info: ErrorInfo,
+ context: IllegalRequestContext = IllegalRequestContext.empty):
Unit = {
+ val errorResponse =
JavaMapping.toScala(parsingErrorHandler.handle(status, info, log, settings,
context))
emitErrorResponse(errorResponse)
}
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/ws/WebSocketClientBlueprint.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/ws/WebSocketClientBlueprint.scala
index 34e34b349..34b4e97d6 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/ws/WebSocketClientBlueprint.scala
+++
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/ws/WebSocketClientBlueprint.scala
@@ -139,7 +139,7 @@ private[http] object WebSocketClientBlueprint {
result.success(InvalidUpgradeResponse(response,
s"WebSocket server at $uri returned $problem"))
failStage(new IllegalArgumentException(s"WebSocket upgrade
did not finish because of '$problem'"))
}
- case MessageStartError(statusCode, errorInfo) =>
+ case MessageStartError(statusCode, errorInfo, _) =>
throw new IllegalStateException(s"Message failed with status
code $statusCode; Error info: $errorInfo")
case other =>
throw new IllegalStateException(s"unexpected element of type
${other.getClass}")
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 af81c84c0..9e85e2342 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
@@ -26,6 +26,7 @@ import pekko.stream.TLSProtocol._
import org.scalatest.matchers.Matcher
import org.scalatest.BeforeAndAfterAll
import pekko.http.scaladsl.settings.{ ParserSettings, WebSocketSettings }
+import pekko.http.IllegalRequestContext
import pekko.http.impl.engine.parsing.ParserOutput._
import pekko.http.impl.settings.WebSocketSettingsImpl
import pekko.http.impl.util._
@@ -684,7 +685,7 @@ abstract class RequestParserSpec(mode: String, newLine:
String) extends AnyFreeS
val result = multiParse(newParser)(Seq("GET /\u0000HTTP/1.1
HTTP/1.1\r\n"))
result.length shouldEqual 1
result.head match {
- case Left(MessageStartError(BadRequest, info)) =>
+ case Left(MessageStartError(BadRequest, info, _)) =>
info.summary should startWith("Illegal request-target")
case other => fail(s"Expected BadRequest MessageStartError but got
$other")
}
@@ -862,6 +863,31 @@ abstract class RequestParserSpec(mode: String, newLine:
String) extends AnyFreeS
|""" should parseToError(BadRequest, ErrorInfo("A chunked request
must not contain a Content-Length header"))
}
}
+
+ "report what it knows about a rejected request" - {
+ "the method and the raw request target when the request target is
illegal" in new Test {
+ illegalRequestContextOf("GET /%% HTTP/1.1\r\n") should be(
+ (Some(GET), Some("/%%"), None))
+ }
+
+ "the method, the raw request target and the protocol when the request
line parsed" in new Test {
+ illegalRequestContextOf("GET /abc HTTP/1.1\r\n\r\n") should be(
+ (Some(GET), Some("/abc"), Some(`HTTP/1.1`)))
+ }
+
+ "nothing when the request failed before the method was known" in new
Test {
+ illegalRequestContextOf("BLAH /abc HTTP/1.1\r\n") should be((None,
None, None))
+ }
+
+ "nothing from the previous request on the same connection" in new Test {
+ // the parser reuses its fields for every message, a rejection must
not report the previous request
+ multiParse(newParser)(Seq("GET /previous HTTP/1.1\r\nHost: x\r\n\r\n",
"BLAH /abc HTTP/1.1\r\n")) match {
+ case Seq(Right(_), Left(MessageStartError(_, _, context))) =>
+ (context.method, context.rawRequestTarget, context.protocol)
should be((None, None, None))
+ case other => fail(s"Expected a request followed by a
MessageStartError but got $other")
+ }
+ }
+ }
}
override def afterAll() = TestKit.shutdownActorSystem(system)
@@ -881,6 +907,14 @@ abstract class RequestParserSpec(mode: String, newLine:
String) extends AnyFreeS
override def toString = req.toString
}
+ /** The context of the single `MessageStartError` that parsing `input` is
expected to produce */
+ def illegalRequestContextOf(input: String): (Option[HttpMethod],
Option[String], Option[HttpProtocol]) =
+ multiParse(newParser)(Seq(input)) match {
+ case Seq(Left(MessageStartError(_, _, context))) =>
+ (context.method, context.rawRequestTarget, context.protocol)
+ case other => fail(s"Expected a single MessageStartError but got
$other")
+ }
+
def strictEqualify[T](x: Either[T, HttpRequest]): Either[T,
StrictEqualHttpRequest] =
x.map(new StrictEqualHttpRequest(_))
@@ -908,7 +942,16 @@ abstract class RequestParserSpec(mode: String, newLine:
String) extends AnyFreeS
parser: HttpRequestParser,
expected: Either[RequestOutput, HttpRequest]*): Matcher[Seq[String]] =
equal(expected.map(strictEqualify))
- .matcher[Seq[Either[RequestOutput,
StrictEqualHttpRequest]]].compose(multiParse(parser))
+ .matcher[Seq[Either[RequestOutput, StrictEqualHttpRequest]]]
+ // the illegal request context is asserted separately, it is not part
of what these expectations describe
+ .compose(multiParse(parser)(_).map(withoutIllegalRequestContext))
+
+ def withoutIllegalRequestContext(
+ output: Either[RequestOutput, StrictEqualHttpRequest]):
Either[RequestOutput, StrictEqualHttpRequest] =
+ output match {
+ case Left(error: MessageStartError) => Left(error.copy(context =
IllegalRequestContext.empty))
+ case other => other
+ }
def multiParse(parser: HttpRequestParser)(input: Seq[String]):
Seq[Either[RequestOutput, StrictEqualHttpRequest]] =
Source(input.toList)
@@ -920,7 +963,7 @@ abstract class RequestParserSpec(mode: String, newLine:
String) extends AnyFreeS
case (Seq(RequestStart(method, uri, protocol, attrs, headers,
createEntity, _, close)), entityParts) =>
closeAfterResponseCompletion :+= close
Right(HttpRequest(method, uri, headers, createEntity(entityParts),
protocol))
- case (Seq(x @ (MessageStartError(_, _) | EntityStreamError(_))),
rest) =>
+ case (Seq(x @ (MessageStartError(_, _, _) | EntityStreamError(_))),
rest) =>
rest.runWith(Sink.cancelled)
Left(x)
}
diff --git
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/ResponseParserSpec.scala
b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/ResponseParserSpec.scala
index cf0ef5acb..46cbbd507 100644
---
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/ResponseParserSpec.scala
+++
b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/ResponseParserSpec.scala
@@ -446,7 +446,7 @@ abstract class ResponseParserSpec(mode: String, newLine:
String) extends PekkoSp
case (Seq(ResponseStart(statusCode, protocol, attributes, headers,
createEntity, close)), entityParts) =>
closeAfterResponseCompletion :+= close
Right(new HttpResponse(statusCode, headers, attributes,
createEntity(entityParts), protocol))
- case (Seq(x @ (MessageStartError(_, _) | EntityStreamError(_))),
tail) =>
+ case (Seq(x @ (MessageStartError(_, _, _) | EntityStreamError(_))),
tail) =>
tail.runWith(Sink.ignore)
Left(x)
}.concatSubstreams
diff --git
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/server/HttpServerSpec.scala
b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/server/HttpServerSpec.scala
index 6d73f2967..e92cde84c 100644
---
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/server/HttpServerSpec.scala
+++
b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/server/HttpServerSpec.scala
@@ -16,7 +16,7 @@ package org.apache.pekko.http.impl.engine.server
import org.apache.pekko
import pekko.actor.ActorSystem
import pekko.event.LoggingAdapter
-import pekko.http.ParsingErrorHandler
+import pekko.http.{ IllegalRequestContext, ParsingErrorHandler }
import pekko.http.impl.engine.ws.ByteStringSinkProbe
import pekko.http.impl.util._
import pekko.http.scaladsl.Http.ServerLayer
@@ -48,6 +48,20 @@ object TestParsingErrorHandler extends ParsingErrorHandler {
HttpResponse(StatusCodes.ImATeapot, entity = HttpEntity("Tea hea"))
}
+/** Renders what the handler was told about the request that was rejected */
+object ContextReportingParsingErrorHandler extends ParsingErrorHandler {
+ override def handle(
+ status: StatusCode, error: ErrorInfo, log: LoggingAdapter, settings:
ServerSettings): HttpResponse =
+ HttpResponse(status, entity = HttpEntity("no context"))
+
+ override def handle(status: StatusCode, error: ErrorInfo, log:
LoggingAdapter, settings: ServerSettings,
+ context: IllegalRequestContext): HttpResponse =
+ HttpResponse(status,
+ entity = HttpEntity(
+ s"${context.method.map(_.value).getOrElse("-")}
${context.rawRequestTarget.getOrElse("-")} " +
+ s"${context.protocol.map(_.value).getOrElse("-")}"))
+}
+
class HttpServerSpec extends PekkoSpec(
"""pekko.loggers =
["org.apache.pekko.http.impl.util.SilenceAllTestEventListener"]
pekko.loglevel = DEBUG
@@ -1615,6 +1629,59 @@ class HttpServerSpec extends PekkoSpec(
netIn.sendComplete()
netOut.expectComplete()
})
+
+ "pass the rejected request to the parsing error handler" in
assertAllStagesStopped(new TestSetup {
+ override def settings: ServerSettings =
+ super.settings.withParsingErrorHandler(
+
"org.apache.pekko.http.impl.engine.server.ContextReportingParsingErrorHandler$")
+
+ send("""GET /%% HTTP/1.1
+ |Host: www.example.com
+ |
+ |""")
+
+ requests.request(1)
+
+ expectResponseWithWipedDate(
+ """|HTTP/1.1 400 Bad Request
+ |Server: pekko-http/test
+ |Date: XXXX
+ |Connection: close
+ |Content-Type: text/plain; charset=UTF-8
+ |Content-Length: 9
+ |
+ |GET /%% -""")
+
+ netIn.sendComplete()
+ netOut.expectComplete()
+ })
+
+ "pass the rejected request to the parsing error handler when the Host
header does not match" in
+ assertAllStagesStopped(new TestSetup {
+ override def settings: ServerSettings =
+ super.settings.withParsingErrorHandler(
+
"org.apache.pekko.http.impl.engine.server.ContextReportingParsingErrorHandler$")
+
+ send("""GET http://www.example.com/unparsable HTTP/1.1
+ |Host: www.example.net
+ |
+ |""")
+
+ requests.request(1)
+
+ expectResponseWithWipedDate(
+ """|HTTP/1.1 400 Bad Request
+ |Server: pekko-http/test
+ |Date: XXXX
+ |Connection: close
+ |Content-Type: text/plain; charset=UTF-8
+ |Content-Length: 46
+ |
+ |GET http://www.example.com/unparsable HTTP/1.1""")
+
+ netIn.sendComplete()
+ netOut.expectComplete()
+ })
}
class TestSetup(maxContentLength: Int = -1) extends HttpServerTestSetupBase {
implicit def system: ActorSystem = spec.system
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
