Skip to content

Commit

Permalink
[JENKINS-73130] Upgrade core from Jetty 10.x to 12.x (EE 8)
Browse files Browse the repository at this point in the history
Co-authored-by: Olivier Lamy <[email protected]>
  • Loading branch information
basil and olamy committed Aug 6, 2024
1 parent 4137e49 commit 7987d74
Show file tree
Hide file tree
Showing 15 changed files with 347 additions and 43 deletions.
4 changes: 1 addition & 3 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ updates:
# version of Jetty we deliver. See:
# https://github.com/jenkinsci/jenkins/pull/5211
- dependency-name: "jakarta.servlet:jakarta.servlet-api"
- dependency-name: "jakarta.servlet.jsp.jstl:jakarta.servlet.jsp.jstl-api"

# Jetty Maven Plugin and Winstone should be upgraded in lockstep in order
# to keep their corresponding Jetty versions aligned.
Expand All @@ -27,9 +28,6 @@ updates:
# and determined to be temporary. Exclusions should be removed from this
# section once the remaining action items have been completed.

# Contains incompatible API changes and needs compatibility work.
- dependency-name: "jakarta.servlet.jsp.jstl:jakarta.servlet.jsp.jstl-api"

# Needs significant testing. See:
# https://github.com/jenkinsci/jenkins/pull/5112#issuecomment-744429487
# https://github.com/jenkinsci/jenkins/pull/5116#issuecomment-744526638
Expand Down
3 changes: 3 additions & 0 deletions .idea/encodings.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion bom/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ THE SOFTWARE.
<properties>
<commons-fileupload2.version>2.0.0-M2</commons-fileupload2.version>
<slf4jVersion>2.0.13</slf4jVersion>
<stapler.version>1892.v73465f3d074d</stapler.version>
<!-- TODO JENKINS-73122 https://github.com/jenkinsci/stapler/pull/537 -->
<stapler.version>1894.v29804a_df796e</stapler.version>
<groovy.version>2.4.21</groovy.version>
</properties>

Expand Down
4 changes: 3 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ THE SOFTWARE.
<module>bom</module>
<module>websocket/spi</module>
<module>websocket/jetty10</module>
<module>websocket/jetty12-ee8</module>
<module>core</module>
<module>war</module>
<module>test</module>
Expand Down Expand Up @@ -99,7 +100,8 @@ THE SOFTWARE.
<bridge-method-injector.version>1.29</bridge-method-injector.version>
<spotless.check.skip>false</spotless.check.skip>
<!-- Make sure to keep the jetty-maven-plugin version in war/pom.xml in sync with the Jetty release in Winstone: -->
<winstone.version>6.21</winstone.version>
<!-- TODO JENKINS-73126 https://github.com/jenkinsci/winstone/pull/383 -->
<winstone.version>7.0-rc921.b_dc6422f61b_6</winstone.version>
</properties>

