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 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); 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 @@ 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 lookup(String name) { return Util.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 | |""") }