Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate custom HTTP method names #4461

Merged
merged 1 commit into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For custom headers received by the server, there is no need to validate them because they have already been parsed according to the spec, so skip the validation in HttpMethod.custom.

}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright (C) 2009-2024 Lightbend Inc. <https://www.lightbend.com>
*/

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!#%&")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most reasonable custom method name I ever saw :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's specified in RFC666 - Gaslighting HTTP resources.

}
"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 ")
}
}
}