ring-clojure / ring

Clojure HTTP server abstraction
MIT License
3.75k stars 519 forks source link

Migrate to Jetty 10/11 #439

Closed mcorbin closed 1 year ago

mcorbin commented 3 years ago

Hello,

The Ring Jetty adapter uses Jetty 9.4.38. Jetty 10 and 11 are available. I would liek to know if you would be interested to migrate to one of these versions. The main difference between 10 and 11 seems to be the renaming of the javax.servlet packages to jakarta.servlet.

I'm not a Jetty expert but I think I can find some time to do the migration if you are interested.

weavejester commented 3 years ago

It would mean updating the minimum JVM version from 8 to 11. Are there any benefits to upgrading before 9.4 is officially deprecated?

mcorbin commented 3 years ago

The most interesting thing is I think the websocket refactoring (https://webtide.com/jetty-10-and-11-have-arrived/).

iorena commented 3 years ago

9.4.38 also has a serious vulnerability https://nvd.nist.gov/vuln/detail/CVE-2021-28165

weavejester commented 3 years ago

9.4.38 also has a serious vulnerability https://nvd.nist.gov/vuln/detail/CVE-2021-28165

I'll update to 9.4.40.

Juholei commented 3 years ago

9.4.40 seems to have two vulnerabilities https://nvd.nist.gov/vuln/detail/CVE-2021-28169 https://nvd.nist.gov/vuln/detail/CVE-2021-34428

antonmos commented 2 years ago

FYI, Jetty 9.x Community support ended on 6/1/22, but security updates will continue probably until 2025. See more at https://github.com/eclipse/jetty.project/issues/7958

ieugen commented 1 year ago

I think Jetty will have at least 2 supported stable versions for some time. Perhaps ring could also support all of them. Here is a table with some information on why you should use a version over the other: https://www.eclipse.org/jetty/download.php .

Jetty 10 also comes with servlet 4 API . According to this Java 8 will receive security support until 2026 but no active development: https://endoflife.date/java .

Instead of migration, maybe supporting multiple versions concurrently is better. I imagine they can glue layer is not that big, hence maintenance should not be that difficult.

WDYT?

weavejester commented 1 year ago

If you want to create and maintain a Ring adapter for Jetty 10, then by all means feel free to do so.

mcorbin commented 1 year ago

There's already https://github.com/sunng87/ring-jetty9-adapter available (the project name is misleading).

ieugen commented 1 year ago

Thanks, I just found the project after James mentioned it just now on slack. Looks pretty sweet.

If ring has the default jetty adapter here, people will feel the impulse to ask for upgrade / features since they are going to use it. If this is mostly a reference implementation maybe this can be mentioned. Eventually a link to good alternatives (like the one above) can also be provided.

Hopefully this will help with managing expectations.

tychedelia commented 1 year ago

Supporting loom would be one good reason to upgrade: https://github.com/eclipse/jetty.project/issues/8007

hlship commented 1 year ago

I'm hitting some challenges right now, moving Pedestal from Jetty 9 to Jetty 11 ... some of the Ring code we rely on (such as ring.middlewhare.multipart-params) is coded against the older servlet API (javax.*) not the newer (jarkara.*) and that's going to cause some real pain.

weavejester commented 1 year ago

If I recall correctly, the multipart-params middleware is the only part of Ring core that depends on the servlet API, and that's because it uses the commons-fileupload library.

At the time it was originally written there weren't many mature options for reading multipart uploads in Java that weren't tied to a particular server implementation, and writing our own multipart parser in order to avoid the servlet dependency didn't garner enough interest to seem worth the trouble.

However, it looks like within the last month commons-fileupload 2.0 has been released, and unlike 1.x, this does not appear to have a dependency on the servlet API. I haven't looked at it closely so I may be mistaken, but we may be able to update commons-upload and cut out the servlet dependency from Ring core once and for all.

weavejester commented 1 year ago

@hlship I've created a branch fileupload2 that uses commons-fileupload 2.0.0-M1, and therefore doesn't have a dependency on the servlet API (provided or otherwise). It looks like commons-fileupload is only at its first milestone release, and not stable yet, but when it is I can release Ring 1.11 and get rid of the servlet dependency for good.

hlship commented 1 year ago

Thanks for that; I created a fork and added a deps.edn so that I can use it in my Pedestal work branch in the meantime.

weavejester commented 1 year ago

For future reference I should probably document why I changed my mind on this, or at least upgraded earlier than 2025, when Jetty's 9 security updates will finally come to an end (assuming enterprise customers don't push the date back further).

There were several factors that tipped the balance:

  1. At the time this issue was raised, Java 11 was 2.5 years old; now it's (almost) 5 years old. The pool of people potentially affected by the change is going to be smaller than it was in 2021.
  2. Apache Commons FileUpload 2.0.0 is also going to move to Java 11. If we want to use this to remove the provided servlet dependency in Ring Core, we again have to move to Java 11.
  3. We're nearing the end of 2023, so we may only have a year or so left of Jetty 9. Moving over a little sooner may not be the worst idea.