<!--
Expand Down
2 changes: 1 addition & 1 deletion test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ THE SOFTWARE.
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>jenkins-test-harness</artifactId>
<version>2225.2230.v6210cb_b_827f9</version>
<version>2250.v03a_1295b_0a_30</version>
<scope>test</scope>
<exclusions>
<exclusion>
Expand Down
2 changes: 1 addition & 1 deletion test/src/test/java/hudson/PluginTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public class PluginTest {
r.createWebClient().assertFails("plugin/matrix-auth/images/%2e%2e%2fWEB-INF/licenses.xml", HttpServletResponse.SC_BAD_REQUEST);
r.createWebClient().assertFails("plugin/matrix-auth/images/%2e.%2fWEB-INF/licenses.xml", HttpServletResponse.SC_BAD_REQUEST);
r.createWebClient().assertFails("plugin/matrix-auth/images/..%2f..%2f..%2f" + r.jenkins.getRootDir().getName() + "%2fsecrets%2fmaster.key", HttpServletResponse.SC_BAD_REQUEST);
r.createWebClient().assertFails("plugin/matrix-auth/" + r.jenkins.getRootDir() + "/secrets/master.key", /* ./ prepended anyway */ HttpServletResponse.SC_NOT_FOUND);
r.createWebClient().assertFails("plugin/matrix-auth/" + r.jenkins.getRootDir() + "/secrets/master.key", /* ./ prepended anyway */ Functions.isWindows() ? HttpServletResponse.SC_BAD_REQUEST : HttpServletResponse.SC_NOT_FOUND);
// SECURITY-155:
r.createWebClient().assertFails("plugin/matrix-auth/WEB-INF/licenses.xml", HttpServletResponse.SC_BAD_REQUEST);
r.createWebClient().assertFails("plugin/matrix-auth/META-INF/MANIFEST.MF", HttpServletResponse.SC_BAD_REQUEST);
Expand Down
23 changes: 17 additions & 6 deletions test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,16 @@ public void doubleDots2() throws Exception {
p.getBuildersList().add(new Shell("mkdir abc; touch abc/def.bin"));
j.buildAndAssertSuccess(p);

// can we see it?
j.createWebClient().goTo("job/" + p.getName() + "/ws/abc%5Cdef.bin", "application/octet-stream");
try (JenkinsRule.WebClient wc = j.createWebClient()) {
// normal path provided by the UI succeeds
wc.goTo("job/" + p.getName() + "/ws/abc/def.bin", "application/octet-stream");

// suspicious path is rejected with 400
wc.setThrowExceptionOnFailingStatusCode(false);
HtmlPage page = wc.goTo("job/" + p.getName() + "/ws/abc%5Cdef.bin");
assertEquals(400, page.getWebResponse().getStatusCode());
assertEquals("Error 400 Suspicious Path Character", page.getTitleText());
}
}

@Test
Expand Down Expand Up @@ -1108,10 +1116,13 @@ public void windows_cannotViewAbsolutePath() throws Exception {
String content = "random data provided as fixed value";
Files.writeString(targetTmpPath, content, StandardCharsets.UTF_8);

JenkinsRule.WebClient wc = j.createWebClient().withThrowExceptionOnFailingStatusCode(false);
Page page = wc.goTo("userContent/" + targetTmpPath.toAbsolutePath() + "/*view*", null);

MatcherAssert.assertThat(page.getWebResponse().getStatusCode(), equalTo(404));
try (JenkinsRule.WebClient wc = j.createWebClient()) {
// suspicious path is rejected with 400
wc.setThrowExceptionOnFailingStatusCode(false);
HtmlPage page = wc.goTo("userContent/" + targetTmpPath.toAbsolutePath() + "/*view*");
assertEquals(400, page.getWebResponse().getStatusCode());
assertEquals("Error 400 Suspicious Path Character", page.getTitleText());
}
}

@Test
Expand Down
23 changes: 14 additions & 9 deletions test/src/test/java/hudson/model/UpdateSiteTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
Expand All @@ -55,16 +56,18 @@
import java.util.jar.Manifest;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import jenkins.model.Jenkins;
import jenkins.security.UpdateSiteWarningsConfiguration;
import jenkins.security.UpdateSiteWarningsMonitor;
import org.apache.commons.io.FilenameUtils;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.util.Callback;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
Expand Down Expand Up @@ -115,19 +118,21 @@ public void setUpWebServer() throws Exception {
server = new Server();
ServerConnector connector = new ServerConnector(server);
server.addConnector(connector);
server.setHandler(new AbstractHandler() {
server.setHandler(new Handler.Abstract() {
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException {
public boolean handle(Request request, Response response, Callback callback) throws IOException {
String target = request.getHttpURI().getPath();
if (target.startsWith(RELATIVE_BASE)) {
target = target.substring(RELATIVE_BASE.length());
}
String responseBody = getResource(target);
if (responseBody != null) {
baseRequest.setHandled(true);
response.setContentType("text/plain; charset=utf-8");
response.setStatus(HttpServletResponse.SC_OK);
response.getOutputStream().write(responseBody.getBytes(StandardCharsets.UTF_8));
response.getHeaders().add(HttpHeader.CONTENT_TYPE, "text/plain; charset=utf-8");
response.setStatus(HttpStatus.OK_200);
response.write(true, ByteBuffer.wrap(responseBody.getBytes(StandardCharsets.UTF_8)), callback);
return true;
}
return false;
}
});
server.start();
Expand Down
29 changes: 17 additions & 12 deletions test/src/test/java/jenkins/install/SetupWizardTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
Expand All @@ -50,15 +51,16 @@
import java.security.cert.X509Certificate;
import java.util.HashSet;
import java.util.Set;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import jenkins.model.Jenkins;
import jenkins.util.JSONSignatureValidator;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.util.Callback;
import org.htmlunit.Page;
import org.junit.Before;
import org.junit.Rule;
Expand Down Expand Up @@ -336,7 +338,7 @@ protected JSONSignatureValidator getJsonSignatureValidator(String name) {
}
}

private static class RemoteUpdateSiteHandler extends AbstractHandler {
private static class RemoteUpdateSiteHandler extends Handler.Abstract {
private String serverContext;
private boolean includeSignature;

Expand All @@ -347,15 +349,18 @@ private static class RemoteUpdateSiteHandler extends AbstractHandler {
}

@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
String responseBody = getWebServerResource(target, request.getParameter("version"));
public boolean handle(Request request, Response response, Callback callback) throws IOException {
String target = request.getHttpURI().getPath();
String version = Request.extractQueryParameters(request).get("version").getValue();
String responseBody = getWebServerResource(target, version);
if (responseBody != null) {
baseRequest.setHandled(true);
response.setContentType("text/plain; charset=utf-8");
response.setStatus(HttpServletResponse.SC_OK);
response.getOutputStream().write(responseBody.getBytes(StandardCharsets.UTF_8));
response.getHeaders().add(HttpHeader.CONTENT_TYPE, "text/plain; charset=utf-8");
response.setStatus(HttpStatus.OK_200);
response.write(true, ByteBuffer.wrap(responseBody.getBytes(StandardCharsets.UTF_8)), callback);
return true;
} else {
response.sendError(404);
Response.writeError(request, response, callback, HttpStatus.NOT_FOUND_404);
return true;
}
}

Expand Down
8 changes: 8 additions & 0 deletions test/src/test/java/jenkins/security/Security3030Test.java
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,14 @@ public HttpResponse doSubmitMultipart(StaplerRequest req) throws FileUploadExcep
return processMultipartAndUnwrap(req);
} else {
actualWrapped = Assert.assertThrows(expectedWrapped, () -> processMultipartAndUnwrap(req));

// The client might still be sending us more of the request, but we have had enough of it already and
// have decided to stop processing it. Drain the read end of the socket so that the client can finish
// sending its request in order to read the response we are about to provide.
try (OutputStream os = OutputStream.nullOutputStream()) {
req.getInputStream().transferTo(os);
}

return HttpResponses.ok();
}
}
Expand Down
6 changes: 3 additions & 3 deletions test/src/test/java/jenkins/util/SystemPropertiesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import static org.hamcrest.Matchers.nullValue;

import javax.servlet.ServletContextEvent;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.ee8.webapp.WebAppContext;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Assume;
Expand Down Expand Up @@ -103,7 +103,7 @@ public void shouldReturnWebAppPropertyIfSystemPropertyNotSetAndDefaultIsSet() th
* @param value value of the property
*/
protected void setWebAppInitParameter(String property, String value) {
Assume.assumeThat(j.jenkins.servletContext, Matchers.instanceOf(ContextHandler.Context.class));
((ContextHandler.Context) j.jenkins.servletContext).getContextHandler().getInitParams().put(property, value);
Assume.assumeThat(j.jenkins.servletContext, Matchers.instanceOf(WebAppContext.Context.class));
((WebAppContext.Context) j.jenkins.servletContext).getContextHandler().getInitParams().put(property, value);
}
}
18 changes: 13 additions & 5 deletions war/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ THE SOFTWARE.
<artifactId>websocket-jetty10</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.main</groupId>
<artifactId>websocket-jetty12-ee8</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<!--
We bundle slf4j binding since we got some components (sshd for example)
Expand Down Expand Up @@ -155,6 +160,7 @@ THE SOFTWARE.
<exclude>org.jenkins-ci.main:cli</exclude>
<exclude>org.jenkins-ci.main:jenkins-core</exclude>
<exclude>org.jenkins-ci.main:websocket-jetty10</exclude>
<exclude>org.jenkins-ci.main:websocket-jetty12-ee8</exclude>
<exclude>org.jenkins-ci.main:websocket-spi</exclude>
<exclude>org.kohsuke.stapler:stapler</exclude>
<exclude>org.kohsuke.stapler:stapler-groovy</exclude>
Expand Down Expand Up @@ -627,22 +633,24 @@ THE SOFTWARE.
</configuration>
</plugin>
<plugin>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-maven-plugin</artifactId>
<version>10.0.20</version>
<groupId>org.eclipse.jetty.ee8</groupId>
<artifactId>jetty-ee8-maven-plugin</artifactId>
<version>12.0.12</version>
<configuration>
<!--
Reload webapp when you hit ENTER. (See JETTY-282 for more)
-->
<reload>manual</reload>
<scan>0</scan>
<httpConnector>
<host>${host}</host>
<port>${port}</port>
</httpConnector>
<loginServices>
<loginService implementation="org.eclipse.jetty.security.HashLoginService">
<name>default</name>
<config>${basedir}/src/realm.properties</config>
<config implementation="org.eclipse.jetty.maven.MavenResource">
<resourceAsString>${basedir}/src/realm.properties</resourceAsString>
</config>
</loginService>
</loginServices>
<systemProperties>
Expand Down
2 changes: 1 addition & 1 deletion websocket/jetty10/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ THE SOFTWARE.
<dependency>
<groupId>org.jenkins-ci</groupId>
<artifactId>winstone</artifactId>
<version>${winstone.version}</version>
<version>6.21</version>
<optional>true</optional>
</dependency>
<dependency>
Expand Down
Loading

0 comments on commit 7987d74

Please sign in to comment.