fix: correct Content-Length rendering based on method+status

docs/src/test/java/docs/http/javadsl/server/directives/CustomHttpMethodExamplesTest.java

@@ -54,7 +54,7 @@ public void testComposition() throws InterruptedException, ExecutionException, T
     // #customHttpMethod
 
     // define custom method type:
-    HttpMethod BOLT = HttpMethods.custom("BOLT", false, true, Expected);
+    HttpMethod BOLT = HttpMethods.custom("BOLT", false, true, Expected, true);
 
     // add custom method to parser settings:
     final ParserSettings parserSettings = ParserSettings.forServer(system).withCustomMethods(BOLT);

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

@@ -37,7 +37,7 @@ class CustomHttpMethodSpec extends PekkoSpec 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)

http-core/src/main/java/org/apache/pekko/http/javadsl/model/HttpMethods.java

@@ -34,7 +34,15 @@ private HttpMethods() {}
   public static final HttpMethod PUT = org.apache.pekko.http.scaladsl.model.HttpMethods.PUT();
   public static final HttpMethod TRACE = org.apache.pekko.http.scaladsl.model.HttpMethods.TRACE();
 
-  /** Create a custom method type. */
+  /**
+   * Create a custom method type.
+   *
+   * @deprecated The created method will compute the presence of Content-Length headers based on
+   *     deprecated logic, use {@link #custom(String, boolean, boolean,
+   *     org.apache.pekko.http.javadsl.model.RequestEntityAcceptance, boolean)} instead. Deprecated
+   *     since 1.4.0.
+   */
+  @Deprecated
   public static HttpMethod custom(
       String value,
       boolean safe,
@@ -47,6 +55,24 @@ public static HttpMethod custom(
         value, safe, idempotent, scalaRequestEntityAcceptance);
   }
 
+  /**
+   * Create a custom method type.
+   *
+   * @since 1.4.0
+   */
+  public static HttpMethod custom(
+      String value,
+      boolean safe,
+      boolean idempotent,
+      org.apache.pekko.http.javadsl.model.RequestEntityAcceptance requestEntityAcceptance,
+      boolean contentLengthAllowed) {
+    // This cast is safe as implementation of RequestEntityAcceptance only exists in Scala
+    org.apache.pekko.http.scaladsl.model.RequestEntityAcceptance scalaRequestEntityAcceptance =
+        (org.apache.pekko.http.scaladsl.model.RequestEntityAcceptance) requestEntityAcceptance;
+    return org.apache.pekko.http.scaladsl.model.HttpMethod.custom(
+        value, safe, idempotent, scalaRequestEntityAcceptance, contentLengthAllowed);
+  }
+
   /** Looks up a predefined HTTP method with the given name. */
   public static Optional<HttpMethod> lookup(String name) {
     return Util.<HttpMethod, org.apache.pekko.http.scaladsl.model.HttpMethod>lookupInRegistry(

http-core/src/main/mima-filters/1.4.x.backwards.excludes/custom-content-type.backwards.excludes

@@ -0,0 +1,21 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# changes related to controlling content length for custom content types
+ProblemFilters.exclude[IncompatibleSignatureProblem]("org.apache.pekko.http.scaladsl.model.HttpMethod.unapply")
+ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.scaladsl.model.HttpMethod.apply")
+ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.scaladsl.model.HttpMethod.copy")

http-core/src/main/scala/org/apache/pekko/http/impl/engine/rendering/HttpResponseRendererFactory.scala

@@ -253,7 +253,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) {

http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/HttpMethod.scala

@@ -19,6 +19,7 @@ import org.apache.pekko
 import pekko.http.impl.util._
 import pekko.http.javadsl.{ model => jm }
 import pekko.http.scaladsl.model.RequestEntityAcceptance._
+import pekko.util.ConstantFun._
 
 sealed trait RequestEntityAcceptance extends jm.RequestEntityAcceptance {
   def isEntityAccepted: Boolean
@@ -40,45 +41,80 @@ object RequestEntityAcceptance {
  * @param isSafe true if the resource should not be altered on the server
  * @param isIdempotent true if requests can be safely (& automatically) repeated
  * @param requestEntityAcceptance Expected if meaning of request entities is properly defined
+ * @param contentLengthAllowed Function defining whether the method-statuscode combination should output the Content-Length header.
  */
 final case class HttpMethod private[http] (
     override val value: String,
     isSafe: Boolean,
     isIdempotent: Boolean,
-    requestEntityAcceptance: RequestEntityAcceptance) extends jm.HttpMethod with SingletonValueRenderable {
+    requestEntityAcceptance: RequestEntityAcceptance,
+    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 the fix
+  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.
+   */
+  @deprecated("Use the overload with contentLengthAllowed parameter", since = "1.4.0")
   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)
+  }
+
+  /**
+   * 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) anyToTrue else anyToFalse)
   }
 
   /**
    * 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)
+    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] =

http-core/src/test/scala/org/apache/pekko/http/impl/engine/client/HostConnectionPoolSpec.scala

@@ -223,7 +223,7 @@ class HostConnectionPoolSpec extends PekkoSpecWithMaterializer(
             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
 
@@ -242,7 +242,7 @@ class HostConnectionPoolSpec extends PekkoSpecWithMaterializer(
             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()

http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala

@@ -57,7 +57,8 @@ 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" - {

http-core/src/test/scala/org/apache/pekko/http/impl/engine/rendering/ResponseRendererSpec.scala

@@ -68,7 +68,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: pekko-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: pekko-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
@@ -107,6 +132,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: pekko-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,
@@ -118,7 +155,6 @@ class ResponseRendererSpec extends AnyFreeSpec with Matchers with BeforeAndAfter
               |Server: pekko-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)
       }
@@ -170,7 +206,6 @@ class ResponseRendererSpec extends AnyFreeSpec with Matchers with BeforeAndAfter
               |Server: pekko-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)
       }
@@ -679,7 +714,7 @@ class ResponseRendererSpec extends AnyFreeSpec with Matchers with BeforeAndAfter
                  |Server: pekko-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))
     }
   }

http-core/src/test/scala/org/apache/pekko/http/impl/engine/server/HttpServerSpec.scala

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