fix #4213: Content-Length according to RFC9110

.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

@@ -27,14 +27,26 @@ private HttpMethods() {}
 
     /**
      * 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
     public static HttpMethod custom(String value, boolean safe, boolean idempotent, akka.http.javadsl.model.RequestEntityAcceptance requestEntityAcceptance) {
         //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);
     }
 
+    /**
+     * Create a custom method type.
+     */
+    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, contentLengthAllowed);
+    }
+
     /**
      * Looks up a predefined HTTP method with the given name.
      */

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,77 @@ 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 {
+
+  // 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
+
+  /**
+   * 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")
     require(!safe || idempotent, "An HTTP method cannot be safe without being idempotent")
-    apply(name, safe, idempotent, requestEntityAcceptance)
+    apply(name, safe, idempotent, requestEntityAcceptance, oldContentLengthCondition)
+  }
+
+  // declare constant functions so custom HttpMethod instances are equal (case class equality) when built with the same parameters
+  private val constantTrue: StatusCode => Boolean = (_) => true
+  private val constantFalse: StatusCode => Boolean = (_) => false
+
+  /**
+   * 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")
+    require(!safe || idempotent, "An HTTP method cannot be safe without being idempotent")
+    apply(name, safe, idempotent, requestEntityAcceptance, if (contentLengthAllowed) constantTrue else constantFalse)
   }
 
   /**
    * 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 1xx (Informational) or 204 (No Content) it is explicitly not allowed
+  // - for 304 (Not Modified) it must match the content-length of hypothetical 200-accepted request, so can not be anticipated
+  private def contentLengthAllowedCommon(forStatus: StatusCode): Boolean = {
+    val code = forStatus.intValue
+    !(code < 200 || code == 204 || code == 304)
+  }
+
   // 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/client/HostConnectionPoolSpec.scala

@@ -200,7 +200,7 @@ class HostConnectionPoolSpec extends AkkaSpecWithMaterializer(
 
           conn1.pushResponse(HttpResponse(entity = HttpEntity.Default(ContentTypes.`application/octet-stream`, 100, Source.empty)))
           val res = expectResponse()
-          res.entity.contentLengthOption.get shouldEqual 100
+          res.entity.contentLengthOption.get shouldEqual 0
 
           // HEAD requests do not require to consume entity
 
@@ -217,7 +217,7 @@ class HostConnectionPoolSpec extends AkkaSpecWithMaterializer(
 
           conn1.pushResponse(HttpResponse(entity = HttpEntity.Default(ContentTypes.`application/octet-stream`, 100, Source.empty)))
           val res = expectResponse()
-          res.entity.contentLengthOption.get shouldEqual 100
+          res.entity.contentLengthOption.get shouldEqual 0
 
           // HEAD requests do not require consumption of entity but users might do anyway
           res.entity.discardBytes()

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
+              |
+              |""", close = false)
+      }
+
       "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)
       }
@@ -159,7 +196,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: 100
               |
               |""", close = false)
       }
@@ -661,7 +697,7 @@ class ResponseRendererSpec extends AnyFreeSpec with Matchers with BeforeAndAfter
                  |Server: akka-http/1.0.0
                  |Date: Thu, 25 Aug 2011 09:10:29 GMT
                  |${renCH.fold("")(_.toString + "\n")}Content-Type: text/plain; charset=UTF-8
-                 |${if (resCD) "" else "Content-Length: 6\n"}
+                 |${if (headReq || resCD) "" else "Content-Length: 6\n"}
                  |${if (headReq) "" else "ENTITY"}""", close))
     }
   }

akka-http-core/src/test/scala/akka/http/impl/engine/server/HttpServerSpec.scala

@@ -498,7 +498,6 @@ class HttpServerSpec extends AkkaSpec(
                |Server: akka-http/test
                |Date: XXXX
                |Content-Type: text/plain; charset=UTF-8
-               |Content-Length: 4
                |
                |""")
       }
@@ -524,7 +523,6 @@ class HttpServerSpec extends AkkaSpec(
                |Server: akka-http/test
                |Date: XXXX
                |Content-Type: text/plain; charset=UTF-8
-               |Content-Length: 4
                |
                |""")
       }

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)