[JENKINS-73130] Upgrade core from Jetty 10.x to 12.x (EE 8) #9590
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
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
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 |
|---|---|---|
|
|
@@ -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 */ Functions.isWindows() ? HttpServletResponse.SC_BAD_REQUEST : HttpServletResponse.SC_NOT_FOUND); | ||
|
There was a problem hiding this comment. See JENKINS-73128 for an explanation of this change. Newer versions of the Servlet API are more secure by default and reject suspicious sequences on Windows with a 400 error rather than a 404. We appreciate this increased security, but our test must be adapted to expect the new error code on Windows. |
||
| // 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); | ||
|
|
||
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 |
|---|---|---|
|
|
@@ -149,8 +149,16 @@ public void doubleDots2() throws Exception { | |
| p.getBuildersList().add(new Shell("mkdir abc; touch abc/def.bin")); | ||
| j.buildAndAssertSuccess(p); | ||
|
|
||
| try (JenkinsRule.WebClient wc = j.createWebClient()) { | ||
|
There was a problem hiding this comment. See JENKINS-73129 for an explanation of this change. Newer versions of the Servlet API are more secure by default and reject suspicious sequences with a 400 error rather than a 404. We appreciate this increased security, but our test must be adapted to expect the new error code on Windows. |
||
| // 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 | ||
|
|
@@ -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); | ||
|
|
||
| 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 | ||
|
|
||
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 |
|---|---|---|
|
|
@@ -55,16 +55,19 @@ | |
| import java.util.jar.Manifest; | ||
| import java.util.zip.ZipEntry; | ||
| import java.util.zip.ZipInputStream; | ||
| 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.io.Content; | ||
| 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.util.Callback; | ||
| import org.junit.After; | ||
| import org.junit.Before; | ||
| import org.junit.Rule; | ||
|
|
@@ -115,19 +118,21 @@ public void setUpWebServer() throws Exception { | |
| server = new Server(); | ||
| ServerConnector connector = new ServerConnector(server); | ||
| server.addConnector(connector); | ||
| server.setHandler(new Handler.Abstract() { | ||
|
There was a problem hiding this comment. Adapting to new Jetty 12 APIs as per the migration guide. |
||
| @Override | ||
| 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) { | ||
| response.getHeaders().add(HttpHeader.CONTENT_TYPE, "text/plain; charset=utf-8"); | ||
| response.setStatus(HttpStatus.OK_200); | ||
| Content.Sink.write(response, true, responseBody, callback); | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
| }); | ||
| server.start(); | ||
|
|
||
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 |
|---|---|---|
|
|
@@ -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); | ||
| } | ||
|
Comment on lines
+276
to
+281
There was a problem hiding this comment. See JENKINS-73127 for an explanation of this change. |
||
|
|
||
| return HttpResponses.ok(); | ||
| } | ||
| } | ||
|
|
||
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 |
|---|---|---|
|
|
@@ -29,7 +29,7 @@ | |
| import static org.hamcrest.Matchers.nullValue; | ||
|
|
||
| import javax.servlet.ServletContextEvent; | ||
| import org.eclipse.jetty.ee8.webapp.WebAppContext; | ||
| import org.hamcrest.Matchers; | ||
| import org.junit.After; | ||
| import org.junit.Assume; | ||
|
|
@@ -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(WebAppContext.Context.class)); | ||
|
There was a problem hiding this comment. Adapting to new Jetty 12 APIs as per the migration guide. |
||
| ((WebAppContext.Context) j.jenkins.servletContext).getContextHandler().getInitParams().put(property, value); | ||
| } | ||
| } | ||
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
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,85 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
|
There was a problem hiding this comment. To ease reviewing, a diff from Jetty 10: The MIT License
-Copyright (c) 2019, CloudBees, Inc.
+Copyright (c) 2019, 2024 CloudBees, Inc.
Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
@@ -32,9 +32,9 @@ THE SOFTWARE.
<relativePath>../..</relativePath>
</parent>
- <artifactId>websocket-jetty10</artifactId>
- <name>Jetty 10 implementation for WebSocket</name>
- <description>An implementation of the WebSocket handler that works with Jetty 10.</description>
+ <artifactId>websocket-jetty12-ee8</artifactId>
+ <name>Jetty 12 (EE 8) implementation for WebSocket</name>
+ <description>An implementation of the WebSocket handler that works with Jetty 12 (EE 8).</description>
<dependencyManagement>
<dependencies>
@@ -52,7 +52,7 @@ THE SOFTWARE.
<dependency>
<groupId>org.jenkins-ci</groupId>
<artifactId>winstone</artifactId>
- <version>6.21</version>
+ <version>${winstone.version}</version>
<optional>true</optional>
</dependency>
<dependency> |
||
| <!-- | ||
| The MIT License | ||
|
|
||
| Copyright (c) 2019, 2024 CloudBees, Inc. | ||
|
|
||
| Permission is hereby granted, free of charge, to any person obtaining a copy | ||
| of this software and associated documentation files (the "Software"), to deal | ||
| in the Software without restriction, including without limitation the rights | ||
| to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
| copies of the Software, and to permit persons to whom the Software is | ||
| furnished to do so, subject to the following conditions: | ||
|
|
||
| The above copyright notice and this permission notice shall be included in | ||
| all copies or substantial portions of the Software. | ||
|
|
||
| THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
| IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
| FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
| AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
| LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
| OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
| THE SOFTWARE. | ||
| --> | ||
| <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 https://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
|
|
||
| <parent> | ||
| <groupId>org.jenkins-ci.main</groupId> | ||
| <artifactId>jenkins-parent</artifactId> | ||
| <version>${revision}${changelist}</version> | ||
| <relativePath>../..</relativePath> | ||
| </parent> | ||
|
|
||
| <artifactId>websocket-jetty12-ee8</artifactId> | ||
| <name>Jetty 12 (EE 8) implementation for WebSocket</name> | ||
| <description>An implementation of the WebSocket handler that works with Jetty 12 (EE 8).</description> | ||
|
|
||
| <dependencyManagement> | ||
| <dependencies> | ||
| <dependency> | ||
| <groupId>org.jenkins-ci.main</groupId> | ||
| <artifactId>jenkins-bom</artifactId> | ||
| <version>${project.version}</version> | ||
| <type>pom</type> | ||
| <scope>import</scope> | ||
| </dependency> | ||
| </dependencies> | ||
| </dependencyManagement> | ||
|
|
||
| <dependencies> | ||
| <dependency> | ||
| <groupId>org.jenkins-ci</groupId> | ||
| <artifactId>winstone</artifactId> | ||
| <version>${winstone.version}</version> | ||
| <optional>true</optional> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.jenkins-ci.main</groupId> | ||
| <artifactId>websocket-spi</artifactId> | ||
| <version>${project.version}</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.kohsuke</groupId> | ||
| <artifactId>access-modifier-annotation</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.kohsuke.metainf-services</groupId> | ||
| <artifactId>metainf-services</artifactId> | ||
| <optional>true</optional> | ||
| </dependency> | ||
| </dependencies> | ||
|
|
||
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-javadoc-plugin</artifactId> | ||
| <configuration> | ||
| <skip>true</skip> | ||
| </configuration> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </project> | ||
Oops, something went wrong.
Oops, something went wrong.
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.
This exclusion remains valid, but not for the reason currently given. The exclusion remains valid for the reason given in the previous exclusion for
jakarta.servlet:jakarta.servlet-api. Indeed, that reason applies to bothjakarta.servlet:jakarta.servlet-apiandjakarta.servlet.jsp.jstl:jakarta.servlet.jsp.jstl-api, so move these exclusions to the same section.