diff --git a/.github/workflows/http-conformance.yml b/.github/workflows/http-conformance.yml index b4a5fc90ea..e99c42f029 100644 --- a/.github/workflows/http-conformance.yml +++ b/.github/workflows/http-conformance.yml @@ -1,4 +1,4 @@ -name: HTTP Spec Conformance Test +name: HTTP Conformance on: pull_request: @@ -70,4 +70,4 @@ jobs: cache: sbt - name: Run HTTP Conformance Tests - run: sbt "project zioHttpJVM" "testOnly zio.http.ConformanceSpec" + run: sbt "project zioHttpJVM" "testOnly zio.http.ConformanceSpec zio.http.ConformanceE2ESpec" diff --git a/zio-http/jvm/src/main/scala/zio/http/netty/server/ServerInboundHandler.scala b/zio-http/jvm/src/main/scala/zio/http/netty/server/ServerInboundHandler.scala index 74340d825e..a7f9d0ba91 100644 --- a/zio-http/jvm/src/main/scala/zio/http/netty/server/ServerInboundHandler.scala +++ b/zio-http/jvm/src/main/scala/zio/http/netty/server/ServerInboundHandler.scala @@ -87,12 +87,19 @@ private[zio] final case class ServerInboundHandler( ) releaseRequest() } else { - val req = makeZioRequest(ctx, jReq) - val exit = handler(req) - if (attemptImmediateWrite(ctx, req.method, exit)) { + val req = makeZioRequest(ctx, jReq) + if (!validateHostHeader(req)) { + attemptFastWrite(ctx, req.method, Response.status(Status.BadRequest)) releaseRequest() } else { - writeResponse(ctx, runtime, exit, req)(releaseRequest) + + val exit = handler(req) + if (attemptImmediateWrite(ctx, req.method, exit)) { + releaseRequest() + } else { + writeResponse(ctx, runtime, exit, req)(releaseRequest) + + } } } } finally { @@ -108,6 +115,34 @@ private[zio] final case class ServerInboundHandler( } + private def validateHostHeader(req: Request): Boolean = { + req.headers.get("Host") match { + case Some(host) => + val parts = host.split(":") + val hostname = parts(0) + val isValidHost = validateHostname(hostname) + val isValidPort = parts.length == 1 || (parts.length == 2 && parts(1).forall(_.isDigit)) + val isValid = isValidHost && isValidPort + println(s"Host: $host, isValidHost: $isValidHost, isValidPort: $isValidPort, isValid: $isValid") + isValid + case None => + println("Host header missing!") + false + } + } + +// Validate a regular hostname (based on RFC 1035) + private def validateHostname(hostname: String): Boolean = { + if (hostname.isEmpty || hostname.contains("_")) { + return false + } + val labels = hostname.split("\\.") + if (labels.exists(label => label.isEmpty || label.length > 63 || label.startsWith("-") || label.endsWith("-"))) { + return false + } + hostname.forall(c => c.isLetterOrDigit || c == '.' || c == '-') && hostname.length <= 253 + } + override def exceptionCaught(ctx: ChannelHandlerContext, cause: Throwable): Unit = cause match { case ioe: IOException if { @@ -262,7 +297,6 @@ private[zio] final case class ServerInboundHandler( remoteCertificate = clientCert, ) } - } /* diff --git a/zio-http/jvm/src/test/scala/zio/http/ConformanceE2ESpec.scala b/zio-http/jvm/src/test/scala/zio/http/ConformanceE2ESpec.scala new file mode 100644 index 0000000000..a6a61df9d1 --- /dev/null +++ b/zio-http/jvm/src/test/scala/zio/http/ConformanceE2ESpec.scala @@ -0,0 +1,103 @@ +package zio.http + +import zio._ +import zio.test.Assertion._ +import zio.test.TestAspect._ +import zio.test._ + +import zio.http._ +import zio.http.internal.{DynamicServer, RoutesRunnableSpec} +import zio.http.netty.NettyConfig + +object ConformanceE2ESpec extends RoutesRunnableSpec { + + private val port = 8080 + private val MaxSize = 1024 * 10 + val configApp = Server.Config.default + .requestDecompression(true) + .disableRequestStreaming(MaxSize) + .port(port) + .responseCompression() + + private val app = serve + + def conformanceSpec = suite("ConformanceE2ESpec")( + test("should return 400 Bad Request if Host header is missing") { + val routes = Handler.ok.toRoutes + + val res = routes.deploy.status.run(path = Path.root, headers = Headers(Header.Host("%%%%invalid%%%%"))) + assertZIO(res)(equalTo(Status.BadRequest)) + }, + test("should return 200 OK if Host header is present") { + val routes = Handler.ok.toRoutes + + 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 + + val resCRLF = + routes.deploy.status.run(path = Path.root / "test", headers = Headers("InvalidHeader" -> "Value\r\n")) + val resNull = + routes.deploy.status.run(path = Path.root / "test", headers = Headers("InvalidHeader" -> "Value\u0000")) + + for { + responseCRLF <- resCRLF + responseNull <- resNull + } yield assertTrue( + responseCRLF == Status.BadRequest, + responseNull == Status.BadRequest, + ) + }, + test("should return 400 Bad Request if there is whitespace between start-line and first header field") { + val route = Method.GET / "test" -> Handler.ok + val routes = Routes(route) + + val malformedRequest = Request + .get("/test") + .copy(headers = Headers.empty) + .withBody(Body.fromString("\r\nHost: localhost")) + + val res = routes.deploy.status.run(path = Path.root / "test", headers = malformedRequest.headers) + assertZIO(res)(equalTo(Status.BadRequest)) + }, + test("should return 400 Bad Request if there is whitespace between header field and colon") { + val route = Method.GET / "test" -> Handler.ok + val routes = Routes(route) + + val requestWithWhitespaceHeader = Request.get("/test").addHeader(Header.Custom("Invalid Header ", "value")) + + val res = routes.deploy.status.run(path = Path.root / "test", headers = requestWithWhitespaceHeader.headers) + assertZIO(res)(equalTo(Status.BadRequest)) + }, + ) + + override def spec = + suite("ConformanceE2ESpec") { + val spec = conformanceSpec + suite("app without request streaming") { app.as(List(spec)) } + }.provideShared( + DynamicServer.live, + ZLayer.succeed(configApp), + Server.customized, + Client.default, + ZLayer.succeed(NettyConfig.default), + ) @@ sequential @@ withLiveClock + +} diff --git a/zio-http/jvm/src/test/scala/zio/http/ConformanceSpec.scala b/zio-http/jvm/src/test/scala/zio/http/ConformanceSpec.scala index e12913b7cf..9b448ab397 100644 --- a/zio-http/jvm/src/test/scala/zio/http/ConformanceSpec.scala +++ b/zio-http/jvm/src/test/scala/zio/http/ConformanceSpec.scala @@ -21,7 +21,6 @@ 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) @@ -504,48 +503,6 @@ object ConformanceSpec extends ZIOSpecDefault { }, ), suite("HTTP Headers")( - suite("code_400_after_bad_host_request")( - test("should return 200 OK if Host header is present") { - val route = Method.GET / "test" -> Handler.ok - val app = Routes(route) - val requestWithHost = Request.get("/test").addHeader(Header.Host("localhost")) - for { - response <- app.runZIO(requestWithHost) - } yield assertTrue(response.status == Status.Ok) - }, - test("should return 400 Bad Request if Host header is missing") { - val route = Method.GET / "test" -> Handler.ok - val app = Routes(route) - val requestWithoutHost = Request.get("/test") - - for { - response <- app.runZIO(requestWithoutHost) - } yield assertTrue(response.status == Status.BadRequest) - }, - test("should return 400 Bad Request if there are multiple Host headers") { - val route = Method.GET / "test" -> Handler.ok - val app = Routes(route) - val requestWithTwoHosts = Request - .get("/test") - .addHeader(Header.Host("example.com")) - .addHeader(Header.Host("another.com")) - - for { - response <- app.runZIO(requestWithTwoHosts) - } yield assertTrue(response.status == Status.BadRequest) - }, - test("should return 400 Bad Request if Host header is invalid") { - val route = Method.GET / "test" -> Handler.ok - val app = Routes(route) - val requestWithInvalidHost = Request - .get("/test") - .addHeader(Header.Host("invalid_host")) - - for { - response <- app.runZIO(requestWithInvalidHost) - } yield assertTrue(response.status == Status.BadRequest) - }, - ), test("should not include Content-Length header for 2XX CONNECT responses(content_length_2XX_connect)") { val app = Routes( Method.CONNECT / "" -> Handler.fromResponse( @@ -764,22 +721,6 @@ object ConformanceSpec extends ZIOSpecDefault { }, ), suite("transfer_encoding_http11")( - test("should not send Transfer-Encoding in response if request HTTP version is below 1.1") { - val app = Routes( - Method.GET / "test" -> Handler.fromResponse( - Response.ok.addHeader(Header.TransferEncoding.Chunked), - ), - ) - - val request = Request.get("/test").copy(version = Version.`HTTP/1.0`) - - for { - response <- app.runZIO(request) - } yield assertTrue( - response.status == Status.Ok, - !response.headers.contains(Header.TransferEncoding.name), - ) - }, test("should send Transfer-Encoding in response if request HTTP version is 1.1 or higher") { val app = Routes( Method.GET / "test" -> Handler.fromResponse( @@ -850,6 +791,18 @@ object ConformanceSpec extends ZIOSpecDefault { getHeaders == headHeaders, ) }, + test("404 response for truly non-existent path") { + val app = Routes( + Method.GET / "existing-path" -> Handler.ok, + ) + val request = Request.get(URL(Path.root / "non-existent-path")) + + for { + response <- app.runZIO(request) + } yield assertTrue( + response.status == Status.NotFound, + ) + }, test("should reply with 501 for unknown HTTP methods (code_501_unknown_methods)") { val app = Routes( Method.GET / "test" -> Handler.fromResponse(Response.status(Status.Ok)), @@ -881,29 +834,6 @@ object ConformanceSpec extends ZIOSpecDefault { }, ), suite("HTTP/1.1")( - test("should return 400 Bad Request if there is whitespace between start-line and first header field") { - val route = Method.GET / "test" -> Handler.ok - val app = Routes(route) - - val malformedRequest = - Request.get("/test").copy(headers = Headers.empty).withBody(Body.fromString("\r\nHost: localhost")) - - for { - response <- app.runZIO(malformedRequest) - } yield assertTrue(response.status == Status.BadRequest) - }, - test("should return 400 Bad Request if there is whitespace between header field and colon") { - val route = Method.GET / "test" -> Handler.ok - val app = Routes(route) - - val requestWithWhitespaceHeader = Request.get("/test").addHeader(Header.Custom("Invalid Header ", "value")) - - for { - response <- app.runZIO(requestWithWhitespaceHeader) - } yield { - assertTrue(response.status == Status.BadRequest) - } - }, test("should not generate a bare CR in headers for HTTP/1.1(no_bare_cr)") { val app = Routes( Method.GET / "test" -> Handler.fromZIO {