From 0e5bc9d57ded7b41bd4a663a42f4d36ec62636a9 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 23 May 2024 10:27:49 +1000 Subject: [PATCH 1/5] merged ee10 to ee11 --- .../jetty/ee11/servlet/AsyncContextEvent.java | 6 +- .../ee11/webapp/AbstractConfiguration.java | 28 ++-- .../jetty/ee11/webapp/ClassMatcher.java | 65 -------- .../jetty/ee11/webapp/Configuration.java | 13 +- .../jetty/ee11/webapp/WebAppClassLoader.java | 1 - .../jetty/ee11/webapp/WebAppContext.java | 144 ++---------------- .../jetty/ee11/webapp/WebAppContextTest.java | 40 +++-- 7 files changed, 59 insertions(+), 238 deletions(-) delete mode 100644 jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/ClassMatcher.java diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/AsyncContextEvent.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/AsyncContextEvent.java index af6730104447..e70ae134eab7 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/AsyncContextEvent.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/AsyncContextEvent.java @@ -20,6 +20,7 @@ import jakarta.servlet.ServletResponse; import jakarta.servlet.http.HttpServletRequest; import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.util.ExceptionUtil; import org.eclipse.jetty.util.thread.Scheduler; public class AsyncContextEvent extends AsyncEvent implements Runnable @@ -149,9 +150,6 @@ public void run() public void addThrowable(Throwable e) { - if (_throwable == null) - _throwable = e; - else if (e != _throwable) - _throwable.addSuppressed(e); + _throwable = ExceptionUtil.combine(_throwable, e); } } diff --git a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/AbstractConfiguration.java b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/AbstractConfiguration.java index db7eb34ff409..aa1fe480476f 100644 --- a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/AbstractConfiguration.java +++ b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/AbstractConfiguration.java @@ -19,13 +19,15 @@ import java.util.Collections; import java.util.List; +import org.eclipse.jetty.util.ClassMatcher; + public class AbstractConfiguration implements Configuration { private final boolean _enabledByDefault; private final List _after; private final List _before; - private final ClassMatcher _system; - private final ClassMatcher _server; + private final ClassMatcher _protected; + private final ClassMatcher _hidden; public static class Builder { @@ -88,7 +90,7 @@ public Builder addDependents(Class... classes) /** * Protect classes from modification by the web application by adding them - * to the {@link WebAppConfiguration#getSystemClasses()} + * to the {@link WebAppConfiguration#getProtectedClasses()} * * @param classes classname or package pattern */ @@ -100,7 +102,7 @@ public Builder protect(String... classes) /** * Hide classes from the web application by adding them - * to the {@link WebAppConfiguration#getServerClasses()} + * to the {@link WebAppConfiguration#getHiddenClasses()} * * @param classes classname or package pattern */ @@ -112,7 +114,7 @@ public Builder hide(String... classes) /** * Expose classes to the web application by adding them - * as exclusions to the {@link WebAppConfiguration#getServerClasses()} + * as exclusions to the {@link WebAppConfiguration#getHiddenClasses()} * * @param classes classname or package pattern */ @@ -129,9 +131,9 @@ public Builder expose(String... classes) /** * Protect classes from modification by the web application by adding them - * to the {@link WebAppConfiguration#getSystemClasses()} and + * to the {@link WebAppConfiguration#getProtectedClasses()} and * expose them to the web application by adding them - * as exclusions to the {@link WebAppConfiguration#getServerClasses()} + * as exclusions to the {@link WebAppConfiguration#getHiddenClasses()} * * @param classes classname or package pattern */ @@ -154,8 +156,8 @@ protected AbstractConfiguration(Builder builder) _enabledByDefault = builder._enabledByDefault; _after = List.copyOf(builder._after); _before = List.copyOf(builder._before); - _system = new ClassMatcher(builder._system).asImmutable(); - _server = new ClassMatcher(builder._server).asImmutable(); + _protected = new ClassMatcher(builder._system).asImmutable(); + _hidden = new ClassMatcher(builder._server).asImmutable(); } @Override @@ -171,15 +173,15 @@ public Collection getDependencies() } @Override - public ClassMatcher getSystemClasses() + public ClassMatcher getProtectedClasses() { - return _system; + return _protected; } @Override - public ClassMatcher getServerClasses() + public ClassMatcher getHiddenClasses() { - return _server; + return _hidden; } @Override diff --git a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/ClassMatcher.java b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/ClassMatcher.java deleted file mode 100644 index 8baa51ccf1ac..000000000000 --- a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/ClassMatcher.java +++ /dev/null @@ -1,65 +0,0 @@ -// -// ======================================================================== -// 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.webapp; - -import java.net.URI; -import java.util.Map; - -import org.eclipse.jetty.util.IncludeExcludeSet; - -/** - * @deprecated Use org.eclipse.jetty.util.ClassMatcher - */ - -@Deprecated(since = "12.0.8", forRemoval = true) -public class ClassMatcher extends org.eclipse.jetty.util.ClassMatcher -{ - public ClassMatcher() - { - super(); - } - - public ClassMatcher(ClassMatcher patterns) - { - super(patterns); - } - - public ClassMatcher(org.eclipse.jetty.util.ClassMatcher patterns) - { - super(patterns); - } - - public ClassMatcher(String... patterns) - { - super(patterns); - } - - public ClassMatcher(String pattern) - { - super(pattern); - } - - protected ClassMatcher(Map entries, IncludeExcludeSet patterns, IncludeExcludeSet locations) - { - super(entries, patterns, locations); - } - - @Override - public ClassMatcher asImmutable() - { - return new ClassMatcher(Map.copyOf(_entries), - _patterns.asImmutable(), - _locations.asImmutable()); - } -} diff --git a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/Configuration.java b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/Configuration.java index e2f1957f43ff..23b592f5d88b 100644 --- a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/Configuration.java +++ b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/Configuration.java @@ -17,6 +17,7 @@ import java.util.Collections; import java.util.ServiceLoader; +import org.eclipse.jetty.util.ClassMatcher; import org.eclipse.jetty.util.TopologicalSort; /** @@ -43,8 +44,8 @@ * (eg {@link JndiConfiguration}, {@link JaasConfiguration}} etc.) can be added or removed without concern * for ordering. *

