-
-
Notifications
You must be signed in to change notification settings - Fork 77
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
Upgrade from javax.servlet-api-3.1
to jakarta.servlet-api:4.0
and ban javax.servlet:javax.servlet-api
#693
Merged
Merged
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
c5365a8
Upgrade from javax.servlet-api-3.1 to jakarta.servlet-api:4.0
jeromepochat 3fee819
Upgrade jarkarta.servlet-api from 4.0.3 to 4.0.4
jeromepochat 7b2d225
Add ServletAPITest
jeromepochat 7e51070
Merge branch 'fix/servlet-api' of github.com:jeromepochat/plugin-pom …
jeromepochat fa67413
Typo
jeromepochat e77fda9
Merge branch 'jenkinsci:master' into fix/servlet-api
jeromepochat 053853b
Merge branch 'jenkinsci:master' into fix/servlet-api
jeromepochat 3bed3c9
Fix Spotless issues
jeromepochat aece721
Ban javax.servlet:javax.servlet-api
jeromepochat File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# install, not verify, because we want to check the artifact as we would be about to deploy it | ||
# release.skipTests normally set in jenkins-release profile since release:perform would do the tests | ||
invoker.goals=-Dstyle.color=always -ntp -Pjenkins-release -Drelease.skipTests=false clean install |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd"> | ||
<modelVersion>4.0.0</modelVersion> | ||
<parent> | ||
<groupId>org.jenkins-ci.plugins</groupId> | ||
<artifactId>plugin</artifactId> | ||
<version>@project.version@</version> | ||
<relativePath /> | ||
</parent> | ||
<groupId>org.jenkins-ci.plugins.its</groupId> | ||
<artifactId>servlet-api</artifactId> | ||
<version>1.0-SNAPSHOT</version> | ||
<packaging>hpi</packaging> | ||
<properties> | ||
<jenkins.version>2.361.4</jenkins.version> | ||
</properties> | ||
<dependencies> | ||
<dependency> | ||
<groupId>org.jenkins-ci.plugins</groupId> | ||
<artifactId>structs</artifactId> | ||
<version>1.5</version> | ||
</dependency> | ||
</dependencies> | ||
<repositories> | ||
<repository> | ||
<id>repo.jenkins-ci.org</id> | ||
<url>https://repo.jenkins-ci.org/public/</url> | ||
</repository> | ||
</repositories> | ||
<pluginRepositories> | ||
<pluginRepository> | ||
<id>repo.jenkins-ci.org</id> | ||
<url>https://repo.jenkins-ci.org/public/</url> | ||
</pluginRepository> | ||
</pluginRepositories> | ||
</project> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
<?jelly escape-by-default='true'?> | ||
<div/> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package test; | ||
|
||
import hudson.security.LegacySecurityRealm; | ||
import jenkins.model.Jenkins; | ||
import org.junit.Rule; | ||
import org.junit.Test; | ||
import org.jvnet.hudson.test.JenkinsRule; | ||
|
||
public class ServletAPITest { | ||
|
||
@Rule | ||
public final JenkinsRule rule = new JenkinsRule(); | ||
|
||
/** | ||
* When having both Servlet APIs 3.1 and 4.0 in classpath, the following error | ||
* is logged on server side: | ||
* | ||
* <pre> | ||
* WARNING o.e.jetty.server.HttpChannel#handleException: /jenkins/j_security_check | ||
* java.lang.AbstractMethodError: Receiver class org.eclipse.jetty.security.authentication.SessionAuthentication does not define or inherit an | ||
* implementation of the resolved method 'abstract void valueBound(javax.servlet.http.HttpSessionBindingEvent)' of interface | ||
* javax.servlet.http.HttpSessionBindingListener. at org.eclipse.jetty.server.session.Session.bindValue(Session.java:357) | ||
* </pre> | ||
* | ||
* And then on client side getting "500 Server Error for | ||
* http://localhost:.../jenkins/j_security_check" | ||
*/ | ||
@Test | ||
public void involveHttpSessionBindingListener() throws Exception { | ||
Jenkins.get().setSecurityRealm(new LegacySecurityRealm()); | ||
rule.createWebClient().login("bob"); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the error is not reproducible after
I point this out because
LegacySecurityRealm
is almost never used (it was superseded in 2007 and should perhaps be dropped altogether along with support for non-Winstone containers), and if a routine usage of HtmlUnit were broken after some core or tooling update we would have noticed that in lots of popular plugins by now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this fix @jglick !
BTW it this PR is still revelant as similar issue could occurs as having multiple versions of APIs in same classloader (at least from plugin tests).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially yes. I suspect the conflict just rarely matters, but using
LegacySecurityRealm
in a test activates a code path in the servlet container which is otherwise rarely encountered.