fix: align HTTP/2 malformed-header handling

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: He-Pin

http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestErrorFlow.scala

@@ -49,22 +49,25 @@ 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)
+      })
   }
 }

http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala

@@ -158,7 +158,7 @@ private[http2] object RequestParsing {
                 parseError("HTTP message must not contain more than one content-type header", "content-type")
 
             case ":status" =>
-              parseError("Pseudo-header ':status' is for responses only; it cannot appear in a request", ":status")
+              protocolError("Pseudo-header ':status' is for responses only; it cannot appear in a request")
 
             case "content-length" =>
               if (contentLength == -1) {
@@ -218,18 +218,18 @@ private[http2] object RequestParsing {
   private[http2] def checkUniquePseudoHeader(name: String, value: AnyRef): Unit =
     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) parseError(s"Pseudo-header field '$name' must not appear after a regular header", name)
+    if (seenRegularHeader) protocolError(s"Pseudo-header field '$name' must not appear after a regular header")
   private[http2] def validateHeader(httpHeader: HttpHeader) = httpHeader.lowercaseName match {
     case "connection" =>
       // https://tools.ietf.org/html/rfc7540#section-8.1.2.2
-      parseError("Header 'Connection' must not be used with HTTP/2", "Connection")
+      protocolError("Header 'Connection' must not be used with HTTP/2")
     case "transfer-encoding" =>
       // https://tools.ietf.org/html/rfc7540#section-8.1.2.2
-      parseError("Header 'Transfer-Encoding' must not be used with HTTP/2", "Transfer-encoding")
+      protocolError("Header 'Transfer-Encoding' must not be used with HTTP/2")
     case "te" =>
       // https://tools.ietf.org/html/rfc7540#section-8.1.2.2
       if (httpHeader.value.compareToIgnoreCase("trailers") != 0)
-        parseError(s"Header 'TE' must not contain value other than 'trailers', value was '${httpHeader.value}", "TE")
+        protocolError(s"Header 'TE' must not contain value other than 'trailers', value was '${httpHeader.value}")
     case _ => // ok
   }
 

http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala

@@ -16,7 +16,7 @@ package org.apache.pekko.http.impl.engine.http2.hpack
 import org.apache.pekko
 import pekko.annotation.InternalApi
 import pekko.http.impl.engine.http2.RequestParsing
-import pekko.http.impl.engine.http2.RequestParsing.parseError
+import pekko.http.impl.engine.http2.RequestParsing.{ parseError, protocolError }
 import pekko.http.scaladsl.model
 import pekko.http.scaladsl.model.HttpHeader.ParsingResult
 import model.{ HttpHeader, HttpMethod, HttpMethods, IllegalUriException, ParsingException, StatusCode, Uri }
@@ -41,7 +41,9 @@ private[pekko] object Http2HeaderParsing {
   }
   object PathAndQuery extends HeaderParser[(Uri.Path, Option[String])](":path") {
     override def parse(name: String, value: String, parserSettings: ParserSettings): (Uri.Path, Option[String]) =
-      try {
+      if (value.isEmpty) {
+        protocolError("Pseudo-header ':path' must not be empty")
+      } else try {
         Uri.parseHttp2PathPseudoHeader(value, mode = parserSettings.uriParsingMode)
       } catch {
         case IllegalUriException(info) => throw new ParsingException(info)

http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala

@@ -118,14 +118,14 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp
       // appear in requests.
 
       "not accept response pseudo-header fields in a request" in {
-        val info = parseExpectError(
+        val ex = parseExpectProtocolError(
           keyValuePairs = Vector(
             ":scheme" -> "https",
             ":method" -> "GET",
             ":path" -> "/",
             ":status" -> "200"
           ))
-        info.summary should
+        ex.getMessage should
         ===("Malformed request: Pseudo-header ':status' is for responses only; it cannot appear in a request")
       }
 
@@ -144,7 +144,9 @@ 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
-          parseExpectError(modified)
+          val ex = parseExpectProtocolError(modified)
+          ex.getMessage should
+          ===(s"Malformed request: Pseudo-header field '${after.head._1}' must not appear after a regular header")
         }
       }
 
@@ -155,26 +157,29 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp
 
       "not accept connection-specific headers" in {
         // Add Connection header to indicate that Foo is a connection-specific header
-        parseExpectError(Vector(
+        val ex = parseExpectProtocolError(Vector(
           ":method" -> "GET",
           ":scheme" -> "https",
           ":path" -> "/",
           "Connection" -> "foo",
           "Foo" -> "bar"
         ))
+        ex.getMessage should ===("Malformed request: Header 'Connection' must not be used with HTTP/2")
       }
 
       "not accept TE with other values than 'trailers'" in {
 
         // 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(
+        val ex = parseExpectProtocolError(Vector(
           ":method" -> "GET",
           ":scheme" -> "https",
           ":path" -> "/",
           "TE" -> "chunked"
         ))
+        ex.getMessage should
+        ===("Malformed request: Header 'TE' must not contain value other than 'trailers', value was 'chunked")
 
       }
 
@@ -467,12 +472,13 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp
       "reject empty ':path' pseudo-headers for http and https" in {
         val schemes = Seq("http", "https")
         forAll(schemes) { (scheme: String) =>
-          parseExpectError(
+          val ex = parseExpectProtocolError(
             keyValuePairs = Vector(
               ":method" -> "POST",
               ":scheme" -> scheme,
               ":path" -> ""
             ))
+          ex.getMessage should ===("Malformed request: Pseudo-header ':path' must not be empty")
         }
       }