This is an automated email from the ASF dual-hosted git repository.
fanningpj pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko-http.git
The following commit(s) were added to refs/heads/main by this push:
new 1a65bf1a5 fix: correct Content-Length rendering based on method+status
(#962)
1a65bf1a5 is described below
commit 1a65bf1a581530ed506a2260885edcc8c4fa6142
Author: PJ Fanning <[email protected]>
AuthorDate: Thu Feb 26 09:54:55 2026 +0100
fix: correct Content-Length rendering based on method+status (#962)
* Initial plan
* fix: Content-Length when entity is unallowed (port of akka/akka-http#4214)
Co-authored-by: pjfanning <[email protected]>
* mima check and small adjustments
* scalafmt
* javafmt
---------
Co-authored-by: copilot-swe-agent[bot]
<[email protected]>
Co-authored-by: pjfanning <[email protected]>
---
.../directives/CustomHttpMethodExamplesTest.java | 2 +-
.../server/directives/CustomHttpMethodSpec.scala | 2 +-
.../pekko/http/javadsl/model/HttpMethods.java | 28 +++++++++-
.../custom-content-type.backwards.excludes | 21 ++++++++
.../rendering/HttpResponseRendererFactory.scala | 2 +-
.../pekko/http/scaladsl/model/HttpMethod.scala | 62 +++++++++++++++++-----
.../engine/client/HostConnectionPoolSpec.scala | 4 +-
.../impl/engine/parsing/RequestParserSpec.scala | 3 +-
.../engine/rendering/ResponseRendererSpec.scala | 43 +++++++++++++--
.../http/impl/engine/server/HttpServerSpec.scala | 2 -
10 files changed, 143 insertions(+), 26 deletions(-)
diff --git
a/docs/src/test/java/docs/http/javadsl/server/directives/CustomHttpMethodExamplesTest.java
b/docs/src/test/java/docs/http/javadsl/server/directives/CustomHttpMethodExamplesTest.java
index ea72a286c..017d199b5 100644
---
a/docs/src/test/java/docs/http/javadsl/server/directives/CustomHttpMethodExamplesTest.java
+++
b/docs/src/test/java/docs/http/javadsl/server/directives/CustomHttpMethodExamplesTest.java
@@ -54,7 +54,7 @@ public class CustomHttpMethodExamplesTest extends
JUnitRouteTest {
// #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);
diff --git
a/docs/src/test/scala/docs/http/scaladsl/server/directives/CustomHttpMethodSpec.scala
b/docs/src/test/scala/docs/http/scaladsl/server/directives/CustomHttpMethodSpec.scala
index b8f093bac..a619ba0c7 100644
---
a/docs/src/test/scala/docs/http/scaladsl/server/directives/CustomHttpMethodSpec.scala
+++
b/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)
diff --git
a/http-core/src/main/java/org/apache/pekko/http/javadsl/model/HttpMethods.java
b/http-core/src/main/java/org/apache/pekko/http/javadsl/model/HttpMethods.java
index e2d6f962b..46dfa32f8 100644
---
a/http-core/src/main/java/org/apache/pekko/http/javadsl/model/HttpMethods.java
+++
b/http-core/src/main/java/org/apache/pekko/http/javadsl/model/HttpMethods.java
@@ -34,7 +34,15 @@ public final class 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 final class HttpMethods {
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(
diff --git
a/http-core/src/main/mima-filters/1.4.x.backwards.excludes/custom-content-type.backwards.excludes
b/http-core/src/main/mima-filters/1.4.x.backwards.excludes/custom-content-type.backwards.excludes
new file mode 100644
index 000000000..92e5eec39
--- /dev/null
+++
b/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")
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/rendering/HttpResponseRendererFactory.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/rendering/HttpResponseRendererFactory.scala
index 56f1570e7..d869da4ae 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/rendering/HttpResponseRendererFactory.scala
+++
b/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) {
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/HttpMethod.scala
b/http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/HttpMethod.scala
index e3e0a29b8..a00caae9b 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/HttpMethod.scala
+++
b/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] =
diff --git
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/client/HostConnectionPoolSpec.scala
b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/client/HostConnectionPoolSpec.scala
index d931c3034..a0446db4e 100644
---
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/client/HostConnectionPoolSpec.scala
+++
b/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()
diff --git
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala
b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala
index d2dcf9e26..161dda7e0 100644
---
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala
+++
b/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" - {
diff --git
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/rendering/ResponseRendererSpec.scala
b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/rendering/ResponseRendererSpec.scala
index 10e08f97a..d2fa564ff 100644
---
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/rendering/ResponseRendererSpec.scala
+++
b/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))
}
}
diff --git
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/server/HttpServerSpec.scala
b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/server/HttpServerSpec.scala
index 6d73f2967..85e5e145c 100644
---
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/server/HttpServerSpec.scala
+++
b/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
|
|""")
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
