From ccf43739f5115ed58c923b5fe3349fca0b5118b3 Mon Sep 17 00:00:00 2001 From: James Roper Date: Wed, 8 Jan 2025 19:57:02 +1100 Subject: [PATCH] chore: Validate custom HTTP method names #4460 --- .../http/impl/model/parser/CommonRules.scala | 4 ++-- .../akka/http/scaladsl/model/HttpMethod.scala | 12 +++++++++-- .../http/scaladsl/model/HttpMethodSpec.scala | 21 +++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 akka-http-core/src/test/scala/akka/http/scaladsl/model/HttpMethodSpec.scala diff --git a/akka-http-core/src/main/scala/akka/http/impl/model/parser/CommonRules.scala b/akka-http-core/src/main/scala/akka/http/impl/model/parser/CommonRules.scala index fd86903d90..9d2e5dcbc6 100644 --- a/akka-http-core/src/main/scala/akka/http/impl/model/parser/CommonRules.scala +++ b/akka-http-core/src/main/scala/akka/http/impl/model/parser/CommonRules.scala @@ -6,11 +6,11 @@ package akka.http.impl.model.parser import scala.collection.immutable import scala.collection.immutable.TreeMap - import akka.http.scaladsl.model._ import akka.http.scaladsl.model.headers._ import akka.parboiled2._ import akka.parboiled2.support.hlist._ +import akka.util.ConstantFun private[parser] trait CommonRules { this: Parser with StringBuilding => protected def maxCommentParsingDepth: Int @@ -465,7 +465,7 @@ private[parser] trait CommonRules { this: Parser with StringBuilding => token ~> { s => HttpMethods.getForKey(s) match { case Some(m) => m - case None => HttpMethod.custom(s) + case None => HttpMethod(s, isSafe = false, isIdempotent = false, requestEntityAcceptance = RequestEntityAcceptance.Expected, ConstantFun.anyToTrue) } } } diff --git a/akka-http-core/src/main/scala/akka/http/scaladsl/model/HttpMethod.scala b/akka-http-core/src/main/scala/akka/http/scaladsl/model/HttpMethod.scala index f393e4f588..cee814ad72 100644 --- a/akka-http-core/src/main/scala/akka/http/scaladsl/model/HttpMethod.scala +++ b/akka-http-core/src/main/scala/akka/http/scaladsl/model/HttpMethod.scala @@ -48,13 +48,21 @@ object HttpMethod { // the allowsEntity condition was used to determine what responses provided the Content-Length header, before #4213 was fixed private def oldContentLengthCondition(status: StatusCode) = status.allowsEntity + // See: + // https://www.rfc-editor.org/rfc/rfc9112.html#name-method + // https://www.rfc-editor.org/rfc/rfc9110.html#name-tokens + private val httpMethodNameRegex = "[a-zA-Z0-9!#$%&'*+.^_`|~-]+".r + + private def validateHttpMethodName(name: String): Unit = + require(httpMethodNameRegex.matches(name), "invalid HTTP method name") + /** * Create a custom method type. * @deprecated The created method will compute the presence of Content-Length headers based on deprecated logic (before issue #4213). */ @Deprecated def custom(name: String, safe: Boolean, idempotent: Boolean, requestEntityAcceptance: RequestEntityAcceptance): HttpMethod = { - require(name.nonEmpty, "value must be non-empty") + validateHttpMethodName(name) require(!safe || idempotent, "An HTTP method cannot be safe without being idempotent") apply(name, safe, idempotent, requestEntityAcceptance, oldContentLengthCondition) } @@ -63,7 +71,7 @@ object HttpMethod { * Create a custom method type. */ def custom(name: String, safe: Boolean, idempotent: Boolean, requestEntityAcceptance: RequestEntityAcceptance, contentLengthAllowed: Boolean): HttpMethod = { - require(name.nonEmpty, "value must be non-empty") + validateHttpMethodName(name) require(!safe || idempotent, "An HTTP method cannot be safe without being idempotent") // use constant functions so custom HttpMethod instances are equal (case class equality) when built with the same parameters apply(name, safe, idempotent, requestEntityAcceptance, if (contentLengthAllowed) ConstantFun.anyToTrue else ConstantFun.anyToFalse) diff --git a/akka-http-core/src/test/scala/akka/http/scaladsl/model/HttpMethodSpec.scala b/akka-http-core/src/test/scala/akka/http/scaladsl/model/HttpMethodSpec.scala new file mode 100644 index 0000000000..7d106533ca --- /dev/null +++ b/akka-http-core/src/test/scala/akka/http/scaladsl/model/HttpMethodSpec.scala @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2009-2024 Lightbend Inc. + */ + +package akka.http.scaladsl.model + +import org.scalatest.matchers.should.Matchers +import org.scalatest.wordspec.AnyWordSpec + +class HttpMethodSpec extends AnyWordSpec with Matchers { + "HttpMethod.custom()" should { + "accept a valid name" in { + HttpMethod.custom("Yes.Thi$_is_1~'VAL|D`_me+h*d-^ame!#%&") + } + "validate that an invalid character is not present" in { + an[Exception] should be thrownBy HttpMethod.custom("INJECT /path HTTP/1.1\r\n") + an[Exception] should be thrownBy HttpMethod.custom("INJECT\r\n") + an[Exception] should be thrownBy HttpMethod.custom("INJECT ") + } + } +}