Skip to content

Commit

Permalink
Close #235: Support Jetty 12
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
kevin-lee committed Nov 13, 2024
1 parent 59090e4 commit c865ba6
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 95 deletions.
75 changes: 9 additions & 66 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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'
Expand All @@ -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
Expand All @@ -121,7 +103,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest]
java: [temurin@11]
java: [temurin@17]
runs-on: ${{ matrix.os }}
steps:
- name: Install sbt
Expand All @@ -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'
Expand Down Expand Up @@ -218,7 +187,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest]
java: [temurin@11]
java: [temurin@17]
runs-on: ${{ matrix.os }}
steps:
- name: Install sbt
Expand All @@ -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'
Expand All @@ -266,7 +222,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest]
java: [temurin@11]
java: [temurin@17]
runs-on: ${{ matrix.os }}
steps:
- name: Install sbt
Expand All @@ -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'
Expand Down
12 changes: 7 additions & 5 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -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 +=
Expand All @@ -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"))
Expand All @@ -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,
Expand Down Expand Up @@ -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,
),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 */
Expand Down Expand Up @@ -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"))
}
}

Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ import cats.effect._
import cats.effect.kernel.Async
import cats.effect.std.Dispatcher
import cats.syntax.all._
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.http2.server.HTTP2CServerConnectionFactory
import org.eclipse.jetty.server.HttpConfiguration
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.util.ssl.SslContextFactory
import org.eclipse.jetty.util.thread.ThreadPool
import org.http4s.jetty.server.JettyBuilder._
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit c865ba6

Please sign in to comment.