fix akka#4213: Content-Length when entity is unallowed

.gitignore

@@ -1,4 +1,12 @@
+# IntelliJ
 /.idea
+
+# VS Code
+**/metals.sbt
+**/.bloop
+.metals
+settings.json
+
 .DS_Store
 target
 

akka-http-core/src/main/java/akka/http/javadsl/model/HttpMethods.java

@@ -28,11 +28,11 @@ private HttpMethods() {}
     /**
      * Create a custom method type.
      */
-    public static HttpMethod custom(String value, boolean safe, boolean idempotent, akka.http.javadsl.model.RequestEntityAcceptance requestEntityAcceptance) {
+    public static HttpMethod custom(String value, boolean safe, boolean idempotent, akka.http.javadsl.model.RequestEntityAcceptance requestEntityAcceptance, boolean contentLengthAllowed) {
         //This cast is safe as implementation of RequestEntityAcceptance only exists in Scala
         akka.http.scaladsl.model.RequestEntityAcceptance scalaRequestEntityAcceptance
           = (akka.http.scaladsl.model.RequestEntityAcceptance) requestEntityAcceptance;
-        return akka.http.scaladsl.model.HttpMethod.custom(value, safe, idempotent, scalaRequestEntityAcceptance);
+        return akka.http.scaladsl.model.HttpMethod.custom(value, safe, idempotent, scalaRequestEntityAcceptance, contentLengthAllowed);
     }
 
     /**

akka-http-core/src/main/scala/akka/http/impl/engine/rendering/HttpResponseRendererFactory.scala

@@ -234,7 +234,7 @@ private[http] class HttpResponseRendererFactory(
           }
 
           def renderContentLengthHeader(contentLength: Long) =
-            if (status.allowsEntity) r ~~ ContentLengthBytes ~~ contentLength ~~ CrLf else r
+            if (ctx.requestMethod.contentLengthAllowed(status)) r ~~ ContentLengthBytes ~~ contentLength ~~ CrLf else r
 
           def headersAndEntity(entityBytes: => Source[ByteString, Any]): StrictOrStreamed =
             if (noEntity) {

akka-http-core/src/main/scala/akka/http/scaladsl/model/HttpMethod.scala

@@ -32,41 +32,52 @@ object RequestEntityAcceptance {
  * @param requestEntityAcceptance Expected if meaning of request entities is properly defined
  */
 final case class HttpMethod private[http] (
-  override val value:      String,
-  isSafe:                  Boolean,
-  isIdempotent:            Boolean,
-  requestEntityAcceptance: RequestEntityAcceptance) extends jm.HttpMethod with SingletonValueRenderable {
+  override val value:       String,
+  isSafe:                   Boolean,
+  isIdempotent:             Boolean,
+  requestEntityAcceptance:  RequestEntityAcceptance,
+  val contentLengthAllowed: StatusCode => Boolean) extends jm.HttpMethod with SingletonValueRenderable {
   override def isEntityAccepted: Boolean = requestEntityAcceptance.isEntityAccepted
   override def toString: String = s"HttpMethod($value)"
 }
 
 object HttpMethod {
-  def custom(name: String, safe: Boolean, idempotent: Boolean, requestEntityAcceptance: RequestEntityAcceptance): HttpMethod = {
+  def custom(name: String, safe: Boolean, idempotent: Boolean, requestEntityAcceptance: RequestEntityAcceptance, contentLengthAllowed: Boolean): HttpMethod = {
     require(name.nonEmpty, "value must be non-empty")
     require(!safe || idempotent, "An HTTP method cannot be safe without being idempotent")
-    apply(name, safe, idempotent, requestEntityAcceptance)
+    apply(name, safe, idempotent, requestEntityAcceptance, (_) => contentLengthAllowed)
   }
 
   /**
    * Creates a custom method by name and assumes properties conservatively to be
-   * safe = false, idempotent = false and requestEntityAcceptance = Expected.
+   * safe = false, idempotent = false, requestEntityAcceptance = Expected and contentLengthAllowed always true.
    */
-  def custom(name: String): HttpMethod = custom(name, safe = false, idempotent = false, requestEntityAcceptance = Expected)
+  def custom(name: String): HttpMethod = custom(name, safe = false, idempotent = false, requestEntityAcceptance = Expected, contentLengthAllowed = true)
 }
 
 object HttpMethods extends ObjectRegistry[String, HttpMethod] {
   private def register(method: HttpMethod): HttpMethod = register(method.value, method)
 
+  // define requirements for content-length according to https://httpwg.org/specs/rfc9110.html#field.content-length
+  // for CONNECT it is explicitly not allowed in the 2xx (Successful) range
+  private def contentLengthAllowedForConnect(forStatus: StatusCode): Boolean = forStatus.intValue < 200 || forStatus.intValue >= 300
+  // for HEAD it is technically allowed, but must match the content-length of hypothetical GET request, so can not be anticipated
+  private def contentLengthAllowedForHead(forStatus: StatusCode): Boolean = false
+  // for other methods there are common rules:
+  // - for 304 (Not Modified) it must match the content-length of hypothetical 200-accepted request, so can not be anticipated
+  // - for 1xx (Informational) or 204 (No Content) it is explicitly not allowed
+  private def contentLengthAllowedCommon(forStatus: StatusCode): Boolean = forStatus.intValue >= 200 && !(forStatus eq StatusCodes.NoContent) && !(forStatus eq StatusCodes.NotModified)
+
   // format: OFF
-  val CONNECT = register(HttpMethod("CONNECT", isSafe = false, isIdempotent = false, requestEntityAcceptance = Disallowed))
-  val DELETE  = register(HttpMethod("DELETE" , isSafe = false, isIdempotent = true , requestEntityAcceptance = Tolerated))
-  val GET     = register(HttpMethod("GET"    , isSafe = true , isIdempotent = true , requestEntityAcceptance = Tolerated))
-  val HEAD    = register(HttpMethod("HEAD"   , isSafe = true , isIdempotent = true , requestEntityAcceptance = Disallowed))
-  val OPTIONS = register(HttpMethod("OPTIONS", isSafe = true , isIdempotent = true , requestEntityAcceptance = Expected))
-  val PATCH   = register(HttpMethod("PATCH"  , isSafe = false, isIdempotent = false, requestEntityAcceptance = Expected))
-  val POST    = register(HttpMethod("POST"   , isSafe = false, isIdempotent = false, requestEntityAcceptance = Expected))
-  val PUT     = register(HttpMethod("PUT"    , isSafe = false, isIdempotent = true , requestEntityAcceptance = Expected))
-  val TRACE   = register(HttpMethod("TRACE"  , isSafe = true , isIdempotent = true , requestEntityAcceptance = Disallowed))
+  val CONNECT = register(HttpMethod("CONNECT", isSafe = false, isIdempotent = false, requestEntityAcceptance = Disallowed, contentLengthAllowed = contentLengthAllowedForConnect))
+  val DELETE  = register(HttpMethod("DELETE" , isSafe = false, isIdempotent = true , requestEntityAcceptance = Tolerated, contentLengthAllowed = contentLengthAllowedCommon))
+  val GET     = register(HttpMethod("GET"    , isSafe = true , isIdempotent = true , requestEntityAcceptance = Tolerated, contentLengthAllowed = contentLengthAllowedCommon))
+  val HEAD    = register(HttpMethod("HEAD"   , isSafe = true , isIdempotent = true , requestEntityAcceptance = Disallowed, contentLengthAllowed = contentLengthAllowedForHead))
+  val OPTIONS = register(HttpMethod("OPTIONS", isSafe = true , isIdempotent = true , requestEntityAcceptance = Expected, contentLengthAllowed = contentLengthAllowedCommon))
+  val PATCH   = register(HttpMethod("PATCH"  , isSafe = false, isIdempotent = false, requestEntityAcceptance = Expected, contentLengthAllowed = contentLengthAllowedCommon))
+  val POST    = register(HttpMethod("POST"   , isSafe = false, isIdempotent = false, requestEntityAcceptance = Expected, contentLengthAllowed = contentLengthAllowedCommon))
+  val PUT     = register(HttpMethod("PUT"    , isSafe = false, isIdempotent = true , requestEntityAcceptance = Expected, contentLengthAllowed = contentLengthAllowedCommon))
+  val TRACE   = register(HttpMethod("TRACE"  , isSafe = true , isIdempotent = true , requestEntityAcceptance = Disallowed, contentLengthAllowed = contentLengthAllowedCommon))
   // format: ON
 
   override def getForKeyCaseInsensitive(key: String)(implicit conv: String <:< String): Option[HttpMethod] =

akka-http-core/src/test/scala/akka/http/impl/engine/parsing/RequestParserSpec.scala

@@ -46,7 +46,7 @@ abstract class RequestParserSpec(mode: String, newLine: String) extends AnyFreeS
   implicit val system: ActorSystem = ActorSystem(getClass.getSimpleName, testConf)
   import system.dispatcher
 
-  val BOLT = HttpMethod.custom("BOLT", safe = false, idempotent = true, requestEntityAcceptance = Expected)
+  val BOLT = HttpMethod.custom("BOLT", safe = false, idempotent = true, requestEntityAcceptance = Expected, contentLengthAllowed = true)
 
   s"The request parsing logic should (mode: $mode)" - {
     "properly parse a request" - {

akka-http-core/src/test/scala/akka/http/impl/engine/rendering/ResponseRendererSpec.scala

@@ -58,7 +58,32 @@ class ResponseRendererSpec extends AnyFreeSpec with Matchers with BeforeAndAfter
         }
       }
 
-      "status 304 and a few headers" in new TestSetup() {
+      "status 204 and a few headers (does not add content-length)" in new TestSetup() {
+        HttpResponse(204, List(RawHeader("X-Fancy", "of course"), Age(0))) should renderTo {
+          """HTTP/1.1 204 No Content
+            |X-Fancy: of course
+            |Age: 0
+            |Server: akka-http/1.0.0
+            |Date: Thu, 25 Aug 2011 09:10:29 GMT
+            |
+            |"""
+        }
+      }
+
+      "status 205 and a few headers (adds content-length)" in new TestSetup() {
+        HttpResponse(205, List(RawHeader("X-Fancy", "of course"), Age(0))) should renderTo {
+          """HTTP/1.1 205 Reset Content
+            |X-Fancy: of course
+            |Age: 0
+            |Server: akka-http/1.0.0
+            |Date: Thu, 25 Aug 2011 09:10:29 GMT
+            |Content-Length: 0
+            |
+            |"""
+        }
+      }
+
+      "status 304 and a few headers (does not add content-length)" in new TestSetup() {
         HttpResponse(304, List(RawHeader("X-Fancy", "of course"), Age(0))) should renderTo {
           """HTTP/1.1 304 Not Modified
             |X-Fancy: of course
@@ -69,6 +94,7 @@ class ResponseRendererSpec extends AnyFreeSpec with Matchers with BeforeAndAfter
             |"""
         }
       }
+
       "a custom status code and no headers" in new TestSetup() {
         HttpResponse(ServerOnTheMove) should renderTo {
           """HTTP/1.1 330 Server on the move
@@ -97,6 +123,18 @@ class ResponseRendererSpec extends AnyFreeSpec with Matchers with BeforeAndAfter
         override def currentTimeMillis() = initial + extraMillis
       }
 
+      "no Content-Length on CONNECT method" in new TestSetup() {
+        ResponseRenderingContext(
+          requestMethod = HttpMethods.CONNECT,
+          response = HttpResponse(headers = List(Age(30), Connection("Keep-Alive")))) should renderTo(
+            """HTTP/1.1 200 OK
+              |Age: 30
+              |Server: akka-http/1.0.0
+              |Date: Thu, 25 Aug 2011 09:10:29 GMT
+              |
+              |""")
+      }
+
       "to a transparent HEAD request (Strict response entity)" in new TestSetup() {
         ResponseRenderingContext(
           requestMethod = HttpMethods.HEAD,
@@ -108,7 +146,6 @@ class ResponseRendererSpec extends AnyFreeSpec with Matchers with BeforeAndAfter
               |Server: akka-http/1.0.0
               |Date: Thu, 25 Aug 2011 09:10:29 GMT
               |Content-Type: text/plain; charset=UTF-8
-              |Content-Length: 23
               |
               |""", close = false)
       }

docs/src/test/scala/docs/http/scaladsl/server/directives/CustomHttpMethodSpec.scala

@@ -27,7 +27,7 @@ class CustomHttpMethodSpec extends AkkaSpec with ScalaFutures
 
       // define custom method type:
       val BOLT = HttpMethod.custom("BOLT", safe = false,
-        idempotent = true, requestEntityAcceptance = Expected)
+        idempotent = true, requestEntityAcceptance = Expected, contentLengthAllowed = true)
 
       // add custom method to parser settings:
       val parserSettings = ParserSettings.forServer(system).withCustomMethods(BOLT)