-
Notifications
You must be signed in to change notification settings - Fork 2k
Issue #4652 - test to demonstrate the WebAppContext ClassLoader isolation #13838
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
base: jetty-12.1.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,183 @@ | ||
| // | ||
| // ======================================================================== | ||
| // Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. | ||
| // | ||
| // This program and the accompanying materials are made available under the | ||
| // terms of the Eclipse Public License v. 2.0 which is available at | ||
| // https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 | ||
| // which is available at https://www.apache.org/licenses/LICENSE-2.0. | ||
| // | ||
| // SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 | ||
| // ======================================================================== | ||
| // | ||
|
|
||
| package org.eclipse.jetty.ee11.test; | ||
|
|
||
| import java.io.IOException; | ||
| import java.net.URI; | ||
| import java.net.URISyntaxException; | ||
| import java.net.URL; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
|
|
||
| import jakarta.servlet.ServletContainerInitializer; | ||
| import org.eclipse.jetty.ee11.webapp.WebAppContext; | ||
| import org.eclipse.jetty.http.HttpTester; | ||
| import org.eclipse.jetty.server.LocalConnector; | ||
| import org.eclipse.jetty.server.Server; | ||
| import org.eclipse.jetty.toolchain.test.FS; | ||
| import org.eclipse.jetty.toolchain.test.MavenPaths; | ||
| import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; | ||
| import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; | ||
| import org.eclipse.jetty.util.TypeUtil; | ||
| import org.eclipse.jetty.util.URIUtil; | ||
| import org.eclipse.jetty.util.component.LifeCycle; | ||
| import org.example.webapp.ClassLoaderGetResourcesServlet; | ||
| import org.example.webapp.ServletContainerInitializerDiscoveryServlet; | ||
| import org.junit.jupiter.api.AfterEach; | ||
| import org.junit.jupiter.api.Assertions; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
|
|
||
| import static org.hamcrest.MatcherAssert.assertThat; | ||
| import static org.hamcrest.Matchers.containsString; | ||
| import static org.hamcrest.Matchers.greaterThan; | ||
|
|
||
| @ExtendWith(WorkDirExtension.class) | ||
| public class ClassLoaderProtectResourcesTest | ||
| { | ||
| public WorkDir workDir; | ||
| private Server server; | ||
| private LocalConnector connector; | ||
|
|
||
| public void startServer(WebAppContext webAppContext) throws Exception | ||
| { | ||
| server = new Server(); | ||
| connector = new LocalConnector(server); | ||
| server.addConnector(connector); | ||
|
|
||
| server.setHandler(webAppContext); | ||
| server.start(); | ||
| } | ||
|
|
||
| @AfterEach | ||
| public void destroy() | ||
| { | ||
| LifeCycle.stop(server); | ||
| } | ||
|
|
||
| @Test | ||
| public void testServiceLoaderVisibility() throws Exception | ||
| { | ||
| ClassLoader serverClassLoader = Thread.currentThread().getContextClassLoader(); | ||
| String resourceName = "META-INF/services/" + ServletContainerInitializer.class.getName(); | ||
| List<URL> allServiceFiles = Collections.list(serverClassLoader.getResources(resourceName)); | ||
| // Find the ee11-apache-jsp URLs | ||
| List<URI> ee11ApacheJspHits = allServiceFiles.stream() | ||
| .map(ClassLoaderProtectResourcesTest::toJarURI) | ||
| .filter(uri -> uri.toASCIIString().contains("ee11-apache-jsp")) | ||
| .toList(); | ||
| assertThat("Expecting some ee11-apache-jsp SCI", ee11ApacheJspHits.size(), greaterThan(0)); | ||
| int expectedHitsFromServlet = allServiceFiles.size() - ee11ApacheJspHits.size(); | ||
|
|
||
| // Create webapp directory | ||
| Path basePath = workDir.getEmptyPathDir(); | ||
| copyTestClassIntoWebapp(ServletContainerInitializerDiscoveryServlet.class, basePath); | ||
|
|
||
| WebAppContext webapp = new WebAppContext(); | ||
| webapp.setContextPath("/"); | ||
| webapp.setBaseResourceAsPath(basePath); | ||
| webapp.addServlet(ServletContainerInitializerDiscoveryServlet.class.getName(), "/lookup"); | ||
|
|
||
| // Protect a specific jar's SCI from being discovered. | ||
| ee11ApacheJspHits.forEach(uri -> | ||
| webapp.getHiddenClassMatcher().add(uri.toASCIIString())); | ||
|
|
||
| startServer(webapp); | ||
|
|
||
| String rawRequest = """ | ||
| GET /lookup HTTP/1.1\r | ||
| Host: localhost\r | ||
| Connection: close\r | ||
| \r | ||
| """; | ||
| String rawResponse = connector.getResponse(rawRequest); | ||
| HttpTester.Response response = HttpTester.parseResponse(rawResponse); | ||
| assertThat(response.getContent(), containsString("Service Count: %s\n".formatted(expectedHitsFromServlet))); | ||
| } | ||
|
|
||
| private static URI toJarURI(URL url) | ||
| { | ||
| try | ||
| { | ||
| return URIUtil.unwrapContainer(url.toURI()); | ||
| } | ||
| catch (URISyntaxException e) | ||
| { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testGetProtectedResources() throws Exception | ||
| { | ||
| // Create webapp directory | ||
| Path basePath = workDir.getEmptyPathDir(); | ||
| copyTestClassIntoWebapp(ClassLoaderGetResourcesServlet.class, basePath); | ||
| WebAppContext webapp = new WebAppContext(); | ||
| webapp.setContextPath("/"); | ||
| webapp.setBaseResourceAsPath(basePath); | ||
| webapp.addServlet(ClassLoaderGetResourcesServlet.class.getName(), "/lookup"); | ||
|
|
||
| // The resource name we will be testing | ||
| String resourceName = "META-INF/services/org.eclipse.jetty.http.HttpFieldPreEncoder"; | ||
|
|
||
| // Protect them from being discovered | ||
| ClassLoader serverClassLoader = Thread.currentThread().getContextClassLoader(); | ||
| protectServerResource(serverClassLoader, resourceName, webapp); | ||
|
|
||
| startServer(webapp); | ||
|
|
||
| String rawRequest = """ | ||
| GET /lookup?resourceName=%s HTTP/1.1\r | ||
| Host: localhost\r | ||
| Connection: close\r | ||
| \r | ||
| """.formatted(resourceName); | ||
| String rawResponse = connector.getResponse(rawRequest); | ||
| HttpTester.Response response = HttpTester.parseResponse(rawResponse); | ||
| assertThat(response.getContent(), containsString("Hits: 0\n")); | ||
| } | ||
|
|
||
| private void protectServerResource(ClassLoader serverClassLoader, String resourceName, WebAppContext webapp) throws IOException, URISyntaxException | ||
| { | ||
| // Find resources that belong only on server side. | ||
| List<URL> urls = Collections.list(serverClassLoader.getResources(resourceName)); | ||
| assert !urls.isEmpty(); | ||
|
|
||
| // Lets setup exclusions, by location ("file:///" urls), for these. | ||
| for (URL url: urls) | ||
| { | ||
| URI uri = URIUtil.unwrapContainer(url.toURI()); | ||
| // This is the key configuration to allow protecting of server resources | ||
| // even when using ClassLoader.getResource() or ClassLoader.getResources() | ||
| webapp.getHiddenClassMatcher().add(uri.toASCIIString()); | ||
| } | ||
| } | ||
|
|
||
| private static void copyTestClassIntoWebapp(Class<?> clazz, Path webappRoot) throws IOException | ||
| { | ||
| String pathToCopy = TypeUtil.toClassReference(clazz); | ||
| Path classFile = MavenPaths.targetDir().resolve("test-classes/" + pathToCopy); | ||
| Assertions.assertTrue(Files.isRegularFile(classFile), "Class should exist file: " + classFile); | ||
|
|
||
| Path classesDir = webappRoot.resolve("WEB-INF/classes"); | ||
| FS.ensureDirExists(classesDir); | ||
|
|
||
| Path destFile = classesDir.resolve(pathToCopy); | ||
| FS.ensureDirExists(destFile.getParent()); | ||
| Files.copy(classFile, destFile); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| // | ||
| // ======================================================================== | ||
| // Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. | ||
| // | ||
| // This program and the accompanying materials are made available under the | ||
| // terms of the Eclipse Public License v. 2.0 which is available at | ||
| // https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 | ||
| // which is available at https://www.apache.org/licenses/LICENSE-2.0. | ||
| // | ||
| // SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 | ||
| // ======================================================================== | ||
| // | ||
|
|
||
| package org.example.webapp; | ||
|
|
||
| import java.io.IOException; | ||
| import java.io.PrintWriter; | ||
| import java.net.URL; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
|
|
||
| import jakarta.servlet.ServletException; | ||
| import jakarta.servlet.http.HttpServlet; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
|
|
||
| public class ClassLoaderGetResourcesServlet extends HttpServlet | ||
| { | ||
| @Override | ||
| protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if it is possible to change this servlet such as it also does This is tricky as the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll see if I can find something that fits the ServiceLoader scenario too. (a different test) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved the tests to The This testcase shows that the isolation works, as designed. |
||
| { | ||
| String resourceName = req.getParameter("resourceName"); | ||
| if (resourceName == null || resourceName.isBlank()) | ||
| throw new ServletException("Missing resourceName parameter"); | ||
| List<URL> hits = Collections.list(Thread.currentThread().getContextClassLoader().getResources(resourceName)); | ||
|
||
| resp.setStatus(200); | ||
| resp.setCharacterEncoding("utf-8"); | ||
| resp.setContentType("text/plain"); | ||
| PrintWriter out = resp.getWriter(); | ||
| out.printf("Hits: %d%n", hits.size()); | ||
| for (int i = 0; i < hits.size(); i++) | ||
| { | ||
| out.printf("[%d] %s%n", i, hits.get(i)); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| // | ||
| // ======================================================================== | ||
| // Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. | ||
| // | ||
| // This program and the accompanying materials are made available under the | ||
| // terms of the Eclipse Public License v. 2.0 which is available at | ||
| // https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 | ||
| // which is available at https://www.apache.org/licenses/LICENSE-2.0. | ||
| // | ||
| // SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 | ||
| // ======================================================================== | ||
| // | ||
|
|
||
| package org.example.webapp; | ||
|
|
||
| import java.io.IOException; | ||
| import java.io.PrintWriter; | ||
| import java.util.List; | ||
| import java.util.ServiceLoader; | ||
|
|
||
| import jakarta.servlet.ServletContainerInitializer; | ||
| import jakarta.servlet.ServletException; | ||
| import jakarta.servlet.http.HttpServlet; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
|
|
||
| public class ServletContainerInitializerDiscoveryServlet extends HttpServlet | ||
| { | ||
| @Override | ||
| protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException | ||
| { | ||
| ServiceLoader<ServletContainerInitializer> services = ServiceLoader.load(ServletContainerInitializer.class); | ||
| List<String> serviceNames = services.stream() | ||
| .map(provider -> | ||
| { | ||
| return provider.get().getClass().getName(); | ||
| }) | ||
| .sorted() | ||
| .toList(); | ||
| resp.setStatus(200); | ||
| resp.setCharacterEncoding("utf-8"); | ||
| resp.setContentType("text/plain"); | ||
| PrintWriter out = resp.getWriter(); | ||
| out.printf("Service Count: %d%n", serviceNames.size()); | ||
| for (int i = 0; i < serviceNames.size(); i++) | ||
| { | ||
| out.printf("[%d] %s%n", i, serviceNames.get(i)); | ||
| } | ||
| } | ||
| } |
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.
I think the issue is about the fact that this protection should be automatic, and not require manual intervention like you're doing here.
Uh oh!
There was an error while loading. Please reload this page.
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.
The ClassMatcher is the one responsible (used in the WebAppClassLoader) to isolate the classes and/or resources in the WebAppContext.
It can isolate (by default), or let things through.
If we use that ClassMatcher with a classname pattern (aka
org.eclipse.jetty.http.), then only classes are protected or exposed (has no impact on resources).But, as shown in this test case, we use a ClassMatcher location instead (aka
file:///path/to/jettybase/lib/jetty-http-12.1.3.jar) then all files in that jar are protected (this is what this testcase is actually doing).This is how we do it for jetty logging in a start.jar module.
jetty.project/jetty-home/src/main/resources/modules/logging-jetty.mod
Lines 23 to 25 in f8d520d
For jetty-home / jetty-base, the protection can be a 1 liner in
server.modThat would protect all jars in
${jetty.home}/lib.We don't do this, as there are some SCIs that we do want exposed to webapps.
And things we need to find in JSP / EL libs too.