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

[MSITE-828] Upgrade jetty to recent version. Update to java 1.8 (required for jetty) #3

Merged
merged 6 commits into from Nov 15, 2018

Conversation

oflebbe
Copy link
Contributor

@oflebbe oflebbe commented Oct 29, 2018

Hi, I am hunting down insecure maven dependencies.

This pull requests tries to start discussion about upgrading jetty to a more recent version.

jetty 6.1.25 does include insecure plugin repositories via its jetty-parent . Found out while I compiled a 3rd party project.

Patch is porting the jetty code to current jetty fixing that issue.

While doing this I found that there is a inner class of a test used in normal business logic "HttpRequest". I refactored it to business logic.

Since jetty is Java 1.8, I changed maven enforcer to check for java 1.8 project wise. Since Java 1.6 is falling out of extended support in a month, java 1.7 now unsupported and java 1.8 now being the old LTS I think there is little reason to stick with java 1.6.

@olamy
Copy link
Member

olamy commented Oct 29, 2018

9.4.12.v20180830 is the last jetty version :)
by the way I will be happy to see 1.8 as a minimum....
@hboutemy @rfscholte wdyt?

@rfscholte
Copy link
Contributor

This seems like a valid reason to require Java 8 for the next release.

@olamy
Copy link
Member

olamy commented Oct 30, 2018

@olamy olamy changed the title Upgrade jetty to recent version. Update to java 1.8 (required for jetty) [MSITE-828] Upgrade jetty to recent version. Update to java 1.8 (required for jetty) Oct 30, 2018
pom.xml Outdated Show resolved Hide resolved
@oflebbe
Copy link
Contributor Author

oflebbe commented Oct 31, 2018

However I see a problem right now:
mvn site:run fails with maven 3.5.4 but runs with maven 3.3.9 ?! Have to look into it

@oflebbe
Copy link
Contributor Author

oflebbe commented Oct 31, 2018

Fixed

mvn install org.apache.maven.plugins:maven-site-plugin:3.8-SNAPSHOT:run

Copy link
Member

@olamy olamy left a comment

Choose a reason for hiding this comment

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

LGTM

@olamy
Copy link
Member

olamy commented Nov 1, 2018

@rfscholte @hboutemy any objections? Dohh might be the first plugin being java8 required!! :)

@michael-o
Copy link
Member

Seriously, because of the tests we need to force Java 8 with zero benefit? I Have the latest 1.7 Jetty in Wagon which does the job for testing.

@olamy
Copy link
Member

olamy commented Nov 8, 2018

@michael-o I would say seriously in November 2018 with 1.8 already EOL and even 9 or 10 what is the point having 1.7. https://www.oracle.com/technetwork/java/javase/eol-135779.html
After this we can then change the code to use 1.8 features.
But come on we should try being an attractive project and get new committers!! And saying "we must be 1.7" is definitely not the way to do it....

Copy link

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM
Nice work!

@oflebbe
Copy link
Contributor Author

oflebbe commented Nov 9, 2018 via email

@michael-o
Copy link
Member

@olamy @oflebbe I definitvely see your point, but Jetty 9.2 does its job for testing. As for bumping a Java version: I see this as valid as soon as someone provides good code using those features. When I see how slow we are changing stuff, I don't see this happening beyond 2019. Just for the sake of upgrading, I wouldn't do this.

@oflebbe
Copy link
Contributor Author

oflebbe commented Nov 11, 2018 via email

@michael-o
Copy link
Member

I see the point, though 9.2 is updated regularily: 9.2.26.v20180806. I don't mind updating per se, but I don't understand how it will improve security if this is testing only and not a compile/runtime dependency. In other words, it is never pulled by a third party.

Anyway, I don't see it fixed with https://github.com/eclipse/jetty.project/commits/jetty-9.2.x.

It seems fine to me to make this bump for 3.8. BUT I'd like to see "Upgrade to Java 8" with a seperate ticket and a subsequent one with Jetty. It makes the stuff transparent for our users.

@olamy
Copy link
Member

olamy commented Nov 13, 2018

a bit of paperwork for sure :) https://issues.apache.org/jira/browse/MSITE-829
jetty-9.2.x is EOL so nothing will be fixed

@slachiewicz
Copy link
Member

Just for reference https://issues.apache.org/jira/browse/MSITE-830 more libs to update

@olamy olamy merged commit a0fb71d into apache:master Nov 15, 2018
@Tibor17
Copy link
Contributor

Tibor17 commented Jul 5, 2019

@olamy
You have to differentiate between Java Compiler and Java Runtime version.
Since we have some rules (Java Compiler 1.7) and you are talking about Java EOL in the comment #3 (comment) we can perfectly recommend using JDK 1.8+ to our users due to JDK security reasons. The Compiler is totally different story. I obey the rules in Maven ASF and these Java 1.7. Later we will embed the plugins into Maven 3.7.0, and then I hope we will better organize the transition of plugins from Java 1.7 to 1.8. I hope all of them will be 1.8 related but it's not this time. We can of course open a branch where we will rework the code to Lambdas and nobody would say that we have old and ugly code.
My recommendation is to use Jetty 9.2.9 yet. Let's open and work on branches java8 in all plugins as a parallel activity. Meanwhile we should work on Maven 3.6.2/3.7.0 and support the plugins at J7 yet.

@michael-o
Copy link
Member

@olamy Rather than lambdas, I'd rather see NIO2 use first in all Maven.

@Tibor17
Copy link
Contributor

Tibor17 commented Jul 5, 2019

@olamy here is the PR and Jira issue #9

@olamy
Copy link
Member

olamy commented Jul 6, 2019 via email

@olamy
Copy link
Member

olamy commented Jul 6, 2019 via email

@oflebbe
Copy link
Contributor Author

oflebbe commented Jul 6, 2019 via email

olamy pushed a commit that referenced this pull request May 14, 2020
…va 1.8 (required for jetty) (#3)

* Upgrade jetty to recent version. Update to java 1.8 (required for jetty)

* Make checkstyle happy

* Introduce jettyVersion property

* Update jettyVersion to 9.4.12.v20180830

* Work around compilation problem for site:run

* Fix obsolete servlet api on classpath
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants