From c197c2674ed44d487330d2e8f60ebd0811c70154 Mon Sep 17 00:00:00 2001 From: Kevin Lee Date: Wed, 13 Nov 2024 15:14:47 +1100 Subject: [PATCH] Close #235: Support Jetty 12 - Jetty versions from 7.0.0 up to 12.0.11 are affected by CVE-2024-6763 (Eclipse Jetty URI parsing of invalid authority). - The current version of http4s-jetty uses Jetty 10. - Community support for Jetty 10 and Jetty 11 ended in January 2024. - To solve the issue, http4s-jetty should use Jetty 12, the current stable version. - Jetty 12 requires Java 17, so dropping support for Java 11 is necessary. - Jetty has multiple versions supporting different versions of Jakarta EE (Java EE). However, for the first version supporting Jetty 12, it is better to support only Jakarta EE 8 to minimize changes, as the API namespace moved from javax to jakarta starting with Jakarta EE 9. --- .github/workflows/ci.yml | 75 +++---------------- build.sbt | 12 +-- .../org/http4s/jetty/client/JettyClient.scala | 2 +- .../jetty/client/ResponseListener.scala | 39 ++++++---- .../jetty/client/StreamRequestContent.scala | 8 +- .../http4s/jetty/server/JettyBuilder.scala | 6 +- .../jetty/server/JettyServerSuite.scala | 4 +- 7 files changed, 51 insertions(+), 95 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bc4a73c..2a170b8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,12 +30,7 @@ jobs: matrix: os: [ubuntu-latest] scala: [2.13, 2.12, 3] - java: [temurin@11, temurin@17] - exclude: - - scala: 2.12 - java: temurin@17 - - scala: 3 - java: temurin@17 + java: [temurin@17] runs-on: ${{ matrix.os }} timeout-minutes: 60 steps: @@ -47,19 +42,6 @@ jobs: with: fetch-depth: 0 - - name: Setup Java (temurin@11) - id: setup-java-temurin-11 - if: matrix.java == 'temurin@11' - uses: actions/setup-java@v4 - with: - distribution: temurin - java-version: 11 - cache: sbt - - - name: sbt update - if: matrix.java == 'temurin@11' && steps.setup-java-temurin-11.outputs.cache-hit == 'false' - run: sbt +update - - name: Setup Java (temurin@17) id: setup-java-temurin-17 if: matrix.java == 'temurin@17' @@ -77,26 +59,26 @@ jobs: run: sbt githubWorkflowCheck - name: Check headers and formatting - if: matrix.java == 'temurin@11' && matrix.os == 'ubuntu-latest' + if: matrix.java == 'temurin@17' && matrix.os == 'ubuntu-latest' run: sbt '++ ${{ matrix.scala }}' headerCheckAll scalafmtCheckAll 'project /' scalafmtSbtCheck - name: Test run: sbt '++ ${{ matrix.scala }}' test - name: Check binary compatibility - if: matrix.java == 'temurin@11' && matrix.os == 'ubuntu-latest' + if: matrix.java == 'temurin@17' && matrix.os == 'ubuntu-latest' run: sbt '++ ${{ matrix.scala }}' mimaReportBinaryIssues - name: Generate API documentation - if: matrix.java == 'temurin@11' && matrix.os == 'ubuntu-latest' + if: matrix.java == 'temurin@17' && matrix.os == 'ubuntu-latest' run: sbt '++ ${{ matrix.scala }}' doc - name: Check scalafix lints - if: matrix.java == 'temurin@11' && !startsWith(matrix.scala, '3') + if: matrix.java == 'temurin@17' && !startsWith(matrix.scala, '3') run: sbt '++ ${{ matrix.scala }}' 'scalafixAll --check' - name: Check unused compile dependencies - if: matrix.java == 'temurin@11' + if: matrix.java == 'temurin@17' run: sbt '++ ${{ matrix.scala }}' unusedCompileDependenciesTest - name: Make target directories @@ -121,7 +103,7 @@ jobs: strategy: matrix: os: [ubuntu-latest] - java: [temurin@11] + java: [temurin@17] runs-on: ${{ matrix.os }} steps: - name: Install sbt @@ -132,19 +114,6 @@ jobs: with: fetch-depth: 0 - - name: Setup Java (temurin@11) - id: setup-java-temurin-11 - if: matrix.java == 'temurin@11' - uses: actions/setup-java@v4 - with: - distribution: temurin - java-version: 11 - cache: sbt - - - name: sbt update - if: matrix.java == 'temurin@11' && steps.setup-java-temurin-11.outputs.cache-hit == 'false' - run: sbt +update - - name: Setup Java (temurin@17) id: setup-java-temurin-17 if: matrix.java == 'temurin@17' @@ -218,7 +187,7 @@ jobs: strategy: matrix: os: [ubuntu-latest] - java: [temurin@11] + java: [temurin@17] runs-on: ${{ matrix.os }} steps: - name: Install sbt @@ -229,19 +198,6 @@ jobs: with: fetch-depth: 0 - - name: Setup Java (temurin@11) - id: setup-java-temurin-11 - if: matrix.java == 'temurin@11' - uses: actions/setup-java@v4 - with: - distribution: temurin - java-version: 11 - cache: sbt - - - name: sbt update - if: matrix.java == 'temurin@11' && steps.setup-java-temurin-11.outputs.cache-hit == 'false' - run: sbt +update - - name: Setup Java (temurin@17) id: setup-java-temurin-17 if: matrix.java == 'temurin@17' @@ -266,7 +222,7 @@ jobs: strategy: matrix: os: [ubuntu-latest] - java: [temurin@11] + java: [temurin@17] runs-on: ${{ matrix.os }} steps: - name: Install sbt @@ -277,19 +233,6 @@ jobs: with: fetch-depth: 0 - - name: Setup Java (temurin@11) - id: setup-java-temurin-11 - if: matrix.java == 'temurin@11' - uses: actions/setup-java@v4 - with: - distribution: temurin - java-version: 11 - cache: sbt - - - name: sbt update - if: matrix.java == 'temurin@11' && steps.setup-java-temurin-11.outputs.cache-hit == 'false' - run: sbt +update - - name: Setup Java (temurin@17) id: setup-java-temurin-17 if: matrix.java == 'temurin@17' diff --git a/build.sbt b/build.sbt index 1c1b0df..bab2df1 100644 --- a/build.sbt +++ b/build.sbt @@ -17,8 +17,8 @@ ThisBuild / crossScalaVersions := Seq(Scala213, "2.12.20", "3.3.4") ThisBuild / scalaVersion := Scala213 // the default Scala ThisBuild / tlJdkRelease := Some(11) ThisBuild / githubWorkflowJavaVersions ~= { - // Jetty 10 bumps the requirement to Java 11 - _.filter { case JavaSpec(_, major) => major.toInt >= 11 } + // Jetty 12 bumps the requirement to Java 17 + _.filter { case JavaSpec(_, major) => major.toInt >= 17 } } ThisBuild / resolvers += @@ -29,11 +29,12 @@ lazy val root = project .enablePlugins(NoPublishPlugin) .aggregate(jettyServer, jettyClient) -val jettyVersion = "10.0.24" +val jettyVersion = "12.0.15" val http4sVersion = "0.23.29" val http4sServletVersion = "0.24.0-RC1" val munitCatsEffectVersion = "2.0.0" val slf4jVersion = "1.7.25" +val scalaJava8Compat = "1.0.2" lazy val jettyServer = project .in(file("jetty-server")) @@ -42,9 +43,9 @@ lazy val jettyServer = project description := "Jetty implementation for http4s servers", libraryDependencies ++= Seq( "org.eclipse.jetty" % "jetty-client" % jettyVersion % Test, - "org.eclipse.jetty" % "jetty-servlet" % jettyVersion, "org.eclipse.jetty" % "jetty-util" % jettyVersion, - "org.eclipse.jetty.http2" % "http2-server" % jettyVersion, + "org.eclipse.jetty.http2" % "jetty-http2-server" % jettyVersion, + "org.eclipse.jetty.ee8" % "jetty-ee8-servlet" % jettyVersion, "org.http4s" %% "http4s-dsl" % http4sVersion % Test, "org.http4s" %% "http4s-servlet" % http4sServletVersion, "org.typelevel" %% "munit-cats-effect" % munitCatsEffectVersion % Test, @@ -77,6 +78,7 @@ lazy val jettyClient = project "org.eclipse.jetty" % "jetty-client" % jettyVersion, "org.eclipse.jetty" % "jetty-http" % jettyVersion, "org.eclipse.jetty" % "jetty-util" % jettyVersion, + "org.scala-lang.modules" %% "scala-java8-compat" % scalaJava8Compat, "org.http4s" %% "http4s-client-testkit" % http4sVersion % Test, ), ) diff --git a/jetty-client/src/main/scala/org/http4s/jetty/client/JettyClient.scala b/jetty-client/src/main/scala/org/http4s/jetty/client/JettyClient.scala index 02e734d..9e4e287 100644 --- a/jetty-client/src/main/scala/org/http4s/jetty/client/JettyClient.scala +++ b/jetty-client/src/main/scala/org/http4s/jetty/client/JettyClient.scala @@ -23,7 +23,7 @@ import cats.effect.std.Dispatcher import cats.syntax.all._ import fs2._ import org.eclipse.jetty.client.HttpClient -import org.eclipse.jetty.client.api.{Request => JettyRequest} +import org.eclipse.jetty.client.{Request => JettyRequest} import org.eclipse.jetty.http.{HttpVersion => JHttpVersion} import org.http4s.client.Client import org.log4s.Logger diff --git a/jetty-client/src/main/scala/org/http4s/jetty/client/ResponseListener.scala b/jetty-client/src/main/scala/org/http4s/jetty/client/ResponseListener.scala index 9c4a5c9..5283a60 100644 --- a/jetty-client/src/main/scala/org/http4s/jetty/client/ResponseListener.scala +++ b/jetty-client/src/main/scala/org/http4s/jetty/client/ResponseListener.scala @@ -24,11 +24,10 @@ import cats.effect.std.Queue import cats.syntax.all._ import fs2.Stream._ import fs2._ -import org.eclipse.jetty.client.api.Result -import org.eclipse.jetty.client.api.{Response => JettyResponse} +import org.eclipse.jetty.client.Result +import org.eclipse.jetty.client.{Response => JettyResponse} import org.eclipse.jetty.http.HttpFields import org.eclipse.jetty.http.{HttpVersion => JHttpVersion} -import org.eclipse.jetty.util.{Callback => JettyCallback} import org.http4s.internal.CollectionCompat.CollectionConverters._ import org.http4s.jetty.client.ResponseListener.Item import org.http4s.jetty.client.internal.loggingAsyncCallback @@ -41,7 +40,7 @@ private[jetty] final case class ResponseListener[F[_]]( cb: Callback[Resource[F, Response[F]]], dispatcher: Dispatcher[F], )(implicit F: Async[F]) - extends JettyResponse.Listener.Adapter { + extends JettyResponse.Listener { import ResponseListener.logger /* Needed to properly propagate client errors */ @@ -89,14 +88,14 @@ private[jetty] final case class ResponseListener[F[_]]( override def onContent( response: JettyResponse, content: ByteBuffer, - callback: JettyCallback, ): Unit = { val copy = ByteBuffer.allocate(content.remaining()) copy.put(content).flip() - enqueue(Item.Buf(copy)) { - case Right(_) => F.delay(callback.succeeded()) + enqueueSync(Item.Buf(copy)) { + case Right(_) => + F.unit case Left(e) => - F.delay(logger.error(e)("Error in asynchronous callback")) >> F.delay(callback.failed(e)) + F.delay(logger.error(e)("Error in asynchronous callback")) } } @@ -115,17 +114,31 @@ private[jetty] final case class ResponseListener[F[_]]( // (the request might complete after the response has been entirely received) override def onComplete(result: Result): Unit = () - private def abort(t: Throwable, response: JettyResponse): Unit = - if (!response.abort(t)) // this also aborts the request - logger.error(t)("Failed to abort the response") - else - closeStream() + private def abort(t: Throwable, response: JettyResponse): Unit = { + import scala.compat.java8.FutureConverters._ + + dispatcher.unsafeRunAndForget( + Async[F] + .fromFuture(F.delay(response.abort(t).toScala)) + .map { aborted => + if (!aborted) + logger.error(t)("Failed to abort the response") + else + closeStream() + } + .attempt + .flatMap(loggingAsyncCallback[F, Unit](logger)) + ) + } private def closeStream(): Unit = enqueue(Item.Done)(loggingAsyncCallback[F, Unit](logger)) private def enqueue(item: Item)(cb: Either[Throwable, Unit] => F[Unit]): Unit = dispatcher.unsafeRunAndForget(queue.offer(item.some).attempt.flatMap(cb)) + + private def enqueueSync(item: Item)(cb: Either[Throwable, Unit] => F[Unit]): Unit = + dispatcher.unsafeRunSync(queue.offer(item.some).attempt.flatMap(cb)) } private[jetty] object ResponseListener { diff --git a/jetty-client/src/main/scala/org/http4s/jetty/client/StreamRequestContent.scala b/jetty-client/src/main/scala/org/http4s/jetty/client/StreamRequestContent.scala index d3fb451..33866f5 100644 --- a/jetty-client/src/main/scala/org/http4s/jetty/client/StreamRequestContent.scala +++ b/jetty-client/src/main/scala/org/http4s/jetty/client/StreamRequestContent.scala @@ -22,7 +22,7 @@ import cats.effect._ import cats.effect.std._ import cats.syntax.all._ import fs2._ -import org.eclipse.jetty.client.util.AsyncRequestContent +import org.eclipse.jetty.client.AsyncRequestContent import org.eclipse.jetty.util.{Callback => JettyCallback} import org.http4s.jetty.client.internal.loggingAsyncCallback import org.log4s.getLogger @@ -45,13 +45,11 @@ private[jetty] class StreamRequestContent[F[_]] private ( private val pipe: Pipe[F, Chunk[Byte], Unit] = _.evalMap { c => write(c) - .ensure(new Exception("something terrible has happened"))(res => res) - .map(_ => ()) } - private def write(chunk: Chunk[Byte]): F[Boolean] = + private def write(chunk: Chunk[Byte]): F[Unit] = s.acquire - .map(_ => super.offer(chunk.toByteBuffer, callback)) + .map(_ => super.write(chunk.toByteBuffer, callback)) private val callback: JettyCallback = new JettyCallback { override def succeeded(): Unit = diff --git a/jetty-server/src/main/scala/org/http4s/jetty/server/JettyBuilder.scala b/jetty-server/src/main/scala/org/http4s/jetty/server/JettyBuilder.scala index d9d9254..8319905 100644 --- a/jetty-server/src/main/scala/org/http4s/jetty/server/JettyBuilder.scala +++ b/jetty-server/src/main/scala/org/http4s/jetty/server/JettyBuilder.scala @@ -28,9 +28,9 @@ import org.eclipse.jetty.server.HttpConnectionFactory import org.eclipse.jetty.server.ServerConnector import org.eclipse.jetty.server.handler.StatisticsHandler import org.eclipse.jetty.server.{Server => JServer} -import org.eclipse.jetty.servlet.FilterHolder -import org.eclipse.jetty.servlet.ServletContextHandler -import org.eclipse.jetty.servlet.ServletHolder +import org.eclipse.jetty.ee8.servlet.FilterHolder +import org.eclipse.jetty.ee8.servlet.ServletContextHandler +import org.eclipse.jetty.ee8.servlet.ServletHolder import org.eclipse.jetty.util.ssl.SslContextFactory import org.eclipse.jetty.util.thread.ThreadPool import org.http4s.jetty.server.JettyBuilder._ diff --git a/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala b/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala index 79f2f29..44ca326 100644 --- a/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala +++ b/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala @@ -24,8 +24,8 @@ import cats.effect.Temporal import munit.CatsEffectSuite import munit.catseffect.IOFixture import org.eclipse.jetty.client.HttpClient -import org.eclipse.jetty.client.api.Request -import org.eclipse.jetty.client.util.StringRequestContent +import org.eclipse.jetty.client.Request +import org.eclipse.jetty.client.StringRequestContent import org.http4s.dsl.io._ import org.http4s.server.Server