- *

Also since Jetty-9.4, Configurations are responsible for providing {@link #getServerClasses()} and - * {@link #getSystemClasses()} to configure the {@link WebAppClassLoader} for each context. + *

Also since Jetty-9.4, Configurations are responsible for providing {@link #getHiddenClasses()} and + * {@link #getProtectedClasses()} to configure the {@link WebAppClassLoader} for each context. *

*/ public interface Configuration @@ -93,21 +94,21 @@ default Collection getDependents() } /** - * Get the system classes associated with this Configuration. + * Get the system (protected) classes associated with this Configuration. * * @return ClassMatcher of system classes. */ - default ClassMatcher getSystemClasses() + default ClassMatcher getProtectedClasses() { return new ClassMatcher(); } /** - * Get the system classes associated with this Configuration. + * Get the server (hidden) classes associated with this Configuration. * * @return ClassMatcher of server classes. */ - default ClassMatcher getServerClasses() + default ClassMatcher getHiddenClasses() { return new ClassMatcher(); } diff --git a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppClassLoader.java b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppClassLoader.java index 6558fbfa1e7e..da46960c5387 100644 --- a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppClassLoader.java +++ b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppClassLoader.java @@ -521,7 +521,6 @@ protected Class loadAsResource(final String name, boolean checkSystemResource if (webappUrl != null && (!checkSystemResource || !_context.isProtectedResource(name, webappUrl))) { - webappClass = this.foundClass(name, webappUrl); resolveClass(webappClass); if (LOG.isDebugEnabled()) diff --git a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppContext.java b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppContext.java index 5d401934cc50..c2bd0ef0465b 100644 --- a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppContext.java +++ b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppContext.java @@ -98,20 +98,6 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL private static final String[] __dftProtectedTargets = {"/WEB-INF", "/META-INF"}; - /** - * @deprecated use {@link WebAppClassLoading#DEFAULT_PROTECTED_CLASSES} - */ - @Deprecated (forRemoval = true, since = "12.0.9") - public static final org.eclipse.jetty.ee11.webapp.ClassMatcher __dftSystemClasses = - new org.eclipse.jetty.ee11.webapp.ClassMatcher(WebAppClassLoading.DEFAULT_PROTECTED_CLASSES); - - /** - * @deprecated use {@link WebAppClassLoading#DEFAULT_HIDDEN_CLASSES} - */ - @Deprecated (forRemoval = true, since = "12.0.9") - public static final org.eclipse.jetty.ee11.webapp.ClassMatcher __dftServerClasses = - new org.eclipse.jetty.ee11.webapp.ClassMatcher(WebAppClassLoading.DEFAULT_HIDDEN_CLASSES); - private final ClassMatcher _protectedClasses = new ClassMatcher(WebAppClassLoading.getProtectedClasses(ServletContextHandler.ENVIRONMENT)); private final ClassMatcher _hiddenClasses = new ClassMatcher(WebAppClassLoading.getHiddenClasses(ServletContextHandler.ENVIRONMENT)); @@ -428,7 +414,7 @@ public void preConfigure() throws Exception // Add the known server class inclusions for all known configurations for (Configuration configuration : Configurations.getKnown()) { - _hiddenClasses.include(configuration.getServerClasses().getInclusions()); + _hiddenClasses.include(configuration.getHiddenClasses().getInclusions()); } // Setup Configuration classes for this webapp! @@ -436,8 +422,8 @@ public void preConfigure() throws Exception _configurations.sort(); for (Configuration configuration : _configurations) { - _protectedClasses.add(configuration.getSystemClasses().getPatterns()); - _hiddenClasses.exclude(configuration.getServerClasses().getExclusions()); + _protectedClasses.add(configuration.getProtectedClasses().getPatterns()); + _hiddenClasses.exclude(configuration.getHiddenClasses().getExclusions()); } // Configure classloader @@ -623,9 +609,9 @@ public PermissionCollection getPermissions() } /** - * Set the hidden (aka server) classes patterns. + * Set the hidden (server) classes patterns. *

- * These classes/packages are used to implement the server and are hiddenClasses + * These classes/packages are used to implement the server and are hidden * from the context. If the context needs to load these classes, it must have its * own copy of them in WEB-INF/lib or WEB-INF/classes. * @@ -638,7 +624,7 @@ public void setHiddenClassMatcher(ClassMatcher hiddenClasses) } /** - * Set the protected (aka system) classes patterns. + * Set the protected (system) classes patterns. *

* These classes/packages are provided by the JVM and * cannot be replaced by classes of the same name from WEB-INF, @@ -683,7 +669,7 @@ public ClassMatcher getProtectedClassMatcher() } /** - * @return The ClassMatcher used to match Server (hiddenClasses) classes + * @return The ClassMatcher used to match Server (hidden) classes */ public ClassMatcher getHiddenClassMatcher() { @@ -696,7 +682,7 @@ public String[] getProtectedClasses() return _protectedClasses.getPatterns(); } - @ManagedAttribute(value = "classes and packages hiddenClasses by the context classloader", readonly = true) + @ManagedAttribute(value = "classes and packages hidden by the context classloader", readonly = true) public String[] getHiddenClasses() { return _hiddenClasses.getPatterns(); @@ -726,116 +712,6 @@ public boolean isProtectedResource(String name, URL url) return _protectedClasses.match(name, url); } - /** - * @deprecated use {@link #setHiddenClassMatcher(ClassMatcher)} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public void setServerClassMatcher(ClassMatcher serverClasses) - { - _hiddenClasses.clear(); - _hiddenClasses.add(serverClasses.getPatterns()); - } - - /** - * @deprecated use {@link #setProtectedClassMatcher(ClassMatcher)} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public void setSystemClassMatcher(ClassMatcher systemClasses) - { - _protectedClasses.clear(); - _protectedClasses.add(systemClasses.getPatterns()); - } - - /** - * @deprecated use {@link #addHiddenClassMatcher(ClassMatcher)} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public void addServerClassMatcher(ClassMatcher serverClasses) - { - _hiddenClasses.add(serverClasses.getPatterns()); - } - - /** - * @deprecated use {@link #addProtectedClassMatcher(ClassMatcher)} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public void addSystemClassMatcher(ClassMatcher systemClasses) - { - _protectedClasses.add(systemClasses.getPatterns()); - } - - /** - * @deprecated use {@link #getProtectedClassMatcher()} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public ClassMatcher getSystemClassMatcher() - { - return _protectedClasses; - } - - /** - * @deprecated use {@link #getHiddenClassMatcher()} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public ClassMatcher getServerClassMatcher() - { - return _hiddenClasses; - } - - /** - * @deprecated use {@link #getProtectedClasses()} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public String[] getSystemClasses() - { - return _protectedClasses.getPatterns(); - } - - /** - * @deprecated use {@link #getHiddenClasses()} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public String[] getServerClasses() - { - return _hiddenClasses.getPatterns(); - } - - /** - * @deprecated use {@link #isHiddenClass(Class)} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public boolean isServerClass(Class clazz) - { - return _hiddenClasses.match(clazz); - } - - /** - * @deprecated use {@link #isProtectedClass(Class)} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public boolean isSystemClass(Class clazz) - { - return _protectedClasses.match(clazz); - } - - /** - * @deprecated use {@link #isHiddenResource(String, URL)} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public boolean isServerResource(String name, URL url) - { - return _hiddenClasses.match(name, url); - } - - /** - * @deprecated use {@link #isProtectedResource(String, URL)} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public boolean isSystemResource(String name, URL url) - { - return _protectedClasses.match(name, url); - } - @Override public void setServer(Server server) { @@ -1590,7 +1466,7 @@ public MetaData getMetaData() } /** - * Add a Server Class pattern to use for all ee9 WebAppContexts. + * Add a Server Class pattern to use for all WebAppContexts. * @param server The {@link Server} instance to add classes to * @param patterns the patterns to use * @see #getHiddenClassMatcher() @@ -1604,7 +1480,7 @@ public static void addServerClasses(Server server, String... patterns) } /** - * Add a System Class pattern to use for all ee9 WebAppContexts. + * Add a System Class pattern to use for all WebAppContexts. * @param server The {@link Server} instance to add classes to * @param patterns the patterns to use * @see #getProtectedClassMatcher() diff --git a/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/WebAppContextTest.java b/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/WebAppContextTest.java index b3ad54ee35c2..bc4a2119ce2e 100644 --- a/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/WebAppContextTest.java +++ b/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/WebAppContextTest.java @@ -940,13 +940,13 @@ public void testSetServerPropagation() } @Test - public void testAddServerClasses() throws Exception + public void testAddHiddenClasses() throws Exception { Server server = newServer(); String testPattern = "org.eclipse.jetty.ee11.webapp.test."; - WebAppContext.addServerClasses(server, testPattern); + WebAppClassLoading.addHiddenClasses(server, testPattern); WebAppContext context = new WebAppContext(); context.setContextPath("/"); @@ -957,25 +957,30 @@ public void testAddServerClasses() throws Exception Path warPath = createWar(testPath, "test.war"); context.setBaseResource(context.getResourceFactory().newResource(warPath)); + // Check context specific + context.getHiddenClassMatcher().add("org.context.specific."); + server.setHandler(context); server.start(); - List serverClasses = List.of(context.getHiddenClasses()); - assertThat("Should have environment specific test pattern", serverClasses, hasItem(testPattern)); - assertThat("Should have pattern from defaults", serverClasses, hasItem("org.eclipse.jetty.")); - assertThat("Should have pattern from JaasConfiguration", serverClasses, hasItem("-org.eclipse.jetty.security.jaas.")); + List hiddenClasses = List.of(context.getHiddenClasses()); + assertThat("Should have environment specific test pattern", hiddenClasses, hasItem(testPattern)); + assertThat("Should have pattern from defaults", hiddenClasses, hasItem("org.eclipse.jetty.")); + assertThat("Should have pattern from JaasConfiguration", hiddenClasses, hasItem("-org.eclipse.jetty.security.jaas.")); for (String defaultServerClass: WebAppClassLoading.DEFAULT_HIDDEN_CLASSES) - assertThat("Should have default patterns", serverClasses, hasItem(defaultServerClass)); + assertThat("Should have default patterns", hiddenClasses, hasItem(defaultServerClass)); + + assertThat("context API", hiddenClasses, hasItem("org.context.specific.")); } @Test - public void testAddSystemClasses() throws Exception + public void testAddProtectedClasses() throws Exception { Server server = newServer(); String testPattern = "org.eclipse.jetty.ee11.webapp.test."; - WebAppContext.addSystemClasses(server, testPattern); + WebAppClassLoading.addProtectedClasses(server, testPattern); WebAppContext context = new WebAppContext(); context.setContextPath("/"); @@ -985,15 +990,20 @@ public void testAddSystemClasses() throws Exception Path warPath = createWar(testPath, "test.war"); context.setBaseResource(context.getResourceFactory().newResource(warPath)); + // Check context specific + context.getProtectedClassMatcher().add("org.context.specific."); + server.setHandler(context); server.start(); - List systemClasses = List.of(context.getProtectedClasses()); - assertThat("Should have environment specific test pattern", systemClasses, hasItem(testPattern)); - assertThat("Should have pattern from defaults", systemClasses, hasItem("javax.")); - assertThat("Should have pattern from defaults", systemClasses, hasItem("jakarta.")); - assertThat("Should have pattern from JaasConfiguration", systemClasses, hasItem("org.eclipse.jetty.security.jaas.")); + List protectedClasses = List.of(context.getProtectedClasses()); + assertThat("Should have environment specific test pattern", protectedClasses, hasItem(testPattern)); + assertThat("Should have pattern from defaults", protectedClasses, hasItem("javax.")); + assertThat("Should have pattern from defaults", protectedClasses, hasItem("jakarta.")); + assertThat("Should have pattern from JaasConfiguration", protectedClasses, hasItem("org.eclipse.jetty.security.jaas.")); for (String defaultSystemClass: WebAppClassLoading.DEFAULT_PROTECTED_CLASSES) - assertThat("Should have default patterns", systemClasses, hasItem(defaultSystemClass)); + assertThat("Should have default patterns", protectedClasses, hasItem(defaultSystemClass)); + + assertThat("context API", protectedClasses, hasItem("org.context.specific.")); } } From 24335af11539dbdc99601f055e36443d2cdc90a1 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 23 May 2024 10:56:27 +1000 Subject: [PATCH 2/5] merged ee10 to ee11 --- .../java/org/eclipse/jetty/ee11/cdi/CdiConfiguration.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-ee11/jetty-ee11-cdi/src/main/java/org/eclipse/jetty/ee11/cdi/CdiConfiguration.java b/jetty-ee11/jetty-ee11-cdi/src/main/java/org/eclipse/jetty/ee11/cdi/CdiConfiguration.java index a42d6dd2afbe..edf22582c290 100644 --- a/jetty-ee11/jetty-ee11-cdi/src/main/java/org/eclipse/jetty/ee11/cdi/CdiConfiguration.java +++ b/jetty-ee11/jetty-ee11-cdi/src/main/java/org/eclipse/jetty/ee11/cdi/CdiConfiguration.java @@ -31,11 +31,11 @@ public CdiConfiguration() { super(new Builder() .protectAndExpose("org.eclipse.jetty.ee11.cdi.CdiServletContainerInitializer") - .hide(getHiddenClasses()) + .hide(getCdiHiddenClasses()) .addDependents(AnnotationConfiguration.class, PlusConfiguration.class)); } - private static String[] getHiddenClasses() + private static String[] getCdiHiddenClasses() { //Only hide the cdi api classes if there is not also an impl on the //environment classpath - vital for embedded uses. From 088788e086eb37ffb3c8e25c388342732b57a52b Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 23 May 2024 11:37:05 +1000 Subject: [PATCH 3/5] remove more system/server classes from ee11 --- .../annotations/AnnotationConfiguration.java | 2 +- .../jetty/ee11/osgi/boot/EE11Activator.java | 10 ++--- .../ee11/plus/webapp/EnvConfiguration.java | 2 +- .../ee11/webapp/JettyWebXmlConfiguration.java | 2 +- .../jetty/ee11/webapp/WebAppClassLoader.java | 24 +++++----- .../jetty/ee11/webapp/WebAppContext.java | 44 ++++--------------- .../ee11/webapp/WebAppClassLoaderTest.java | 4 +- .../jetty/ee11/webapp/WebAppContextTest.java | 4 +- 8 files changed, 32 insertions(+), 60 deletions(-) diff --git a/jetty-ee11/jetty-ee11-annotations/src/main/java/org/eclipse/jetty/ee11/annotations/AnnotationConfiguration.java b/jetty-ee11/jetty-ee11-annotations/src/main/java/org/eclipse/jetty/ee11/annotations/AnnotationConfiguration.java index 2d18fa87518b..5db255685227 100644 --- a/jetty-ee11/jetty-ee11-annotations/src/main/java/org/eclipse/jetty/ee11/annotations/AnnotationConfiguration.java +++ b/jetty-ee11/jetty-ee11-annotations/src/main/java/org/eclipse/jetty/ee11/annotations/AnnotationConfiguration.java @@ -351,7 +351,7 @@ public void addStartupClasses(Class... clazzes) protected Set> resolveStartupClasses() throws Exception { final Set> classes = new HashSet<>(); - WebAppClassLoader.runWithServerClassAccess(() -> + WebAppClassLoader.runWithHiddenClassAccess(() -> { for (String name:_startupClassNames) { diff --git a/jetty-ee11/jetty-ee11-osgi/jetty-ee11-osgi-boot/src/main/java/org/eclipse/jetty/ee11/osgi/boot/EE11Activator.java b/jetty-ee11/jetty-ee11-osgi/jetty-ee11-osgi-boot/src/main/java/org/eclipse/jetty/ee11/osgi/boot/EE11Activator.java index 93bf311400fa..2782247b908b 100644 --- a/jetty-ee11/jetty-ee11-osgi/jetty-ee11-osgi-boot/src/main/java/org/eclipse/jetty/ee11/osgi/boot/EE11Activator.java +++ b/jetty-ee11/jetty-ee11-osgi/jetty-ee11-osgi-boot/src/main/java/org/eclipse/jetty/ee11/osgi/boot/EE11Activator.java @@ -293,10 +293,10 @@ public ContextHandler createContextHandler(AbstractContextProvider provider, App try { Thread.currentThread().setContextClassLoader(contextHandler.getClassLoader()); - WebAppClassLoader.runWithServerClassAccess(() -> + WebAppClassLoader.runWithHiddenClassAccess(() -> { XmlConfiguration xmlConfiguration = new XmlConfiguration(ResourceFactory.of(contextHandler).newResource(contextXmlURI)); - WebAppClassLoader.runWithServerClassAccess(() -> + WebAppClassLoader.runWithHiddenClassAccess(() -> { Map properties = new HashMap<>(); xmlConfiguration.getIdMap().put("Server", osgiApp.getDeploymentManager().getServer()); @@ -382,7 +382,7 @@ public ContextHandler createContextHandler(AbstractContextProvider provider, App try { Thread.currentThread().setContextClassLoader(environmentLoader); - WebAppClassLoader.runWithServerClassAccess(() -> + WebAppClassLoader.runWithHiddenClassAccess(() -> { Configurations.getKnown(); return null; @@ -468,10 +468,10 @@ public ContextHandler createContextHandler(AbstractContextProvider provider, App try { Thread.currentThread().setContextClassLoader(webApp.getClassLoader()); - WebAppClassLoader.runWithServerClassAccess(() -> + WebAppClassLoader.runWithHiddenClassAccess(() -> { XmlConfiguration xmlConfiguration = new XmlConfiguration(ResourceFactory.of(webApp).newResource(contextXmlUri)); - WebAppClassLoader.runWithServerClassAccess(() -> + WebAppClassLoader.runWithHiddenClassAccess(() -> { Map properties = new HashMap<>(); xmlConfiguration.getIdMap().put("Server", osgiApp.getDeploymentManager().getServer()); diff --git a/jetty-ee11/jetty-ee11-plus/src/main/java/org/eclipse/jetty/ee11/plus/webapp/EnvConfiguration.java b/jetty-ee11/jetty-ee11-plus/src/main/java/org/eclipse/jetty/ee11/plus/webapp/EnvConfiguration.java index 5119d801b28d..684fa5e09830 100644 --- a/jetty-ee11/jetty-ee11-plus/src/main/java/org/eclipse/jetty/ee11/plus/webapp/EnvConfiguration.java +++ b/jetty-ee11/jetty-ee11-plus/src/main/java/org/eclipse/jetty/ee11/plus/webapp/EnvConfiguration.java @@ -87,7 +87,7 @@ public void configure(WebAppContext context) throws Exception { XmlConfiguration configuration = new XmlConfiguration(jettyEnvXmlResource); configuration.setJettyStandardIdsAndProperties(context.getServer(), null); - WebAppClassLoader.runWithServerClassAccess(() -> + WebAppClassLoader.runWithHiddenClassAccess(() -> { configuration.configure(context); return null; diff --git a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/JettyWebXmlConfiguration.java b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/JettyWebXmlConfiguration.java index 12b532b49f05..e3c9419c76a8 100644 --- a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/JettyWebXmlConfiguration.java +++ b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/JettyWebXmlConfiguration.java @@ -69,7 +69,7 @@ public void configure(WebAppContext context) throws Exception try { - WebAppClassLoader.runWithServerClassAccess(() -> + WebAppClassLoader.runWithHiddenClassAccess(() -> { jetty_config.configure(context); return null; diff --git a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppClassLoader.java b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppClassLoader.java index da46960c5387..6c73bf678c4b 100644 --- a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppClassLoader.java +++ b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppClassLoader.java @@ -71,7 +71,7 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility } private static final Logger LOG = LoggerFactory.getLogger(WebAppClassLoader.class); - private static final ThreadLocal __loadServerClasses = new ThreadLocal<>(); + private static final ThreadLocal __loadHiddenClasses = new ThreadLocal<>(); private final Context _context; private final ClassLoader _parent; @@ -117,7 +117,7 @@ public interface Context extends ClassVisibilityChecker } /** - * Run an action with access to ServerClasses + * Run an action with access to Hidden Classes *

Run the passed {@link PrivilegedExceptionAction} with the classloader * configured so as to allow server classes to be visible

* @@ -126,20 +126,20 @@ public interface Context extends ClassVisibilityChecker * @return The return from the action * @throws Exception if thrown by the action */ - public static T runWithServerClassAccess(PrivilegedExceptionAction action) throws Exception + public static T runWithHiddenClassAccess(PrivilegedExceptionAction action) throws Exception { - Boolean lsc = __loadServerClasses.get(); + Boolean lsc = __loadHiddenClasses.get(); try { - __loadServerClasses.set(true); + __loadHiddenClasses.set(true); return action.run(); } finally { if (lsc == null) - __loadServerClasses.remove(); + __loadHiddenClasses.remove(); else - __loadServerClasses.set(lsc); + __loadHiddenClasses.set(lsc); } } @@ -297,7 +297,7 @@ public Enumeration getResources(String name) throws IOException while (urls != null && urls.hasMoreElements()) { URL url = urls.nextElement(); - if (Boolean.TRUE.equals(__loadServerClasses.get()) || !_context.isHiddenResource(name, url)) + if (Boolean.TRUE.equals(__loadHiddenClasses.get()) || !_context.isHiddenResource(name, url)) fromParent.add(url); } @@ -345,7 +345,7 @@ public URL getResource(String name) // return if we have a url the webapp is allowed to see if (parentUrl != null && - (Boolean.TRUE.equals(__loadServerClasses.get()) || + (Boolean.TRUE.equals(__loadHiddenClasses.get()) || !_context.isHiddenResource(name, parentUrl))) resource = parentUrl; else @@ -373,7 +373,7 @@ public URL getResource(String name) // Couldn't find or see a webapp resource, so try a parent URL parentUrl = _parent.getResource(name); if (parentUrl != null && - (Boolean.TRUE.equals(__loadServerClasses.get()) || + (Boolean.TRUE.equals(__loadHiddenClasses.get()) || !_context.isHiddenResource(name, parentUrl))) resource = parentUrl; // We couldn't find a parent resource, so OK to return a webapp one if it exists @@ -420,7 +420,7 @@ protected Class loadClass(String name, boolean resolve) throws ClassNotFoundE throw new ClassNotFoundException("Bad ClassLoader: returned null for loadClass(" + name + ")"); // If the webapp is allowed to see this class - if (Boolean.TRUE.equals(__loadServerClasses.get()) || !_context.isHiddenClass(parentClass)) + if (Boolean.TRUE.equals(__loadHiddenClasses.get()) || !_context.isHiddenClass(parentClass)) { return parentClass; } @@ -468,7 +468,7 @@ else if (e != ex) { parentClass = _parent.loadClass(name); // If the webapp is allowed to see this class - if (Boolean.TRUE.equals(__loadServerClasses.get()) || !_context.isHiddenClass(parentClass)) + if (Boolean.TRUE.equals(__loadHiddenClasses.get()) || !_context.isHiddenClass(parentClass)) { return parentClass; } diff --git a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppContext.java b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppContext.java index c2bd0ef0465b..f8d65006a832 100644 --- a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppContext.java +++ b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebAppContext.java @@ -841,18 +841,18 @@ public String toString() @Override public void dump(Appendable out, String indent) throws IOException { - List systemClasses = null; + List protectedClasses = null; if (_protectedClasses != null) { - systemClasses = new ArrayList<>(_protectedClasses); - Collections.sort(systemClasses); + protectedClasses = new ArrayList<>(_protectedClasses); + Collections.sort(protectedClasses); } - List serverClasses = null; + List hiddenClasses = null; if (_hiddenClasses != null) { - serverClasses = new ArrayList<>(_hiddenClasses); - Collections.sort(serverClasses); + hiddenClasses = new ArrayList<>(_hiddenClasses); + Collections.sort(hiddenClasses); } String name = getDisplayName(); @@ -883,8 +883,8 @@ else if (getBaseResource() != null) dumpObjects(out, indent, new ClassLoaderDump(getClassLoader()), - new DumpableCollection("Systemclasses " + name, systemClasses), - new DumpableCollection("Serverclasses " + name, serverClasses), + new DumpableCollection("Protected classes " + name, protectedClasses), + new DumpableCollection("Hidden classes " + name, hiddenClasses), new DumpableCollection("Configurations " + name, _configurations), new DumpableCollection("Handler attributes " + name, asAttributeMap().entrySet()), new DumpableCollection("Context attributes " + name, getContext().asAttributeMap().entrySet()), @@ -1464,32 +1464,4 @@ public MetaData getMetaData() { return _metadata; } - - /** - * Add a Server Class pattern to use for all WebAppContexts. - * @param server The {@link Server} instance to add classes to - * @param patterns the patterns to use - * @see #getHiddenClassMatcher() - * @see #getHiddenClasses() - * @deprecated use {@link WebAppClassLoading#addProtectedClasses(Server, String...)} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public static void addServerClasses(Server server, String... patterns) - { - WebAppClassLoading.addHiddenClasses(server, patterns); - } - - /** - * Add a System Class pattern to use for all WebAppContexts. - * @param server The {@link Server} instance to add classes to - * @param patterns the patterns to use - * @see #getProtectedClassMatcher() - * @see #getProtectedClasses() - * @deprecated use {@link WebAppClassLoading#addHiddenClasses(Server, String...)} - */ - @Deprecated(since = "12.0.8", forRemoval = true) - public static void addSystemClasses(Server server, String... patterns) - { - WebAppClassLoading.addProtectedClasses(server, patterns); - } } diff --git a/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/WebAppClassLoaderTest.java b/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/WebAppClassLoaderTest.java index 72c8c9dcd5e6..1811843d5db0 100644 --- a/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/WebAppClassLoaderTest.java +++ b/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/WebAppClassLoaderTest.java @@ -353,7 +353,7 @@ public void testResources() throws Exception expected.add(webappWebInfLibAcme); expected.add(webappWebInfClasses); - assertThat("Resources Found (Parent Loader Priority == true) (with serverClasses filtering)", resources, ordered(expected)); + assertThat("Resources Found (Parent Loader Priority == true) (with hiddenClasses filtering)", resources, ordered(expected)); // dump(resources); // assertEquals(2,resources.size()); @@ -374,7 +374,7 @@ public void testResources() throws Exception expected.clear(); expected.add(targetTestClasses); - assertThat("Resources Found (Parent Loader Priority == true) (with systemClasses filtering)", resources, ordered(expected)); + assertThat("Resources Found (Parent Loader Priority == true) (with hiddenClasses filtering)", resources, ordered(expected)); } @Test diff --git a/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/WebAppContextTest.java b/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/WebAppContextTest.java index bc4a2119ce2e..20252099aff5 100644 --- a/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/WebAppContextTest.java +++ b/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/WebAppContextTest.java @@ -951,7 +951,7 @@ public void testAddHiddenClasses() throws Exception WebAppContext context = new WebAppContext(); context.setContextPath("/"); - Path testPath = MavenPaths.targetTestDir("testAddServerClasses"); + Path testPath = MavenPaths.targetTestDir("testAddHiddenClasses"); FS.ensureDirExists(testPath); FS.ensureEmpty(testPath); Path warPath = createWar(testPath, "test.war"); @@ -984,7 +984,7 @@ public void testAddProtectedClasses() throws Exception WebAppContext context = new WebAppContext(); context.setContextPath("/"); - Path testPath = MavenPaths.targetTestDir("testAddServerClasses"); + Path testPath = MavenPaths.targetTestDir("testAddHiddenClasses"); FS.ensureDirExists(testPath); FS.ensureEmpty(testPath); Path warPath = createWar(testPath, "test.war"); From c10df2d7ab235b7dda24bc265e223ac5a6e07726 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 23 May 2024 11:51:25 +1000 Subject: [PATCH 4/5] remove more system/server classes from ee11 --- .../it/jetty-cdi-start-forked/src/main/jetty/jetty-context.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-ee11/jetty-ee11-maven-plugin/src/it/jetty-cdi-start-forked/src/main/jetty/jetty-context.xml b/jetty-ee11/jetty-ee11-maven-plugin/src/it/jetty-cdi-start-forked/src/main/jetty/jetty-context.xml index 78aea23ca197..9194671bf36a 100644 --- a/jetty-ee11/jetty-ee11-maven-plugin/src/it/jetty-cdi-start-forked/src/main/jetty/jetty-context.xml +++ b/jetty-ee11/jetty-ee11-maven-plugin/src/it/jetty-cdi-start-forked/src/main/jetty/jetty-context.xml @@ -4,7 +4,7 @@ - + From 1e95caa1efeb1ede499d33e3b6f4b3310cced884 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 23 May 2024 14:49:00 +1000 Subject: [PATCH 5/5] Implement Servlet 6.1 Error dispatch changes (#11821) Implement Servlet 6.1 Error dispatch changes + Error dispatches are done as GET methods + New ERROR_METHOD and ERROR_QUERY_STRING attributes + Moved most error attributes to Dispatcher.ErrorRequest + removed duplicate attributes * updates from review * Update jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java Co-authored-by: Jan Bartel --- .../jetty/ee11/servlet/Dispatcher.java | 43 +++++++++++-- .../jetty/ee11/servlet/ErrorHandler.java | 3 +- .../ee11/servlet/ErrorPageErrorHandler.java | 4 +- .../jetty/ee11/servlet/ServletApiRequest.java | 21 ++++--- .../jetty/ee11/servlet/ServletChannel.java | 7 ++- .../ee11/servlet/ServletChannelState.java | 8 +-- .../jetty/ee11/servlet/ErrorPageTest.java | 60 ++++++++++++++++++- 7 files changed, 118 insertions(+), 28 deletions(-) diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/Dispatcher.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/Dispatcher.java index f533f551479e..0f735327b75f 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/Dispatcher.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/Dispatcher.java @@ -39,9 +39,11 @@ import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpServletResponseWrapper; import org.eclipse.jetty.ee11.servlet.util.ServletOutputStreamWrapper; +import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.pathmap.MatchedResource; import org.eclipse.jetty.io.WriterOutputStream; +import org.eclipse.jetty.server.handler.ErrorHandler; import org.eclipse.jetty.util.Fields; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.StringUtil; @@ -765,9 +767,12 @@ public Enumeration getAttributeNames() private class ErrorRequest extends ParameterRequestWrapper { - public ErrorRequest(HttpServletRequest httpRequest) + private final HttpServletRequest _httpServletRequest; + + public ErrorRequest(HttpServletRequest httpServletRequest) { - super(httpRequest); + super(httpServletRequest); + _httpServletRequest = Objects.requireNonNull(httpServletRequest); } @Override @@ -776,16 +781,22 @@ public DispatcherType getDispatcherType() return DispatcherType.ERROR; } + @Override + public String getMethod() + { + return HttpMethod.GET.asString(); + } + @Override public String getPathInfo() { - return _servletPathMapping.getPathInfo(); + return Objects.requireNonNull(_servletPathMapping).getPathInfo(); } @Override public String getServletPath() { - return _servletPathMapping.getServletPath(); + return Objects.requireNonNull(_servletPathMapping).getServletPath(); } @Override @@ -797,7 +808,7 @@ public HttpServletMapping getHttpServletMapping() @Override public String getRequestURI() { - return _uri.getPath(); + return Objects.requireNonNull(_uri).getPath(); } @Override @@ -809,6 +820,28 @@ public StringBuffer getRequestURL() .port(getServerPort()) .asString()); } + + @Override + public Object getAttribute(String name) + { + return switch (name) + { + case ERROR_METHOD -> _httpServletRequest.getMethod(); + case ERROR_REQUEST_URI -> _httpServletRequest.getRequestURI(); + case ERROR_QUERY_STRING -> _httpServletRequest.getQueryString(); + case ERROR_STATUS_CODE -> super.getAttribute(ErrorHandler.ERROR_STATUS); + case ERROR_MESSAGE -> super.getAttribute(ErrorHandler.ERROR_MESSAGE); + default -> super.getAttribute(name); + }; + } + + @Override + public Enumeration getAttributeNames() + { + List names = new ArrayList<>(List.of(ERROR_METHOD, ERROR_REQUEST_URI, ERROR_QUERY_STRING, ERROR_STATUS_CODE, ERROR_MESSAGE)); + names.addAll(Collections.list(super.getAttributeNames())); + return Collections.enumeration(names); + } } @Override diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java index e7952da7fb9f..1fc353f87db0 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java @@ -50,9 +50,8 @@ public class ErrorHandler implements Request.Handler { - // TODO This classes API needs to be majorly refactored/cleanup in jetty-10 + // TODO This class's API needs to be majorly refactored/cleanup private static final Logger LOG = LoggerFactory.getLogger(ErrorHandler.class); - public static final String ERROR_CONTEXT = "org.eclipse.jetty.server.error_context"; public static final String ERROR_CHARSET = "org.eclipse.jetty.server.error_charset"; boolean _showServlet = true; diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorPageErrorHandler.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorPageErrorHandler.java index 4b243f1f0c5f..05a4b2e8e46b 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorPageErrorHandler.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorPageErrorHandler.java @@ -23,6 +23,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.eclipse.jetty.server.handler.ErrorHandler.ERROR_STATUS; + /** * An ErrorHandler that maps exceptions and status codes to URIs for dispatch using * the internal ERROR style of dispatch. @@ -108,7 +110,7 @@ public String getErrorPage(HttpServletRequest request) pageSource = PageLookupTechnique.STATUS_CODE; // look for an exact code match - errorStatusCode = (Integer)request.getAttribute(Dispatcher.ERROR_STATUS_CODE); + errorStatusCode = (Integer)request.getAttribute(ERROR_STATUS); if (errorStatusCode != null) { errorPage = _errorPages.get(Integer.toString(errorStatusCode)); diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java index 13da1c37e297..7651b7a49513 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java @@ -843,19 +843,22 @@ public Object getAttribute(String name) @Override public Enumeration getAttributeNames() { - Set set = getRequest().getAttributeNameSet(); if (_async != null) { - set = new HashSet<>(set); - set.add(AsyncContext.ASYNC_REQUEST_URI); - set.add(AsyncContext.ASYNC_CONTEXT_PATH); - set.add(AsyncContext.ASYNC_SERVLET_PATH); - set.add(AsyncContext.ASYNC_PATH_INFO); - set.add(AsyncContext.ASYNC_QUERY_STRING); - set.add(AsyncContext.ASYNC_MAPPING); + Set names = new HashSet<>(Set.of( + + AsyncContext.ASYNC_REQUEST_URI, + AsyncContext.ASYNC_CONTEXT_PATH, + AsyncContext.ASYNC_SERVLET_PATH, + AsyncContext.ASYNC_PATH_INFO, + AsyncContext.ASYNC_QUERY_STRING, + AsyncContext.ASYNC_MAPPING + )); + names.addAll(getRequest().getAttributeNameSet()); + return Collections.enumeration(names); } - return Collections.enumeration(set); + return Collections.enumeration(getRequest().getAttributeNameSet()); } @Override diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletChannel.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletChannel.java index 303153e8e018..e36dbfe6f0a3 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletChannel.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletChannel.java @@ -44,6 +44,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.eclipse.jetty.server.handler.ErrorHandler.ERROR_CONTEXT; import static org.eclipse.jetty.util.thread.Invocable.InvocationType.NON_BLOCKING; /** @@ -459,7 +460,7 @@ public void handle() // the following is needed as you cannot trust the response code and reason // as those could have been modified after calling sendError - Integer code = (Integer)_servletContextRequest.getAttribute(RequestDispatcher.ERROR_STATUS_CODE); + Integer code = (Integer)_servletContextRequest.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_STATUS); if (code == null) code = HttpStatus.INTERNAL_SERVER_ERROR_500; getServletContextResponse().setStatus(code); @@ -472,7 +473,7 @@ public void handle() if (!_httpInput.consumeAvailable()) ResponseUtils.ensureNotPersistent(_servletContextRequest, _servletContextRequest.getServletContextResponse()); - ContextHandler.ScopedContext context = (ContextHandler.ScopedContext)_servletContextRequest.getAttribute(ErrorHandler.ERROR_CONTEXT); + ContextHandler.ScopedContext context = (ContextHandler.ScopedContext)_servletContextRequest.getAttribute(ERROR_CONTEXT); Request.Handler errorHandler = ErrorHandler.getErrorHandler(getServer(), context == null ? null : context.getContextHandler()); // If we can't have a body or have no ErrorHandler, then create a minimal error response. @@ -527,7 +528,7 @@ public void handle() finally { // clean up the context that was set in Response.sendError - _servletContextRequest.removeAttribute(ErrorHandler.ERROR_CONTEXT); + _servletContextRequest.removeAttribute(ERROR_CONTEXT); } break; } diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletChannelState.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletChannelState.java index 06cffc62f520..e9b7f0aa9b02 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletChannelState.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletChannelState.java @@ -36,10 +36,7 @@ import static jakarta.servlet.RequestDispatcher.ERROR_EXCEPTION; import static jakarta.servlet.RequestDispatcher.ERROR_EXCEPTION_TYPE; -import static jakarta.servlet.RequestDispatcher.ERROR_MESSAGE; -import static jakarta.servlet.RequestDispatcher.ERROR_REQUEST_URI; import static jakarta.servlet.RequestDispatcher.ERROR_SERVLET_NAME; -import static jakarta.servlet.RequestDispatcher.ERROR_STATUS_CODE; /** * holder of the state of request-response cycle. @@ -1039,11 +1036,8 @@ public void sendError(int code, String message) response.setStatus(code); servletContextRequest.errorClose(); - request.setAttribute(org.eclipse.jetty.ee11.servlet.ErrorHandler.ERROR_CONTEXT, servletContextRequest.getErrorContext()); - request.setAttribute(ERROR_REQUEST_URI, httpServletRequest.getRequestURI()); request.setAttribute(ERROR_SERVLET_NAME, servletContextRequest.getServletName()); - request.setAttribute(ERROR_STATUS_CODE, code); - request.setAttribute(ERROR_MESSAGE, message); + // Additional servlet error attributes are provided in org.eclipse.jetty.ee11.servlet.Dispatcher.ErrorRequest // Set Jetty Specific Attributes. request.setAttribute(ErrorHandler.ERROR_CONTEXT, servletContextRequest.getServletContext()); diff --git a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ErrorPageTest.java b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ErrorPageTest.java index 8ba9e8586b0a..85303aa1e844 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ErrorPageTest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ErrorPageTest.java @@ -153,6 +153,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t assertThat(body, containsString("ERROR_EXCEPTION: null")); assertThat(body, containsString("ERROR_EXCEPTION_TYPE: null")); assertThat(body, containsString("ERROR_SERVLET: " + errorContentServlet.getClass().getName())); + assertThat(body, containsString("ERROR_METHOD: GET")); assertThat(body, containsString("ERROR_REQUEST_URI: /error-mime-charset-writer/")); } @@ -734,6 +735,61 @@ protected void doGet(HttpServletRequest req, HttpServletResponse response) throw assertThat(responseBody, Matchers.containsString("getParameterMap()[ ]=[]")); } + @Test + public void testErrorAttributes() throws Exception + { + ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS); + contextHandler.setContextPath("/"); + + HttpServlet failServlet = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse response) throws IOException + { + response.sendError(599); + } + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + doGet(req, resp); + } + }; + + contextHandler.addServlet(failServlet, "/fail/599"); + contextHandler.addServlet(ErrorDumpServlet.class, "/error/*"); + + ErrorPageErrorHandler errorPageErrorHandler = new ErrorPageErrorHandler(); + errorPageErrorHandler.addErrorPage(599, "/error/599"); + contextHandler.setErrorHandler(errorPageErrorHandler); + + startServer(contextHandler); + + String rawRequest = """ + POST /fail/599?name=value HTTP/1.1\r + Host: test\r + Connection: close\r + \r + """; + + String rawResponse = _connector.getResponse(rawRequest); + + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(599)); + assertThat(response.get(HttpHeader.DATE), notNullValue()); + + String responseBody = response.getContent(); + + assertThat(responseBody, Matchers.containsString("ERROR_PAGE: /599")); + assertThat(responseBody, Matchers.containsString("ERROR_CODE: 599")); + assertThat(responseBody, Matchers.containsString("ERROR_EXCEPTION: null")); + assertThat(responseBody, Matchers.containsString("ERROR_EXCEPTION_TYPE: null")); + assertThat(responseBody, Matchers.containsString("ERROR_SERVLET: " + failServlet.getClass().getName())); + assertThat(responseBody, Matchers.containsString("ERROR_METHOD: POST")); + assertThat(responseBody, Matchers.containsString("ERROR_REQUEST_URI: /fail/599")); + assertThat(responseBody, Matchers.containsString("ERROR_QUERY_STRING: name=value")); + } + @Test public void testErrorCode() throws Exception { @@ -1733,7 +1789,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) public static class ErrorDumpServlet extends HttpServlet { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { if (request.getDispatcherType() != DispatcherType.ERROR && request.getDispatcherType() != DispatcherType.ASYNC) throw new IllegalStateException("Bad Dispatcher Type " + request.getDispatcherType()); @@ -1746,7 +1802,9 @@ protected void service(HttpServletRequest request, HttpServletResponse response) writer.println("ERROR_EXCEPTION: " + request.getAttribute(Dispatcher.ERROR_EXCEPTION)); writer.println("ERROR_EXCEPTION_TYPE: " + request.getAttribute(Dispatcher.ERROR_EXCEPTION_TYPE)); writer.println("ERROR_SERVLET: " + request.getAttribute(Dispatcher.ERROR_SERVLET_NAME)); + writer.println("ERROR_METHOD: " + request.getAttribute(Dispatcher.ERROR_METHOD)); writer.println("ERROR_REQUEST_URI: " + request.getAttribute(Dispatcher.ERROR_REQUEST_URI)); + writer.println("ERROR_QUERY_STRING: " + request.getAttribute(Dispatcher.ERROR_QUERY_STRING)); writer.printf("getRequestURI()=%s%n", valueOf(request.getRequestURI())); writer.printf("getRequestURL()=%s%n", valueOf(request.getRequestURL()));