Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Close #235: Add support for Jetty 12 to address CVE-2024-6763 #236

Open
wants to merge 1 commit into
base: series/0.24
Choose a base branch
from

Conversation

kevin-lee
Copy link

@kevin-lee kevin-lee commented Nov 13, 2024

Close #235: Add support for Jetty 12 to address CVE-2024-6763

  • 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 kevin-lee force-pushed the issue/235/support-jetty-12 branch 2 times, most recently from c197c26 to c865ba6 Compare November 13, 2024 04:32
Comment on lines +140 to +141
private def enqueueSync(item: Item)(cb: Either[Throwable, Unit] => F[Unit]): Unit =
dispatcher.unsafeRunSync(queue.offer(item.some).attempt.flatMap(cb))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add it to use it for onContent. With fire-and-forget, some tests fail. My guess is:

  • There's no longer a callback to report success or failure in onContent(JettyResponse, ByteBuffer): Unit in ContentListener (Listener), which ResponseListener inherits from.
  • onContent without callback is used by onContent(Response response, Content.Chunk chunk, Runnable demander) in ContentListener.
  • If the body of onContent without callback is executed with fire-and-forget, there might be a chance that demander.run() in ContentListener.onContent(Response, Content.Chunk, Runnable) is executed in an incorrect state. Please have a look at the method body of ContentListener.onContent(Response, Content.Chunk, Runnable):
    default void onContent(Response response, Content.Chunk chunk, Runnable demander) throws Exception
    {
      onContent(response, chunk.getByteBuffer());
      demander.run();
    }
  • So I had to make onContent(response, chunk.getByteBuffer()) sync with enqueueSync that I added.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the failed tests when enqueue is used instead of enqueueSync, they are as shown below. Sometimes three of them fail, and other times only two fail.

Click to expand
org.http4s.jetty.client.JettyClientSuite:
==> X org.http4s.jetty.client.JettyClientSuite.JettyClient Repeat a simple request  0.332s munit.ComparisonFailException: value is not true
=> Obtained
false
=> Diff (- obtained, + expected)
-false
+true
    at munit.Assertions.failComparison(Assertions.scala:278)
    at apply @ munit.CatsEffectAssertions.$anonfun$assertIO$1(CatsEffectAssertions.scala:52)
    at unsafeToFuture @ munit.CatsEffectSuite$$anonfun$1.applyOrElse(CatsEffectSuite.scala:82)
    at parTraverse$extension @ org.http4s.client.testkit.ClientRouteTestBattery.$anonfun$new$1(ClientRouteTestBattery.scala:81)
    at parTraverse$extension @ org.http4s.client.testkit.ClientRouteTestBattery.$anonfun$new$1(ClientRouteTestBattery.scala:81)
    at parTraverse$extension @ org.http4s.client.testkit.ClientRouteTestBattery.$anonfun$new$1(ClientRouteTestBattery.scala:81)
  + JettyClient POST an empty body 0.015s
  + JettyClient POST a normal body 0.009s
  + JettyClient POST a chunked body 0.009s
  + JettyClient POST a multipart body 0.048s
  + JettyClient Execute GET /chunked 0.012s
==> X org.http4s.jetty.client.JettyClientSuite.JettyClient Execute GET /large  0.036s munit.ComparisonFailException: value is not true
=> Obtained
false
=> Diff (- obtained, + expected)
-false
+true
    at munit.Assertions.failComparison(Assertions.scala:278)
    at apply @ munit.CatsEffectAssertions.$anonfun$assertIO$1(CatsEffectAssertions.scala:52)
    at apply @ org.http4s.client.testkit.ClientRouteTestBattery.$anonfun$checkResponse$5(ClientRouteTestBattery.scala:190)
    at map @ org.http4s.client.testkit.ClientRouteTestBattery.$anonfun$checkResponse$5(ClientRouteTestBattery.scala:190)
    at flatMap @ munit.CatsEffectAssertions.assertIO(CatsEffectAssertions.scala:52)
    at flatMap @ org.http4s.client.testkit.ClientRouteTestBattery.$anonfun$checkResponse$5(ClientRouteTestBattery.scala:190)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at modify @ fs2.internal.Scope.close(Scope.scala:262)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at rethrow$extension @ fs2.Compiler$Target.$anonfun$compile$1(Compiler.scala:157)
==> X org.http4s.jetty.client.JettyClientSuite.JettyClient Execute GET /not-found  0.027s munit.ComparisonFailException: value is not true
=> Obtained
false
=> Diff (- obtained, + expected)
-false
+true
    at munit.Assertions.failComparison(Assertions.scala:278)
    at apply @ munit.CatsEffectAssertions.$anonfun$assertIO$1(CatsEffectAssertions.scala:52)
    at apply @ org.http4s.client.testkit.ClientRouteTestBattery.$anonfun$checkResponse$5(ClientRouteTestBattery.scala:190)
    at map @ org.http4s.client.testkit.ClientRouteTestBattery.$anonfun$checkResponse$5(ClientRouteTestBattery.scala:190)
    at flatMap @ munit.CatsEffectAssertions.assertIO(CatsEffectAssertions.scala:52)
    at flatMap @ org.http4s.client.testkit.ClientRouteTestBattery.$anonfun$checkResponse$5(ClientRouteTestBattery.scala:190)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at modify @ fs2.internal.Scope.close(Scope.scala:262)
    at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
    at rethrow$extension @ fs2.Compiler$Target.$anonfun$compile$1(Compiler.scala:157)

Screenshot 2024-11-13 at 3 55 06 pm

@kevin-lee kevin-lee marked this pull request as ready for review November 13, 2024 05:00
- 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 kevin-lee force-pushed the issue/235/support-jetty-12 branch from c865ba6 to cff1987 Compare November 13, 2024 12:42
@kevin-lee kevin-lee changed the title Close #235: Support Jetty 12 Close #235: Add support for Jetty 12 to address CVE-2024-6763 Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Jetty 12 to address CVE-2024-6763
1 participant