-
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?
Issue #4652 - test to demonstrate the WebAppContext ClassLoader isolation #13838
Conversation
…tion Testcase uses WebAppContext.hiddenClassMatcher to set server jars (by location) that have the resource that needs protecting.
...y-ee11-test-integration/src/test/java/org/example/webapp/ClassLoaderGetResourcesServlet.java
Fixed
Show fixed
Hide fixed
| 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 comment
The 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 ServiceLoader.load(HttpFieldPreEncoder.class) as this is closer to the original issue which states If a webapp uses the ServiceLoader...?
This is tricky as the HttpFieldPreEncoder is not visible in this servlet's classloader so that might not be possible.
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'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 comment
The reason will be displayed to describe this comment to others. Learn more.
I moved the tests to jetty-ee11/jetty-ee11-tests/jetty-ee11-test-integration.
That way I can use the ServiceContainerInitializer.class (available from jakarta servlet jar) against ServiceLoader to find some SCI that exist in the server classpath.
The jetty-ee11-test-integration project finds SCI's from apache-jsp, and various websocket jars (ours and jakarta versions).
This testcase shows that the isolation works, as designed.
|
|
||
| // Protect them from being discovered | ||
| ClassLoader serverClassLoader = Thread.currentThread().getContextClassLoader(); | ||
| protectServerResource(serverClassLoader, resourceName, webapp); |
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.
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
| [ini] | |
| jetty.webapp.addHiddenClasses+=,org.eclipse.jetty.logging. | |
| jetty.webapp.addHiddenClasses+=,${jetty.home.uri}/lib/logging/ |
For jetty-home / jetty-base, the protection can be a 1 liner in server.mod
[ini]
jetty.webapp.addHiddenClasses+=,${jetty.home.uri}/lib/
That 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.
...y-ee11-test-integration/src/test/java/org/example/webapp/ClassLoaderGetResourcesServlet.java
Dismissed
Show dismissed
Hide dismissed
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 all looks reasonable to me, but I'd feel better if @gregw agrees that this is sufficient to close the original issue
Testcase uses
WebAppContext.getHiddenClassMatcher().add(String)to set server jars (by location) that have the resource that needs protecting
in the webapp. This properly isolates resources from being seen by the WebAppContext.
Closes #4652