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.

Sign up for GitHub

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 9 commits into from
Apr 28, 2023
Merged

Upgrade from javax.servlet-api-3.1 to jakarta.servlet-api:4.0 and ban javax.servlet:javax.servlet-api #693

Show file tree
Hide file tree
Changes from 4 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 Feb 16, 2023
3fee819
Upgrade jarkarta.servlet-api from 4.0.3 to 4.0.4
jeromepochat Feb 17, 2023
7b2d225
Add ServletAPITest
jeromepochat Feb 17, 2023
7e51070
Merge branch 'fix/servlet-api' of github.com:jeromepochat/plugin-pom …
jeromepochat Feb 17, 2023
fa67413
Typo
jeromepochat Feb 17, 2023
e77fda9
Merge branch 'jenkinsci:master' into fix/servlet-api
jeromepochat Apr 24, 2023
053853b
Merge branch 'jenkinsci:master' into fix/servlet-api
jeromepochat Apr 26, 2023
3bed3c9
Fix Spotless issues
jeromepochat Apr 26, 2023
aece721
Ban javax.servlet:javax.servlet-api
jeromepochat Apr 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,9 @@
</dependency>
<dependency>
<!-- used in JTH and jenkins core > 2.x -->
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
<version>3.1.0</version>
<groupId>jakarta.servlet</groupId>
<artifactId>jakarta.servlet-api</artifactId>
<version>4.0.4</version>
</dependency>
<dependency>
<groupId>junit</groupId>
Expand Down Expand Up @@ -239,8 +239,8 @@

<!-- dependencies provided by virtue of running in Jenkins -->
<dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
<groupId>jakarta.servlet</groupId>
<artifactId>jakarta.servlet-api</artifactId>
<scope>provided</scope>
</dependency>

Expand Down
3 changes: 3 additions & 0 deletions src/it/servlet-api/invoker.properties
View file
Open in desktop
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
37 changes: 37 additions & 0 deletions src/it/servlet-api/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?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>
<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>
<dependencies>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>structs</artifactId>
<version>1.5</version>
</dependency>
</dependencies>
</project>
2 changes: 2 additions & 0 deletions src/it/servlet-api/src/main/resources/index.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<?jelly escape-by-default='true'?>
<div/>
34 changes: 34 additions & 0 deletions src/it/servlet-api/src/test/java/test/ServletAPITest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package test;

import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;

import hudson.security.LegacySecurityRealm;
import jenkins.model.Jenkins;

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 emethod 'abstract void valueBound(javax.servlet.http.HttpSessionBindingEvent)' of interface
jeromepochat marked this conversation as resolved.
Show resolved Hide resolved
* 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());
Copy link
Member

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

Suggested change
Jenkins.get().setSecurityRealm(new LegacySecurityRealm());
Jenkins.get().setSecurityRealm(rule.createDummySecurityRealm());

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.

Copy link
Contributor Author

@jeromepochat jeromepochat Feb 17, 2023 •
edited
Loading

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).

Copy link
Member

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.

rule.createWebClient().login("bob");
}
}