From c76d1cc93e7446ba8241f931ab5c13095dc01b22 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Feb 2026 12:31:04 +0000 Subject: [PATCH 1/6] Initial plan From adf7c86e15c5314b5197429647b86a93c19f0d24 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Feb 2026 12:42:12 +0000 Subject: [PATCH 2/6] Port akka-http PR #4227: fix invalid HTTP2 request headers leading to bad request responses Co-authored-by: pjfanning <11783444+pjfanning@users.noreply.github.com> --- ...d-header-http2-response.backwards.excludes | 6 + .../http/impl/engine/http2/FrameEvent.scala | 6 +- .../http/impl/engine/http2/FrameLogger.scala | 2 +- .../impl/engine/http2/Http2Blueprint.scala | 16 +- .../engine/http2/Http2StreamHandling.scala | 4 +- .../engine/http2/HttpMessageRendering.scala | 4 +- .../impl/engine/http2/RequestErrorFlow.scala | 79 +++++++ .../impl/engine/http2/RequestParsing.scala | 62 +++-- .../engine/http2/client/ResponseParsing.scala | 16 +- .../http2/hpack/HeaderCompression.scala | 2 +- .../http2/hpack/HeaderDecompression.scala | 8 +- .../http2/hpack/Http2HeaderParsing.scala | 11 +- .../engine/http2/Http2ClientServerSpec.scala | 12 + .../engine/http2/Http2ServerDemuxSpec.scala | 2 +- .../engine/http2/RequestParsingSpec.scala | 216 ++++++++++-------- 15 files changed, 310 insertions(+), 136 deletions(-) create mode 100644 http-core/src/main/mima-filters/1.2.x.backwards.excludes/4226-bad-header-http2-response.backwards.excludes create mode 100644 http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala diff --git a/http-core/src/main/mima-filters/1.2.x.backwards.excludes/4226-bad-header-http2-response.backwards.excludes b/http-core/src/main/mima-filters/1.2.x.backwards.excludes/4226-bad-header-http2-response.backwards.excludes new file mode 100644 index 0000000000..b974d145f6 --- /dev/null +++ b/http-core/src/main/mima-filters/1.2.x.backwards.excludes/4226-bad-header-http2-response.backwards.excludes @@ -0,0 +1,6 @@ +# internals only +ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent#ParsedHeadersFrame.copy") +ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent#ParsedHeadersFrame.this") +ProblemFilters.exclude[MissingTypesProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent$ParsedHeadersFrame$") +ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent#ParsedHeadersFrame.apply") +ProblemFilters.exclude[IncompatibleSignatureProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent#ParsedHeadersFrame.unapply") diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/FrameEvent.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/FrameEvent.scala index 6c93916b41..9888ee0ea6 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/FrameEvent.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/FrameEvent.scala @@ -18,6 +18,7 @@ import pekko.annotation.InternalApi import pekko.http.impl.engine.http2.Http2Protocol.ErrorCode import pekko.http.impl.engine.http2.Http2Protocol.FrameType import pekko.http.impl.engine.http2.Http2Protocol.SettingIdentifier +import pekko.http.scaladsl.model.ErrorInfo import pekko.util.ByteString import scala.collection.immutable @@ -112,11 +113,14 @@ private[http] object FrameEvent { /** * Convenience (logical) representation of a parsed HEADERS frame with zero, one or * many CONTINUATIONS Frames into a single, decompressed object. + * + * @param headerParseErrorDetails Only used server side, passes header errors from decompression into error response logic */ final case class ParsedHeadersFrame( streamId: Int, endStream: Boolean, keyValuePairs: Seq[(String, AnyRef)], - priorityInfo: Option[PriorityFrame]) extends StreamFrameEvent + priorityInfo: Option[PriorityFrame], + headerParseErrorDetails: Option[ErrorInfo]) extends StreamFrameEvent } diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/FrameLogger.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/FrameLogger.scala index 418cb3ac54..00caa04853 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/FrameLogger.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/FrameLogger.scala @@ -79,7 +79,7 @@ private[http2] object FrameLogger { case GoAwayFrame(lastStreamId, errorCode, debug) => LogEntry(0, "GOAY", s"lastStreamId = $lastStreamId, errorCode = $errorCode, debug = ${debug.utf8String}") - case ParsedHeadersFrame(streamId, endStream, kvPairs, prio) => + case ParsedHeadersFrame(streamId, endStream, kvPairs, prio, _) => val prioInfo = if (prio.isDefined) display(entryForFrame(prio.get)) + " " else "" val kvInfo = kvPairs.map { case (key, value) => s"$key -> $value" diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/Http2Blueprint.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/Http2Blueprint.scala index 3b14c0b86b..576faa00d2 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/Http2Blueprint.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/Http2Blueprint.scala @@ -274,7 +274,7 @@ private[http] object Http2Blueprint { * * To make use of parallelism requests and responses need to be associated (other than by ordering), suggestion * is to add a special (virtual) header containing the streamId (or any other kind of token) is added to the HttRequest - * that must be reproduced in an HttpResponse. This can be done automatically for the `bind`` API but for + * that must be reproduced in an HttpResponse. This can be done automatically for the `bind` API but for * `bindFlow` the user needs to take of this manually. */ def httpLayer(settings: ServerSettings, log: LoggingAdapter, dateHeaderRendering: DateHeaderRendering) @@ -284,12 +284,14 @@ private[http] object Http2Blueprint { // HttpHeaderParser is not thread safe and should not be called concurrently, // the internal trie, however, has built-in protection and will do copy-on-write val masterHttpHeaderParser = HttpHeaderParser(parserSettings, log) - BidiFlow.fromFlows( - Flow[HttpResponse].map(new ResponseRendering(settings, log, dateHeaderRendering)), - Flow[Http2SubStream].via(StreamUtils.statefulAttrsMap { attrs => - val headerParser = masterHttpHeaderParser.createShallowCopy() - RequestParsing.parseRequest(headerParser, settings, attrs) - })) + RequestErrorFlow().atop( + BidiFlow.fromFlows( + Flow[HttpResponse].map(new ResponseRendering(settings, log, dateHeaderRendering)), + Flow[Http2SubStream].via(StreamUtils.statefulAttrsMap { attrs => + val headerParser = masterHttpHeaderParser.createShallowCopy() + RequestParsing.parseRequest(headerParser, settings, attrs) + })) + ) } /** diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/Http2StreamHandling.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/Http2StreamHandling.scala index a09f4ef64a..e17ca4dc35 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/Http2StreamHandling.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/Http2StreamHandling.scala @@ -299,7 +299,7 @@ private[http2] trait Http2StreamHandling extends GraphStageLogic with LogHelper nextStateStream: IncomingStreamBuffer => StreamState, correlationAttributes: Map[AttributeKey[_], _] = Map.empty): StreamState = event match { - case frame @ ParsedHeadersFrame(streamId, endStream, _, _) => + case frame @ ParsedHeadersFrame(streamId, endStream, _, _, _) => if (endStream) { dispatchSubstream(frame, Left(ByteString.empty), correlationAttributes) nextStateEmpty @@ -833,7 +833,7 @@ private[http2] trait Http2StreamHandling extends GraphStageLogic with LogHelper "Found both an attribute with trailing headers, and headers in the `LastChunk`. This is not supported.") trailer = OptionVal.Some(ParsedHeadersFrame(streamId, endStream = true, HttpMessageRendering.renderHeaders(headers, log, isServer, shouldRenderAutoHeaders = false, - dateHeaderRendering = DateHeaderRendering.Unavailable), None)) + dateHeaderRendering = DateHeaderRendering.Unavailable), None, None)) } maybePull() diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/HttpMessageRendering.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/HttpMessageRendering.scala index e434b857e2..d2dc3028e3 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/HttpMessageRendering.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/HttpMessageRendering.scala @@ -94,11 +94,11 @@ private[http2] sealed abstract class MessageRendering[R <: HttpMessage] extends shouldRenderAutoHeaders = true, dateHeaderRendering) val streamId = nextStreamId(r) - val headersFrame = ParsedHeadersFrame(streamId, endStream = r.entity.isKnownEmpty, headerPairs.result(), None) + val headersFrame = ParsedHeadersFrame(streamId, endStream = r.entity.isKnownEmpty, headerPairs.result(), None, None) val trailingHeadersFrame = r.attribute(AttributeKeys.trailer) match { case Some(trailer) if trailer.headers.nonEmpty => - OptionVal.Some(ParsedHeadersFrame(streamId, endStream = true, trailer.headers, None)) + OptionVal.Some(ParsedHeadersFrame(streamId, endStream = true, trailer.headers, None, None)) case _ => OptionVal.None } diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala new file mode 100644 index 0000000000..043a570736 --- /dev/null +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * license agreements; and to You under the Apache License, version 2.0: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * This file is part of the Apache Pekko project, which was derived from Akka. + */ + +/* + * Copyright (C) 2009-2022 Lightbend Inc. + */ + +package org.apache.pekko.http.impl.engine.http2 + +import org.apache.pekko.NotUsed +import org.apache.pekko.annotation.InternalApi +import org.apache.pekko.http.impl.engine.http2.RequestParsing.ParseRequestResult +import org.apache.pekko.http.scaladsl.model.HttpRequest +import org.apache.pekko.http.scaladsl.model.HttpResponse +import org.apache.pekko.http.scaladsl.model.StatusCodes +import org.apache.pekko.stream.Attributes +import org.apache.pekko.stream.BidiShape +import org.apache.pekko.stream.Inlet +import org.apache.pekko.stream.Outlet +import org.apache.pekko.stream.scaladsl.BidiFlow +import org.apache.pekko.stream.stage.GraphStage +import org.apache.pekko.stream.stage.GraphStageLogic +import org.apache.pekko.stream.stage.InHandler +import org.apache.pekko.stream.stage.OutHandler + +/** + * INTERNAL API + */ +@InternalApi +private[http2] object RequestErrorFlow { + + private val _bidiFlow = BidiFlow.fromGraph(new RequestErrorFlow) + def apply(): BidiFlow[HttpResponse, HttpResponse, ParseRequestResult, HttpRequest, NotUsed] = _bidiFlow + +} + +/** + * INTERNAL API + */ +@InternalApi +private[http2] final class RequestErrorFlow + extends GraphStage[BidiShape[HttpResponse, HttpResponse, ParseRequestResult, HttpRequest]] { + + val requestIn = Inlet[ParseRequestResult]("requestIn") + val requestOut = Outlet[HttpRequest]("requestOut") + val responseIn = Inlet[HttpResponse]("responseIn") + val responseOut = Outlet[HttpResponse]("responseOut") + + override val shape: BidiShape[HttpResponse, HttpResponse, ParseRequestResult, HttpRequest] = + BidiShape(responseIn, responseOut, requestIn, requestOut) + + override def createLogic(inheritedAttributes: Attributes): GraphStageLogic = new GraphStageLogic(shape) { + setHandlers(requestIn, requestOut, new InHandler with OutHandler { + override def onPush(): Unit = { + grab(requestIn) match { + case RequestParsing.OkRequest(request) => push(requestOut, request) + case notOk: RequestParsing.BadRequest => + emit(responseOut, + HttpResponse(StatusCodes.BadRequest, entity = notOk.info.summary).addAttribute(Http2.streamId, + notOk.streamId)) + pull(requestIn) + } + } + + override def onPull(): Unit = pull(requestIn) + }) + setHandlers(responseIn, responseOut, new InHandler with OutHandler { + override def onPush(): Unit = push(responseOut, grab(responseIn)) + override def onPull(): Unit = if (!hasBeenPulled(responseIn)) pull(responseIn) + }) + + } +} diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala index e5f557ee70..dd0549d228 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala @@ -16,7 +16,6 @@ package org.apache.pekko.http.impl.engine.http2 import javax.net.ssl.SSLSession import org.apache.pekko import pekko.annotation.InternalApi -import pekko.http.impl.engine.http2.Http2Compliance.Http2ProtocolException import pekko.http.impl.engine.parsing.HttpHeaderParser import pekko.http.impl.engine.server.HttpAttributes import pekko.http.scaladsl.model @@ -27,8 +26,10 @@ import pekko.stream.Attributes import pekko.util.ByteString import pekko.util.OptionVal +import scala.annotation.nowarn import scala.annotation.tailrec import scala.collection.immutable.VectorBuilder +import scala.util.control.NoStackTrace /** * INTERNAL API @@ -36,8 +37,13 @@ import scala.collection.immutable.VectorBuilder @InternalApi private[http2] object RequestParsing { + sealed trait ParseRequestResult + final case class OkRequest(request: HttpRequest) extends ParseRequestResult + final case class BadRequest(info: ErrorInfo, streamId: Int) extends ParseRequestResult + + @nowarn("msg=use remote-address-attribute instead") def parseRequest(httpHeaderParser: HttpHeaderParser, serverSettings: ServerSettings, streamAttributes: Attributes) - : Http2SubStream => HttpRequest = { + : Http2SubStream => ParseRequestResult = { val remoteAddressAttribute: Option[RemoteAddress] = if (serverSettings.remoteAddressAttribute) { @@ -151,19 +157,19 @@ private[http2] object RequestParsing { rec(incomingHeaders, offset + 1, method, scheme, authority, pathAndRawQuery, OptionVal.Some(ContentType.get(value)), contentLength, cookies, true, headers) else - malformedRequest("HTTP message must not contain more than one content-type header") + parseError("HTTP message must not contain more than one content-type header", "content-type") case ":status" => - malformedRequest("Pseudo-header ':status' is for responses only; it cannot appear in a request") + parseError("Pseudo-header ':status' is for responses only; it cannot appear in a request", ":status") case "content-length" => if (contentLength == -1) { val contentLengthValue = ContentLength.get(value).toLong if (contentLengthValue < 0) - malformedRequest("HTTP message must not contain a negative content-length header") + parseError("HTTP message must not contain a negative content-length header", "content-length") rec(incomingHeaders, offset + 1, method, scheme, authority, pathAndRawQuery, contentType, contentLengthValue, cookies, true, headers) - } else malformedRequest("HTTP message must not contain more than one content-length header") + } else parseError("HTTP message must not contain more than one content-length header", "content-length") case "cookie" => // Compress cookie headers as described here https://tools.ietf.org/html/rfc7540#section-8.1.2.5 @@ -182,11 +188,21 @@ private[http2] object RequestParsing { } } - val incomingHeaders = subStream.initialHeaders.keyValuePairs.toIndexedSeq - if (incomingHeaders.size > serverSettings.parserSettings.maxHeaderCount) - malformedRequest( - s"HTTP message contains more than the configured limit of ${serverSettings.parserSettings.maxHeaderCount} headers") - else rec(incomingHeaders, 0) + try { + subStream.initialHeaders.headerParseErrorDetails match { + case Some(details) => + // header errors already found in decompression + BadRequest(details, subStream.streamId) + case None => + val incomingHeaders = subStream.initialHeaders.keyValuePairs.toIndexedSeq + if (incomingHeaders.size > serverSettings.parserSettings.maxHeaderCount) + parseError( + s"HTTP message contains more than the configured limit of ${serverSettings.parserSettings.maxHeaderCount} headers") + else OkRequest(rec(incomingHeaders, 0)) + } + } catch { + case bre: ParsingException => BadRequest(bre.info, subStream.streamId) + } } } @@ -201,25 +217,33 @@ private[http2] object RequestParsing { } private[http2] def checkRequiredPseudoHeader(name: String, value: AnyRef): Unit = - if (value eq null) malformedRequest(s"Mandatory pseudo-header '$name' missing") + if (value eq null) protocolError(s"Mandatory pseudo-header '$name' missing") private[http2] def checkUniquePseudoHeader(name: String, value: AnyRef): Unit = - if (value ne null) malformedRequest(s"Pseudo-header '$name' must not occur more than once") + if (value ne null) protocolError(s"Pseudo-header '$name' must not occur more than once") private[http2] def checkNoRegularHeadersBeforePseudoHeader(name: String, seenRegularHeader: Boolean): Unit = - if (seenRegularHeader) malformedRequest(s"Pseudo-header field '$name' must not appear after a regular header") - private[http2] def malformedRequest(msg: String): Nothing = - throw new Http2ProtocolException(s"Malformed request: $msg") + if (seenRegularHeader) parseError(s"Pseudo-header field '$name' must not appear after a regular header", name) private[http2] def validateHeader(httpHeader: HttpHeader) = httpHeader.lowercaseName match { case "connection" => // https://tools.ietf.org/html/rfc7540#section-8.1.2.2 - malformedRequest("Header 'Connection' must not be used with HTTP/2") + parseError("Header 'Connection' must not be used with HTTP/2", "Connection") case "transfer-encoding" => // https://tools.ietf.org/html/rfc7540#section-8.1.2.2 - malformedRequest("Header 'Transfer-Encoding' must not be used with HTTP/2") + parseError("Header 'Transfer-Encoding' must not be used with HTTP/2", "Transfer-encoding") case "te" => // https://tools.ietf.org/html/rfc7540#section-8.1.2.2 if (httpHeader.value.compareToIgnoreCase("trailers") != 0) - malformedRequest(s"Header 'TE' must not contain value other than 'trailers', value was '${httpHeader.value}") + parseError(s"Header 'TE' must not contain value other than 'trailers', value was '${httpHeader.value}", "TE") case _ => // ok } + // parse errors lead to BadRequest response while Protocol + private[http2] def protocolError(summary: String): Nothing = + throw new Http2Compliance.Http2ProtocolException(s"Malformed request: $summary") + + private[http2] def parseError(summary: String, headerName: String): Nothing = + throw new ParsingException(ErrorInfo(s"Malformed request: $summary").withErrorHeaderName(headerName)) with NoStackTrace + + private def parseError(summary: String): Nothing = + throw new ParsingException(ErrorInfo(s"Malformed request: $summary")) with NoStackTrace + } diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsing.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsing.scala index 80b0545e19..5aae224233 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsing.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsing.scala @@ -16,6 +16,7 @@ package client import org.apache.pekko import pekko.annotation.InternalApi +import pekko.http.impl.engine.http2.Http2Compliance.Http2ProtocolException import pekko.http.impl.engine.http2.RequestParsing._ import pekko.http.impl.engine.parsing.HttpHeaderParser import pekko.http.impl.engine.server.HttpAttributes @@ -85,26 +86,26 @@ private[http2] object ResponseParsing { rec(remainingHeaders.tail, status, OptionVal.Some(contentTypeValue), contentLength, seenRegularHeader, headers) else - malformedRequest("HTTP message must not contain more than one content-type header") + malformedResponse("HTTP message must not contain more than one content-type header") case ("content-type", ct: String) => if (contentType.isEmpty) { val contentTypeValue = - ContentType.parse(ct).getOrElse(malformedRequest(s"Invalid content-type: '$ct'")) + ContentType.parse(ct).right.getOrElse(malformedResponse(s"Invalid content-type: '$ct'")) rec(remainingHeaders.tail, status, OptionVal.Some(contentTypeValue), contentLength, seenRegularHeader, headers) - } else malformedRequest("HTTP message must not contain more than one content-type header") + } else malformedResponse("HTTP message must not contain more than one content-type header") case ("content-length", length: String) => if (contentLength == -1) { val contentLengthValue = length.toLong if (contentLengthValue < 0) - malformedRequest("HTTP message must not contain a negative content-length header") + malformedResponse("HTTP message must not contain a negative content-length header") rec(remainingHeaders.tail, status, contentType, contentLengthValue, seenRegularHeader, headers) - } else malformedRequest("HTTP message must not contain more than one content-length header") + } else malformedResponse("HTTP message must not contain more than one content-length header") case (name, _) if name.startsWith(':') => - malformedRequest(s"Unexpected pseudo-header '$name' in response") + malformedResponse(s"Unexpected pseudo-header '$name' in response") case (_, httpHeader: HttpHeader) => rec(remainingHeaders.tail, status, contentType, contentLength, seenRegularHeader = true, @@ -119,4 +120,7 @@ private[http2] object ResponseParsing { rec(subStream.initialHeaders.keyValuePairs) } + + private def malformedResponse(msg: String): Nothing = + throw new Http2ProtocolException(s"Malformed response: $msg") } diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderCompression.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderCompression.scala index 2738a8f09a..60fec77d8b 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderCompression.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderCompression.scala @@ -48,7 +48,7 @@ private[http2] object HeaderCompression extends GraphStage[FlowShape[FrameEvent, case ack @ SettingsAckFrame(s) => applySettings(s) push(eventsOut, ack) - case ParsedHeadersFrame(streamId, endStream, kvs, prioInfo) => + case ParsedHeadersFrame(streamId, endStream, kvs, prioInfo, _) => // When ending the stream without any payload, use a DATA frame rather than // a HEADERS frame to work around https://github.com/golang/go/issues/47851. if (endStream && kvs.isEmpty) push(eventsOut, DataFrame(streamId, endStream, ByteString.empty)) diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderDecompression.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderDecompression.scala index 6ed39670a3..4dfab40bb4 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderDecompression.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderDecompression.scala @@ -21,6 +21,7 @@ import pekko.http.impl.engine.http2.Http2Protocol.ErrorCode import pekko.http.impl.engine.http2.RequestParsing.parseHeaderPair import pekko.http.impl.engine.http2._ import pekko.http.impl.engine.parsing.HttpHeaderParser +import pekko.http.scaladsl.model.ParsingException import pekko.http.scaladsl.settings.ParserSettings import pekko.http.shaded.com.twitter.hpack.HeaderListener import pekko.stream._ @@ -97,9 +98,12 @@ private[http2] final class HeaderDecompression(masterHeaderParser: HttpHeaderPar decoder.decode(stream, Receiver) // only compact ByteString supports InputStream with mark/reset decoder.endHeaderBlock() // TODO: do we have to check the result here? - push(eventsOut, ParsedHeadersFrame(streamId, endStream, headers.result(), prioInfo)) + push(eventsOut, ParsedHeadersFrame(streamId, endStream, headers.result(), prioInfo, None)) } catch { - case ex: IOException => + case ex: ParsingException => + // push details further and let RequestErrorFlow handle responding with bad request + push(eventsOut, ParsedHeadersFrame(streamId, endStream, Seq.empty, prioInfo, Some(ex.info))) + case _: IOException => // this is signalled by the decoder when it failed, we want to react to this by rendering a GOAWAY frame fail(eventsOut, new Http2Compliance.Http2ProtocolException(ErrorCode.COMPRESSION_ERROR, "Decompression failed.")) diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala index ae135ccebb..2f349dc6d4 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala @@ -15,7 +15,8 @@ package org.apache.pekko.http.impl.engine.http2.hpack import org.apache.pekko import pekko.annotation.InternalApi -import pekko.http.impl.engine.http2.RequestParsing.malformedRequest +import pekko.http.impl.engine.http2.RequestParsing +import pekko.http.impl.engine.http2.RequestParsing.parseError import pekko.http.scaladsl.model import pekko.http.scaladsl.model.HttpHeader.ParsingResult import model.{ HttpHeader, HttpMethod, HttpMethods, IllegalUriException, ParsingException, StatusCode, Uri } @@ -36,7 +37,7 @@ private[pekko] object Http2HeaderParsing { override def parse(name: String, value: String, parserSettings: ParserSettings): HttpMethod = HttpMethods.getForKey(value) .orElse(parserSettings.customMethods(value)) - .getOrElse(malformedRequest(s"Unknown HTTP method: '$value'")) + .getOrElse(RequestParsing.parseError(s"Unknown HTTP method: '$value'", ":method")) } object PathAndQuery extends HeaderParser[(Uri.Path, Option[String])](":path") { override def parse(name: String, value: String, parserSettings: ParserSettings): (Uri.Path, Option[String]) = @@ -60,7 +61,11 @@ private[pekko] object Http2HeaderParsing { } object ContentType extends HeaderParser[model.ContentType]("content-type") { override def parse(name: String, value: String, parserSettings: ParserSettings): model.ContentType = - model.ContentType.parse(value).getOrElse(malformedRequest(s"Invalid content-type: '$value'")) + model.ContentType.parse(value) match { + case Right(tpe) => tpe + case Left(_) => + parseError(s"Invalid content-type: '$value'", "content-type") + } } object ContentLength extends Verbatim("content-length") object Cookie extends Verbatim("cookie") diff --git a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ClientServerSpec.scala b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ClientServerSpec.scala index 75c8d9f5fc..60aa2edd3e 100644 --- a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ClientServerSpec.scala +++ b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ClientServerSpec.scala @@ -30,6 +30,7 @@ import pekko.http.scaladsl.model.{ HttpHeader, HttpMethod, HttpMethods, + HttpProtocols, HttpRequest, HttpResponse, RequestResponseAssociation, @@ -133,6 +134,17 @@ class Http2ClientServerSpec extends PekkoSpecWithMaterializer( // expect idle timeout exception to propagate to user clientResponsesIn.expectError() shouldBe a[HttpIdleTimeoutException] } + + "return bad request response when header parsing fails" in new TestSetup { + val badRequest = HttpRequest( + // easiest valid way to cause parsing to fail - unknown method + method = HttpMethod.custom("UNKNOWN_TO_SERVER"), + uri = "http://www.example.com/test" + ) + sendClientRequest(badRequest) + val response = expectClientResponse() + response.status should be(StatusCodes.BadRequest) + } } case class ServerRequest(request: HttpRequest, promise: Promise[HttpResponse]) { diff --git a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerDemuxSpec.scala b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerDemuxSpec.scala index 8be03fa12d..55f6adc393 100644 --- a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerDemuxSpec.scala +++ b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerDemuxSpec.scala @@ -54,7 +54,7 @@ class Http2ServerDemuxSpec extends PekkoSpecWithMaterializer(""" // The request is taken from the HTTP/1.1 request that had the Upgrade // header and is passed to the handler code 'directly', bypassing the demux stage, // so the first thing the demux stage sees of this request is the response: - val response = ParsedHeadersFrame(streamId = 1, endStream = true, Seq((":status", "200")), None) + val response = ParsedHeadersFrame(streamId = 1, endStream = true, Seq((":status", "200")), None, None) substreamProducer.sendNext(new Http2SubStream( response, OptionVal.None, diff --git a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala index a844bd762b..9eec3a6fe1 100644 --- a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala +++ b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala @@ -21,6 +21,7 @@ import FrameEvent._ import org.apache.pekko import pekko.http.impl.engine.http2.hpack.HeaderDecompression import pekko.http.impl.engine.parsing.HttpHeaderParser +import pekko.http.impl.engine.http2.Http2Compliance.Http2ProtocolException import pekko.http.impl.engine.server.HttpAttributes import pekko.http.impl.util.PekkoSpecWithMaterializer import pekko.http.scaladsl.model._ @@ -29,8 +30,10 @@ import pekko.http.scaladsl.settings.ServerSettings import pekko.stream.Attributes import pekko.stream.scaladsl.{ Sink, Source } import pekko.util.{ ByteString, OptionVal } +import FrameEvent._ import org.scalatest.{ Inside, Inspectors } +import org.scalatest.exceptions.TestFailedException class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Inspectors { "RequestParsing" should { @@ -38,10 +41,10 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp /** Helper to test parsing */ def parse( keyValuePairs: Seq[(String, String)], - data: Source[ByteString, Any] = Source.empty, - attributes: Attributes = Attributes(), - uriParsingMode: Uri.ParsingMode = Uri.ParsingMode.Relaxed, - settings: ServerSettings = ServerSettings(system)): HttpRequest = { + data: Source[ByteString, Any], + attributes: Attributes, + uriParsingMode: Uri.ParsingMode, + settings: ServerSettings): RequestParsing.ParseRequestResult = { val (serverSettings, parserSettings) = { val ps = settings.parserSettings.withUriParsingMode(uriParsingMode) (settings.withParserSettings(ps), ps) @@ -52,30 +55,59 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp val frame = HeadersFrame(1, data == Source.empty, endHeaders = true, encoder.encodeHeaderPairs(keyValuePairs), None) - val parseRequest: Http2SubStream => HttpRequest = - RequestParsing.parseRequest(headerParser, serverSettings, attributes) - - try Source.single(frame) - .via(new HeaderDecompression(headerParser, parserSettings)) - .map { // emulate demux - case headers: ParsedHeadersFrame => - Http2SubStream( - initialHeaders = headers, - trailingHeaders = OptionVal.None, - data = Right(data), - correlationAttributes = Map.empty) - } - .map(parseRequest) - .runWith(Sink.head) - .futureValue - catch { case ex: Throwable => throw ex.getCause } // unpack futureValue exceptions + val parseRequest: Http2SubStream => ParseRequestResult = RequestParsing.parseRequest(headerParser, serverSettings, + attributes) + + Source.single(frame) + .via(new HeaderDecompression(headerParser, parserSettings)) + .map { // emulate demux + case headers: ParsedHeadersFrame => + Http2SubStream( + initialHeaders = headers, + trailingHeaders = OptionVal.None, + data = Right(data), + correlationAttributes = Map.empty) + } + .map(parseRequest) + .runWith(Sink.head) + .futureValue } - def shouldThrowMalformedRequest[T](block: => T): Exception = { - val thrown = the[RuntimeException] thrownBy block - thrown.getMessage should startWith("Malformed request: ") - thrown - } + def parseExpectOk( + keyValuePairs: Seq[(String, String)], + data: Source[ByteString, Any] = Source.empty, + attributes: Attributes = Attributes(), + uriParsingMode: Uri.ParsingMode = Uri.ParsingMode.Relaxed, + settings: ServerSettings = ServerSettings(system)): HttpRequest = + parse(keyValuePairs, data, attributes, uriParsingMode, settings) match { + case RequestParsing.OkRequest(req) => req + case RequestParsing.BadRequest(info, _) => fail(s"Failed parsing request: $info") + } + + def parseExpectError( + keyValuePairs: Seq[(String, String)], + data: Source[ByteString, Any] = Source.empty, + attributes: Attributes = Attributes(), + uriParsingMode: Uri.ParsingMode = Uri.ParsingMode.Relaxed, + settings: ServerSettings = ServerSettings(system)): ErrorInfo = + parse(keyValuePairs, data, attributes, uriParsingMode, settings) match { + case RequestParsing.OkRequest(req) => fail("Unexpectedly succeeded parsing request") + case RequestParsing.BadRequest(info, _) => info + } + + def parseExpectProtocolError( + keyValuePairs: Seq[(String, String)], + data: Source[ByteString, Any] = Source.empty, + attributes: Attributes = Attributes(), + uriParsingMode: Uri.ParsingMode = Uri.ParsingMode.Relaxed, + settings: ServerSettings = ServerSettings(system)): Http2ProtocolException = + try { + parse(keyValuePairs, data, attributes, uriParsingMode, settings) + fail("expected parsing to throw") + } catch { + case futureValueEx: TestFailedException if futureValueEx.getCause.isInstanceOf[Http2ProtocolException] => + futureValueEx.getCause.asInstanceOf[Http2ProtocolException] + } "follow RFC7540" should { @@ -85,13 +117,13 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp // appear in requests. "not accept response pseudo-header fields in a request" in { - val thrown = shouldThrowMalformedRequest(parse( + val info = parseExpectError( keyValuePairs = Vector( ":scheme" -> "https", ":method" -> "GET", ":path" -> "/", - ":status" -> "200"))) - thrown.getMessage should ===( + ":status" -> "200")) + info.summary should ===( "Malformed request: Pseudo-header ':status' is for responses only; it cannot appear in a request") } @@ -109,7 +141,7 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp // Insert the Foo header so it occurs before at least one pseudo-header val (before, after) = pseudoHeaders.splitAt(insertPoint) val modified = before ++ Vector("Foo" -> "bar") ++ after - shouldThrowMalformedRequest(parse(modified)) + parseExpectError(modified) } } @@ -119,32 +151,30 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp // be treated as malformed... "not accept connection-specific headers" in { - shouldThrowMalformedRequest { - // Add Connection header to indicate that Foo is a connection-specific header - parse(Vector( - ":method" -> "GET", - ":scheme" -> "https", - ":path" -> "/", - "Connection" -> "foo", - "Foo" -> "bar")) - } + // Add Connection header to indicate that Foo is a connection-specific header + parseExpectError(Vector( + ":method" -> "GET", + ":scheme" -> "https", + ":path" -> "/", + "Connection" -> "foo", + "Foo" -> "bar")) } "not accept TE with other values than 'trailers'" in { - shouldThrowMalformedRequest { - // The only exception to this is the TE header field, which MAY be - // present in an HTTP/2 request; when it is, it MUST NOT contain any - // value other than "trailers". - parse(Vector( - ":method" -> "GET", - ":scheme" -> "https", - ":path" -> "/", - "TE" -> "chunked")) - } + + // The only exception to this is the TE header field, which MAY be + // present in an HTTP/2 request; when it is, it MUST NOT contain any + // value other than "trailers". + parseExpectError(Vector( + ":method" -> "GET", + ":scheme" -> "https", + ":path" -> "/", + "TE" -> "chunked")) + } "accept TE with 'trailers' as value" in { - parse(Vector( + parseExpectOk(Vector( ":method" -> "GET", ":scheme" -> "https", ":path" -> "/", @@ -159,7 +189,7 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp "parse the ':method' pseudo-header correctly" in { val methods = Seq("GET", "POST", "DELETE", "OPTIONS") forAll(methods) { (method: String) => - val request: HttpRequest = parse( + val request: HttpRequest = parseExpectOk( keyValuePairs = Vector( ":method" -> method, ":scheme" -> "https", @@ -181,7 +211,7 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp // can't be constructed with any other schemes. val schemes = Seq("http", "https", "ws", "wss") forAll(schemes) { (scheme: String) => - val request: HttpRequest = parse( + val request: HttpRequest = parseExpectOk( keyValuePairs = Vector( ":method" -> "POST", ":scheme" -> scheme, @@ -206,7 +236,7 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp ("example.com:8042", "example.com", Some(8042))) forAll(authorities) { case (authority, host, optPort) => - val request: HttpRequest = parse( + val request: HttpRequest = parseExpectOk( keyValuePairs = Vector( ":method" -> "POST", ":scheme" -> "https", @@ -221,14 +251,13 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp val authorities = Seq("?", " ", "@", ":") forAll(authorities) { authority => - val thrown = the[ParsingException] thrownBy - (parse( - keyValuePairs = Vector( - ":method" -> "POST", - ":scheme" -> "https", - ":authority" -> authority, - ":path" -> "/"))) - thrown.getMessage should include("http2-authority-pseudo-header") + val info = parseExpectError( + keyValuePairs = Vector( + ":method" -> "POST", + ":scheme" -> "https", + ":authority" -> authority, + ":path" -> "/")) + info.summary should include("http2-authority-pseudo-header") } } } @@ -269,9 +298,14 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp "follow RFC3986 for the ':path' pseudo-header" should { def parsePath(path: String, uriParsingMode: Uri.ParsingMode = Uri.ParsingMode.Relaxed): Uri = { - parse(Seq(":method" -> "GET", ":scheme" -> "https", ":path" -> path), uriParsingMode = uriParsingMode).uri + parseExpectOk(Seq(":method" -> "GET", ":scheme" -> "https", ":path" -> path), + uriParsingMode = uriParsingMode).uri } + def parsePathExpectError(path: String, uriParsingMode: Uri.ParsingMode = Uri.ParsingMode.Relaxed): ErrorInfo = + parseExpectError(Seq(":method" -> "GET", ":scheme" -> "https", ":path" -> path), + uriParsingMode = uriParsingMode) + // sub-delims = "!" / "$" / "&" / "'" / "(" / ")" // / "*" / "+" / "," / ";" / "=" @@ -333,8 +367,8 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp "?", "&", "=", "#", ":", "?", "#", "[", "]", "@", " ", "http://localhost/foo") forAll(invalidAbsolutePaths) { (absPath: String) => - val exception = the[ParsingException] thrownBy (parsePath(absPath)) - exception.getMessage should include("http2-path-pseudo-header") + val info = parsePathExpectError(absPath) + info.summary should include("http2-path-pseudo-header") } } @@ -343,8 +377,8 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp // Illegal for path-absolute in RFC3986 to start with multiple slashes "//", "//x") forAll(invalidAbsolutePaths) { (absPath: String) => - val exception = the[ParsingException] thrownBy (parsePath(absPath, uriParsingMode = Uri.ParsingMode.Strict)) - exception.getMessage should include("http2-path-pseudo-header") + val info = parsePathExpectError(absPath, uriParsingMode = Uri.ParsingMode.Strict) + info.summary should include("http2-path-pseudo-header") } } @@ -386,10 +420,11 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp "reject a ':path' containing an invalid 'query'" in pendingUntilFixed { val invalidQueries: Seq[String] = Seq( ":", "/", "?", "#", "[", "]", "@", " ") + forAll(absolutePaths.take(3)) { case (inputPath, _) => forAll(invalidQueries) { (query: String) => - shouldThrowMalformedRequest(parsePath(inputPath + "?" + query, uriParsingMode = Uri.ParsingMode.Strict)) + parsePathExpectError(inputPath + "?" + query, uriParsingMode = Uri.ParsingMode.Strict) } } } @@ -400,7 +435,7 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp // value '*' for the ":path" pseudo-header field. "handle a ':path' with an asterisk" in pendingUntilFixed { - val request: HttpRequest = parse( + val request: HttpRequest = parseExpectOk( keyValuePairs = Vector( ":method" -> "OPTIONS", ":scheme" -> "http", @@ -411,14 +446,14 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp // [The ":path"] pseudo-header field MUST NOT be empty for "http" or "https" // URIs... - "reject empty ':path' pseudo-headers for http and https" in pendingUntilFixed { + "reject empty ':path' pseudo-headers for http and https" in { val schemes = Seq("http", "https") forAll(schemes) { (scheme: String) => - shouldThrowMalformedRequest(parse( + parseExpectError( keyValuePairs = Vector( ":method" -> "POST", ":scheme" -> scheme, - ":path" -> ""))) + ":path" -> "")) } } @@ -439,12 +474,12 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp "reject requests without a mandatory pseudo-headers" in { val mandatoryPseudoHeaders = Seq(":method", ":scheme", ":path") forAll(mandatoryPseudoHeaders) { (name: String) => - val thrown = shouldThrowMalformedRequest(parse( + val ex = parseExpectProtocolError( keyValuePairs = Vector( ":scheme" -> "https", ":method" -> "GET", - ":path" -> "/").filter(_._1 != name))) - thrown.getMessage should ===(s"Malformed request: Mandatory pseudo-header '$name' missing") + ":path" -> "/").filter(_._1 != name)) + ex.getMessage should ===(s"Malformed request: Mandatory pseudo-header '$name' missing") } } @@ -453,13 +488,13 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp Seq(":method" -> "POST", ":scheme" -> "http", ":path" -> "/other", ":authority" -> "example.org") forAll(pseudoHeaders) { case (name: String, alternative: String) => - val thrown = shouldThrowMalformedRequest(parse( + val ex = parseExpectProtocolError( keyValuePairs = Vector( ":scheme" -> "https", ":method" -> "GET", ":authority" -> "pekko.apache.org", - ":path" -> "/") :+ (name -> alternative))) - thrown.getMessage should ===(s"Malformed request: Pseudo-header '$name' must not occur more than once") + ":path" -> "/") :+ (name -> alternative)) + ex.getMessage should ===(s"Malformed request: Pseudo-header '$name' must not occur more than once") } } @@ -479,7 +514,7 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp Seq("a=b", "c=d; e=f") -> "a=b; c=d; e=f") forAll(cookieHeaders) { case (inValues, outValue) => - val httpRequest: HttpRequest = parse( + val httpRequest: HttpRequest = parseExpectOk( Vector( ":method" -> "GET", ":scheme" -> "https", @@ -495,7 +530,7 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp // 8.1.3. Examples "parse GET example" in { - val request: HttpRequest = parse( + val request: HttpRequest = parseExpectOk( keyValuePairs = Vector( ":method" -> "GET", ":scheme" -> "https", @@ -518,7 +553,7 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp } "parse POST example" in { - val request: HttpRequest = parse( + val request: HttpRequest = parseExpectOk( keyValuePairs = Vector( ":method" -> "POST", ":scheme" -> "https", @@ -552,7 +587,7 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp // Tests that don't come from an RFC document... "parse GET https://localhost:8000/ correctly" in { - val request: HttpRequest = parse( + val request: HttpRequest = parseExpectOk( keyValuePairs = Vector( ":method" -> "GET", ":scheme" -> "https", @@ -571,42 +606,41 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp } "reject requests with multiple content length headers" in { - val thrown = shouldThrowMalformedRequest(parse( + val info = parseExpectError( keyValuePairs = Vector( ":method" -> "GET", ":scheme" -> "https", ":authority" -> "localhost:8000", ":path" -> "/", "content-length" -> "123", - "content-length" -> "124"))) - thrown.getMessage should ===( + "content-length" -> "124")) + info.summary should ===( s"Malformed request: HTTP message must not contain more than one content-length header") } "reject requests with multiple content type headers" in { - val thrown = shouldThrowMalformedRequest(parse( + val info = parseExpectError( keyValuePairs = Vector( ":method" -> "GET", ":scheme" -> "https", ":authority" -> "localhost:8000", ":path" -> "/", "content-type" -> "text/json", - "content-type" -> "text/json"))) - thrown.getMessage should ===( + "content-type" -> "text/json")) + info.summary should ===( s"Malformed request: HTTP message must not contain more than one content-type header") } "reject requests with too many headers" in { val maxHeaderCount = ServerSettings(system).parserSettings.maxHeaderCount - val thrown = shouldThrowMalformedRequest( - parse((0 to (maxHeaderCount + 1)).map(n => s"x-my-header-$n" -> n.toString).toVector)) - thrown.getMessage should ===( + val info = parseExpectError((0 to (maxHeaderCount + 1)).map(n => s"x-my-header-$n" -> n.toString).toVector) + info.summary should ===( s"Malformed request: HTTP message contains more than the configured limit of $maxHeaderCount headers") } "add remote address request attribute if enabled" in { val theAddress = InetAddress.getByName("127.5.2.1") - val request: HttpRequest = parse( + val request: HttpRequest = parseExpectOk( keyValuePairs = Vector( ":method" -> "GET", ":scheme" -> "https", From e00055eed9464e80b61e65876a815a3432e3187c Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 23 Feb 2026 13:58:09 +0100 Subject: [PATCH 3/6] small changes --- ...d-header-http2-response.backwards.excludes | 6 ----- ...d-header-http2-response.backwards.excludes | 23 +++++++++++++++++ .../impl/engine/http2/RequestErrorFlow.scala | 25 +++++++------------ 3 files changed, 32 insertions(+), 22 deletions(-) delete mode 100644 http-core/src/main/mima-filters/1.2.x.backwards.excludes/4226-bad-header-http2-response.backwards.excludes create mode 100644 http-core/src/main/mima-filters/1.4.x.backwards.excludes/bad-header-http2-response.backwards.excludes diff --git a/http-core/src/main/mima-filters/1.2.x.backwards.excludes/4226-bad-header-http2-response.backwards.excludes b/http-core/src/main/mima-filters/1.2.x.backwards.excludes/4226-bad-header-http2-response.backwards.excludes deleted file mode 100644 index b974d145f6..0000000000 --- a/http-core/src/main/mima-filters/1.2.x.backwards.excludes/4226-bad-header-http2-response.backwards.excludes +++ /dev/null @@ -1,6 +0,0 @@ -# internals only -ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent#ParsedHeadersFrame.copy") -ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent#ParsedHeadersFrame.this") -ProblemFilters.exclude[MissingTypesProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent$ParsedHeadersFrame$") -ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent#ParsedHeadersFrame.apply") -ProblemFilters.exclude[IncompatibleSignatureProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent#ParsedHeadersFrame.unapply") diff --git a/http-core/src/main/mima-filters/1.4.x.backwards.excludes/bad-header-http2-response.backwards.excludes b/http-core/src/main/mima-filters/1.4.x.backwards.excludes/bad-header-http2-response.backwards.excludes new file mode 100644 index 0000000000..fab687cc99 --- /dev/null +++ b/http-core/src/main/mima-filters/1.4.x.backwards.excludes/bad-header-http2-response.backwards.excludes @@ -0,0 +1,23 @@ +# 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. + +# Invalid HTTP/2 request headers return 400 Bad Request instead of GOAWAY +ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent#ParsedHeadersFrame.copy") +ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent#ParsedHeadersFrame.this") +ProblemFilters.exclude[MissingTypesProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent$ParsedHeadersFrame$") +ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent#ParsedHeadersFrame.apply") +ProblemFilters.exclude[IncompatibleSignatureProblem]("org.apache.pekko.http.impl.engine.http2.FrameEvent#ParsedHeadersFrame.unapply") diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala index 043a570736..453144d8cd 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala @@ -8,26 +8,19 @@ */ /* - * Copyright (C) 2009-2022 Lightbend Inc. + * Copyright (C) 2009-2023 Lightbend Inc. */ package org.apache.pekko.http.impl.engine.http2 -import org.apache.pekko.NotUsed -import org.apache.pekko.annotation.InternalApi -import org.apache.pekko.http.impl.engine.http2.RequestParsing.ParseRequestResult -import org.apache.pekko.http.scaladsl.model.HttpRequest -import org.apache.pekko.http.scaladsl.model.HttpResponse -import org.apache.pekko.http.scaladsl.model.StatusCodes -import org.apache.pekko.stream.Attributes -import org.apache.pekko.stream.BidiShape -import org.apache.pekko.stream.Inlet -import org.apache.pekko.stream.Outlet -import org.apache.pekko.stream.scaladsl.BidiFlow -import org.apache.pekko.stream.stage.GraphStage -import org.apache.pekko.stream.stage.GraphStageLogic -import org.apache.pekko.stream.stage.InHandler -import org.apache.pekko.stream.stage.OutHandler +import org.apache.pekko +import pekko.NotUsed +import pekko.annotation.InternalApi +import pekko.http.impl.engine.http2.RequestParsing.ParseRequestResult +import pekko.http.scaladsl.model.{ HttpRequest, HttpResponse, StatusCodes } +import pekko.stream.{ Attributes, BidiShape, Inlet, Outlet } +import pekko.stream.scaladsl.BidiFlow +import pekko.stream.stage.{ GraphStage, GraphStageLogic, InHandler, OutHandler } /** * INTERNAL API From ed84a1a4722e10f487327f0d2425c5edbb7c2e83 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Feb 2026 14:04:32 +0000 Subject: [PATCH 4/6] Fix post-merge issues: remove .right deprecation, remove duplicate import, remove unused @nowarn Co-authored-by: pjfanning <11783444+pjfanning@users.noreply.github.com> --- .../apache/pekko/http/impl/engine/http2/RequestParsing.scala | 2 -- .../pekko/http/impl/engine/http2/client/ResponseParsing.scala | 2 +- .../pekko/http/impl/engine/http2/RequestParsingSpec.scala | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala index dd0549d228..8b66babc24 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala @@ -26,7 +26,6 @@ import pekko.stream.Attributes import pekko.util.ByteString import pekko.util.OptionVal -import scala.annotation.nowarn import scala.annotation.tailrec import scala.collection.immutable.VectorBuilder import scala.util.control.NoStackTrace @@ -41,7 +40,6 @@ private[http2] object RequestParsing { final case class OkRequest(request: HttpRequest) extends ParseRequestResult final case class BadRequest(info: ErrorInfo, streamId: Int) extends ParseRequestResult - @nowarn("msg=use remote-address-attribute instead") def parseRequest(httpHeaderParser: HttpHeaderParser, serverSettings: ServerSettings, streamAttributes: Attributes) : Http2SubStream => ParseRequestResult = { diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsing.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsing.scala index 5aae224233..78868b2c62 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsing.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsing.scala @@ -91,7 +91,7 @@ private[http2] object ResponseParsing { case ("content-type", ct: String) => if (contentType.isEmpty) { val contentTypeValue = - ContentType.parse(ct).right.getOrElse(malformedResponse(s"Invalid content-type: '$ct'")) + ContentType.parse(ct).getOrElse(malformedResponse(s"Invalid content-type: '$ct'")) rec(remainingHeaders.tail, status, OptionVal.Some(contentTypeValue), contentLength, seenRegularHeader, headers) } else malformedResponse("HTTP message must not contain more than one content-type header") diff --git a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala index 9eec3a6fe1..a51def4a35 100644 --- a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala +++ b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala @@ -22,6 +22,7 @@ import org.apache.pekko import pekko.http.impl.engine.http2.hpack.HeaderDecompression import pekko.http.impl.engine.parsing.HttpHeaderParser import pekko.http.impl.engine.http2.Http2Compliance.Http2ProtocolException +import pekko.http.impl.engine.http2.RequestParsing.ParseRequestResult import pekko.http.impl.engine.server.HttpAttributes import pekko.http.impl.util.PekkoSpecWithMaterializer import pekko.http.scaladsl.model._ @@ -30,7 +31,6 @@ import pekko.http.scaladsl.settings.ServerSettings import pekko.stream.Attributes import pekko.stream.scaladsl.{ Sink, Source } import pekko.util.{ ByteString, OptionVal } -import FrameEvent._ import org.scalatest.{ Inside, Inspectors } import org.scalatest.exceptions.TestFailedException From 1bf766085caa2756d455e5d08c03e0cb48d717bb Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 23 Feb 2026 20:04:13 +0100 Subject: [PATCH 5/6] scalafmt --- .../impl/engine/http2/RequestErrorFlow.scala | 34 ++++++++++--------- .../impl/engine/http2/RequestParsing.scala | 3 +- .../http2/hpack/Http2HeaderParsing.scala | 2 +- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala index 453144d8cd..c379449921 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala @@ -49,24 +49,26 @@ private[http2] final class RequestErrorFlow BidiShape(responseIn, responseOut, requestIn, requestOut) override def createLogic(inheritedAttributes: Attributes): GraphStageLogic = new GraphStageLogic(shape) { - setHandlers(requestIn, requestOut, new InHandler with OutHandler { - override def onPush(): Unit = { - grab(requestIn) match { - case RequestParsing.OkRequest(request) => push(requestOut, request) - case notOk: RequestParsing.BadRequest => - emit(responseOut, - HttpResponse(StatusCodes.BadRequest, entity = notOk.info.summary).addAttribute(Http2.streamId, - notOk.streamId)) - pull(requestIn) + setHandlers(requestIn, requestOut, + new InHandler with OutHandler { + override def onPush(): Unit = { + grab(requestIn) match { + case RequestParsing.OkRequest(request) => push(requestOut, request) + case notOk: RequestParsing.BadRequest => + emit(responseOut, + HttpResponse(StatusCodes.BadRequest, entity = notOk.info.summary).addAttribute(Http2.streamId, + notOk.streamId)) + pull(requestIn) + } } - } - override def onPull(): Unit = pull(requestIn) - }) - setHandlers(responseIn, responseOut, new InHandler with OutHandler { - override def onPush(): Unit = push(responseOut, grab(responseIn)) - override def onPull(): Unit = if (!hasBeenPulled(responseIn)) pull(responseIn) - }) + override def onPull(): Unit = pull(requestIn) + }) + setHandlers(responseIn, responseOut, + new InHandler with OutHandler { + override def onPush(): Unit = push(responseOut, grab(responseIn)) + override def onPull(): Unit = if (!hasBeenPulled(responseIn)) pull(responseIn) + }) } } diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala index 8b66babc24..9647274403 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala @@ -239,7 +239,8 @@ private[http2] object RequestParsing { throw new Http2Compliance.Http2ProtocolException(s"Malformed request: $summary") private[http2] def parseError(summary: String, headerName: String): Nothing = - throw new ParsingException(ErrorInfo(s"Malformed request: $summary").withErrorHeaderName(headerName)) with NoStackTrace + throw new ParsingException(ErrorInfo(s"Malformed request: $summary").withErrorHeaderName(headerName)) + with NoStackTrace private def parseError(summary: String): Nothing = throw new ParsingException(ErrorInfo(s"Malformed request: $summary")) with NoStackTrace diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala index 2f349dc6d4..7b2066065f 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala @@ -63,7 +63,7 @@ private[pekko] object Http2HeaderParsing { override def parse(name: String, value: String, parserSettings: ParserSettings): model.ContentType = model.ContentType.parse(value) match { case Right(tpe) => tpe - case Left(_) => + case Left(_) => parseError(s"Invalid content-type: '$value'", "content-type") } } From 4b192defa167a68ab6337d77638a8c7eb2b9b116 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 23 Feb 2026 20:17:02 +0100 Subject: [PATCH 6/6] Update RequestParsingSpec.scala --- .../impl/engine/http2/RequestParsingSpec.scala | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala index a51def4a35..e1ca5a235a 100644 --- a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala +++ b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala @@ -278,14 +278,14 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp val schemes = Seq("http", "https") forAll(schemes) { (scheme: String) => forAll(authorities) { (authority: String) => - val exception = the[Exception] thrownBy - (parse( - keyValuePairs = Vector( - ":method" -> "POST", - ":scheme" -> scheme, - ":authority" -> authority, - ":path" -> "/"))) - exception.getMessage should startWith("Illegal http2-authority-pseudo-header") + val info = parseExpectError( + keyValuePairs = Vector( + ":method" -> "POST", + ":scheme" -> scheme, + ":authority" -> authority, + ":path" -> "/" + )) + info.summary should startWith("Illegal http2-authority-pseudo-header") } } }