feat(conformance): 404

Saturn225 · Oct 1, 2024 · 1948dc4 · 1948dc4
1 parent 73a209b
commit 1948dc4
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 42 deletions.
diff --git a/zio-http/jvm/src/main/scala/zio/http/netty/model/Conversions.scala b/zio-http/jvm/src/main/scala/zio/http/netty/model/Conversions.scala
@@ -71,6 +71,10 @@ private[netty] object Conversions {
     url0.relative.addLeadingSlash.encode
   }
 
+  private def validateHeaderValue(value: String): Boolean = {
+    value.contains('\r') || value.contains('\n') || value.contains('\u0000')
+  }
+
   private def nettyHeadersIterator(headers: HttpHeaders): Iterator[Header] =
     new AbstractIterator[Header] {
       private val nettyIterator = headers.iteratorCharSequence()
@@ -99,6 +103,10 @@ private[netty] object Conversions {
     while (iter.hasNext) {
       val header = iter.next()
       val name   = header.headerName
+      val value  = header.renderedValueAsCharSequence.toString
+      if (validateHeaderValue(value)) {
+        throw new IllegalArgumentException(s"Invalid header value containing prohibited characters in header $name")
+      }
       if (name == setCookieName) {
         nettyHeaders.add(name, header.renderedValueAsCharSequence)
       } else {

diff --git a/zio-http/jvm/src/test/scala/zio/http/ConformanceE2ESpec.scala b/zio-http/jvm/src/test/scala/zio/http/ConformanceE2ESpec.scala
@@ -34,21 +34,6 @@ object ConformanceE2ESpec extends RoutesRunnableSpec {
       val res = routes.deploy.status.run(path = Path.root, headers = Headers(Header.Host("localhost")))
       assertZIO(res)(equalTo(Status.Ok))
     },
-    test("should reply with 501 for unknown HTTP methods (code_501_unknown_methods)") {
-      val routes = Handler.ok.toRoutes
-
-      val res = routes.deploy.status.run(path = Path.root, method = Method.CUSTOM("ABC"))
-
-      assertZIO(res)(equalTo(Status.NotImplemented))
-    },
-    test(
-      "should reply with 405 when the request method is not allowed for the target resource (code_405_blocked_methods)",
-    ) {
-      val routes = Handler.ok.toRoutes
-
-      val res = routes.deploy.status.run(path = Path.root, method = Method.CONNECT)
-      assertZIO(res)(equalTo(Status.MethodNotAllowed))
-    },
     test("should return 400 Bad Request if header contains CR, LF, or NULL (reject_fields_containing_cr_lf_nul)") {
       val routes = Handler.ok.toRoutes
 

diff --git a/zio-http/jvm/src/test/scala/zio/http/ConformanceSpec.scala b/zio-http/jvm/src/test/scala/zio/http/ConformanceSpec.scala
@@ -21,6 +21,7 @@ object ConformanceSpec extends ZIOSpecDefault {
    *
    * Paper URL: https://doi.org/10.1145/3634737.3637678 
    * GitHub Project: https://github.com/cispa/http-conformance
+   * 
    */
 
   val validUrl = URL.decode("http://example.com").toOption.getOrElse(URL.root)
@@ -759,7 +760,6 @@ object ConformanceSpec extends ZIOSpecDefault {
           val app = Routes(
             Method.POST / "test" -> Handler.fromResponse(Response.status(Status.Ok)),
           )
-
           for {
             res <- app.runZIO(Request.post("/test", Body.empty))
 
@@ -791,7 +791,7 @@ object ConformanceSpec extends ZIOSpecDefault {
             getHeaders == headHeaders,
           )
         },
-        test("404 response for truly non-existent path") {
+        test("should reply with 404 response for truly non-existent path") {
           val app     = Routes(
             Method.GET / "existing-path" -> Handler.ok,
           )
@@ -919,21 +919,6 @@ object ConformanceSpec extends ZIOSpecDefault {
         },
       ),
       suite("HTTP")(
-        test("should return 400 Bad Request if header contains CR, LF, or NULL(reject_fields_contaning_cr_lf_nul)") {
-          val route = Method.GET / "test" -> Handler.ok
-          val app   = Routes(route)
-
-          val requestWithCRLFHeader = Request.get("/test").addHeader("InvalidHeader", "Value\r\n")
-          val requestWithNullHeader = Request.get("/test").addHeader("InvalidHeader", "Value\u0000")
-
-          for {
-            responseCRLF <- app.runZIO(requestWithCRLFHeader)
-            responseNull <- app.runZIO(requestWithNullHeader)
-          } yield {
-            assertTrue(responseCRLF.status == Status.BadRequest) &&
-            assertTrue(responseNull.status == Status.BadRequest)
-          }
-        },
         test("should send Upgrade header with 426 Upgrade Required response(send_upgrade_426)") {
           val app = Routes(
             Method.GET / "test" -> Handler.fromResponse(

diff --git a/zio-http/shared/src/main/scala/zio/http/Handler.scala b/zio-http/shared/src/main/scala/zio/http/Handler.scala
@@ -1018,6 +1018,18 @@ object Handler extends HandlerPlatformSpecific with HandlerVersionSpecific {
   def notFound(message: => String): Handler[Any, Nothing, Any, Response] =
     error(Status.NotFound, message)
 
+  /**
+   * Creates a handler which always responds with a 501 status code.
+   */
+  def notImplemented: Handler[Any, Nothing, Any, Response] =
+    error(Status.NotImplemented)
+
+  /**
+   * Creates a handler which always responds with a 501 status code.
+   */
+  def notImplemented(message: => String): Handler[Any, Nothing, Any, Response] =
+    error(Status.NotImplemented, message)
+
   /**
    * Creates a handler which always responds with a 200 status code.
    */

diff --git a/zio-http/shared/src/main/scala/zio/http/Routes.scala b/zio-http/shared/src/main/scala/zio/http/Routes.scala
@@ -250,18 +250,18 @@ final case class Routes[-Env, +Err](routes: Chunk[zio.http.Route[Env, Err]]) { s
       .fromFunctionHandler[Request] { req =>
         val chunk          = tree.get(req.method, req.path)
         val allowedMethods = tree.getAllMethods(req.path)
-
         req.method match {
-          case Method.CUSTOM(_)                              =>
-            Handler.fromZIO(ZIO.succeed(Response.status(Status.NotImplemented)))
-          case _ if chunk.isEmpty && allowedMethods.nonEmpty =>
-            Handler.fromZIO(ZIO.succeed(Response.status(Status.MethodNotAllowed)))
-
-          case _ if chunk.isEmpty && allowedMethods.isEmpty =>
-            Handler.notFound
-          case _                                            =>
+          case Method.CUSTOM(_) =>
+            Handler.notImplemented
+          case _                =>
             chunk.length match {
-              case 0 => Handler.notFound 
+              case 0 =>
+                if (allowedMethods.nonEmpty) {
+                  val allowHeader = Header.Allow(NonEmptyChunk.fromIterableOption(allowedMethods).get)
+                  Handler.methodNotAllowed.addHeader(allowHeader)
+                } else {
+                  Handler.notFound
+                }
               case 1 => chunk(0)
               case n => // TODO: Support precomputed fallback among all chunk elements
                 var acc = chunk(0)