Close http4s#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.
kevin-lee · Nov 13, 2024 · c197c26 · c197c26
1 parent 59090e4
commit c197c26
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 95 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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