From ab3bcabfb8481c689723098ec7405b7b81250208 Mon Sep 17 00:00:00 2001
From: Antoine Bursaux <bursauxa@gmail.com>
Date: Thu, 12 Jan 2023 15:34:01 +0100
Subject: [PATCH] fix #4213: Content-Length when entity is unallowed

---
 .gitignore                                    |  8 ++++
 .../HttpResponseRendererFactory.scala         | 17 +++++++-
 .../rendering/ResponseRendererSpec.scala      | 41 ++++++++++++++++++-
 3 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/.gitignore b/.gitignore
index d57c4ee89e9..a4336c12e4e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,4 +1,12 @@
+# IntelliJ
 /.idea
+
+# VS Code
+**/metals.sbt
+**/.bloop
+.metals
+settings.json
+
 .DS_Store
 target
 
diff --git a/akka-http-core/src/main/scala/akka/http/impl/engine/rendering/HttpResponseRendererFactory.scala b/akka-http-core/src/main/scala/akka/http/impl/engine/rendering/HttpResponseRendererFactory.scala
index a29707e1b87..294bd29a72f 100644
--- a/akka-http-core/src/main/scala/akka/http/impl/engine/rendering/HttpResponseRendererFactory.scala
+++ b/akka-http-core/src/main/scala/akka/http/impl/engine/rendering/HttpResponseRendererFactory.scala
@@ -233,8 +233,21 @@ private[http] class HttpResponseRendererFactory(
               r ~~ `Transfer-Encoding` ~~ ChunkedBytes ~~ CrLf
           }
 
-          def renderContentLengthHeader(contentLength: Long) =
-            if (status.allowsEntity) r ~~ ContentLengthBytes ~~ contentLength ~~ CrLf else r
+          def renderContentLengthHeader(contentLength: Long) = {
+            // evaluate requirements for content-length according to https://httpwg.org/specs/rfc9110.html#field.content-length
+            // - for HEAD it is technically allowed, but must match the content-length of hypothetical GET request, which is not handled here
+            // - for 304 (Not Modified) it must similarly match the content-length of hypothetical 200-accepted request, which is not handled here
+            // - for 1xx (Informational) or 204 (No Content) it is explicitly not allowed
+            // - for 2xx (Successful) it is explicitly not allowed when the method is CONNECT
+            val isContentLengthForbidden =
+              (ctx.requestMethod eq HttpMethods.HEAD) ||
+              (status eq StatusCodes.NotModified) ||
+              (100 to 199 contains status.intValue) ||
+              (status eq StatusCodes.NoContent) ||
+              ((ctx.requestMethod eq HttpMethods.CONNECT) && (200 to 299 contains status.intValue))
+
+            if (isContentLengthForbidden) r else r ~~ ContentLengthBytes ~~ contentLength ~~ CrLf
+          }
 
           def headersAndEntity(entityBytes: => Source[ByteString, Any]): StrictOrStreamed =
             if (noEntity) {
diff --git a/akka-http-core/src/test/scala/akka/http/impl/engine/rendering/ResponseRendererSpec.scala b/akka-http-core/src/test/scala/akka/http/impl/engine/rendering/ResponseRendererSpec.scala
index 69f776b18c3..f6edb0bbf91 100644
--- a/akka-http-core/src/test/scala/akka/http/impl/engine/rendering/ResponseRendererSpec.scala
+++ b/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)
       }