This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 5027181e5213d059de2299a50b27438b59aa7d52 Author: opencode <[email protected]> AuthorDate: Wed Sep 30 11:08:50 2026 +0200 Add spec-mandated validation to ApplicationServletRegistration: throw IllegalStateException when the registration is modified after the context has been initialised (using a deliberately lenient availability test to avoid breaking existing applications that configure registrations before start) and IllegalArgumentException from addMapping for null or empty URL patterns --- .../core/ApplicationServletRegistration.java | 25 ++++- .../apache/catalina/core/LocalStrings.properties | 2 + .../core/TestApplicationServletRegistration.java | 107 ++++++++++++++++++++- 3 files changed, 131 insertions(+), 3 deletions(-) diff --git a/java/org/apache/catalina/core/ApplicationServletRegistration.java b/java/org/apache/catalina/core/ApplicationServletRegistration.java index 981ce6f50e..f5bd7ebe72 100644 --- a/java/org/apache/catalina/core/ApplicationServletRegistration.java +++ b/java/org/apache/catalina/core/ApplicationServletRegistration.java @@ -95,6 +95,7 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam throw new IllegalArgumentException( sm.getString("applicationServletRegistration.nullInitParam", name, value)); } + checkState(); if (getInitParameter(name) != null) { return false; } @@ -119,6 +120,8 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam } } + checkState(); + // Have to add in a separate loop since spec requires no updates at all // if there is an issue if (conflicts.isEmpty()) { @@ -132,6 +135,7 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam @Override public void setAsyncSupported(boolean asyncSupported) { + checkState(); wrapper.setAsyncSupported(asyncSupported); } @@ -170,8 +174,9 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam @SuppressWarnings("deprecation") @Override public Set<String> addMapping(String... urlPatterns) { - if (urlPatterns == null) { - return Collections.emptySet(); + if (urlPatterns == null || urlPatterns.length == 0) { + throw new IllegalArgumentException( + sm.getString("applicationServletRegistration.nullUrlPatterns", getName(), context.getName())); } String[] decodedUrlPatterns = new String[urlPatterns.length]; @@ -186,6 +191,8 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam } } + checkState(); + Set<String> conflicts = new HashSet<>(); Set<String> overrides = new HashSet<>(); @@ -246,4 +253,18 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam return wrapper.getRunAs(); } + + private void checkState() { + // The specification requires an IllegalStateException if the + // ServletContext has already been initialised. A stricter check + // (only allowing calls during STARTING_PREP, as used by + // setServletSecurity()) risks breaking existing applications that + // configure registrations before the context starts, so the looser + // test below only rejects calls made once the context is available. + if (context.getState().isAvailable()) { + throw new IllegalStateException(sm.getString("applicationServletRegistration.ise", getName(), + context.getName())); + } + } + } diff --git a/java/org/apache/catalina/core/LocalStrings.properties b/java/org/apache/catalina/core/LocalStrings.properties index 4b09e016b5..803c1a0716 100644 --- a/java/org/apache/catalina/core/LocalStrings.properties +++ b/java/org/apache/catalina/core/LocalStrings.properties @@ -76,6 +76,8 @@ applicationServletRegistration.nullInitParams=Unable to set initialisation param applicationServletRegistration.nullUrlPattern=URL patterns must not be null or empty applicationServletRegistration.setServletSecurity.iae=Null constraint specified for servlet [{0}] deployed to context with name [{1}] applicationServletRegistration.setServletSecurity.ise=Security constraints can''t be added to servlet [{0}] deployed to context with name [{1}] as the context has already been initialised +applicationServletRegistration.ise=Servlet [{0}] deployed to context with name [{1}] can''t be modified after the context has been initialised +applicationServletRegistration.nullUrlPatterns=Null or empty URL patterns specified for servlet [{0}] deployed to context with name [{1}] applicationSessionCookieConfig.ise=Property [{0}] cannot be added to SessionCookieConfig for context [{1}] as the context has been initialised diff --git a/test/org/apache/catalina/core/TestApplicationServletRegistration.java b/test/org/apache/catalina/core/TestApplicationServletRegistration.java index 4452a0baf9..8cb90c3257 100644 --- a/test/org/apache/catalina/core/TestApplicationServletRegistration.java +++ b/test/org/apache/catalina/core/TestApplicationServletRegistration.java @@ -16,12 +16,19 @@ */ package org.apache.catalina.core; +import jakarta.servlet.ServletRegistration; + import org.junit.Assert; import org.junit.Test; +import org.apache.catalina.Context; +import org.apache.catalina.LifecycleState; import org.apache.catalina.Wrapper; +import org.apache.catalina.startup.TesterServlet; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; -public class TestApplicationServletRegistration { +public class TestApplicationServletRegistration extends TomcatBaseTest { @Test public void testUrlPatternEncoded() { @@ -45,4 +52,102 @@ public class TestApplicationServletRegistration { Assert.assertTrue(registration.addMapping("/servlet%25").isEmpty()); Assert.assertEquals("servlet", context.findServletMapping(expectedPattern)); } + + + @Test + public void testAddMappingNullAndEmptyPatterns() { + StandardContext context = new StandardContext(); + + Wrapper wrapper = context.createWrapper(); + wrapper.setName("servlet"); + context.addChild(wrapper); + + ApplicationServletRegistration registration = new ApplicationServletRegistration(wrapper, context); + + try { + registration.addMapping((String[]) null); + Assert.fail("Expected an IllegalArgumentException for null patterns"); + } catch (IllegalArgumentException e) { + // Expected + } + + try { + registration.addMapping(); + Assert.fail("Expected an IllegalArgumentException for an empty pattern array"); + } catch (IllegalArgumentException e) { + // Expected + } + + try { + registration.addMapping(""); + Assert.fail("Expected an IllegalArgumentException for an empty pattern"); + } catch (IllegalArgumentException e) { + // Expected + } + } + + + @Test + public void testAddMappingWhileContextAvailable() { + CustomContext context = new CustomContext(); + context.setState(LifecycleState.NEW); + + Wrapper wrapper = context.createWrapper(); + wrapper.setName("servlet"); + context.addChild(wrapper); + + context.setState(LifecycleState.STARTED); + + ApplicationServletRegistration registration = new ApplicationServletRegistration(wrapper, context); + + try { + registration.addMapping("/servlet"); + Assert.fail("Expected an IllegalStateException once the context is available"); + } catch (IllegalStateException e) { + // Expected + } + } + + + @Test + public void testModificationAfterContextInitialised() throws Exception { + Tomcat tomcat = getTomcatInstance(); + Context root = getProgrammaticRootContext(); + Tomcat.addServlet(root, "servlet", new TesterServlet()); + root.addServletMapping("/test", "servlet"); + + tomcat.start(); + + ServletRegistration registration = + root.getServletContext().getServletRegistration("servlet"); + + try { + registration.setInitParameter("param", "value"); + Assert.fail("Expected an IllegalStateException after initialisation"); + } catch (IllegalStateException e) { + // Expected + } + + try { + ((ServletRegistration.Dynamic) registration).addMapping("/other"); + Assert.fail("Expected an IllegalStateException after initialisation"); + } catch (IllegalStateException e) { + // Expected + } + } + + + private static class CustomContext extends StandardContext { + private volatile LifecycleState state; + + @Override + public LifecycleState getState() { + return state; + } + + @Override + public synchronized void setState(LifecycleState state) { + this.state = state; + } + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
