This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 99d1d5672584e858740eab2b3093f0c9fe598246 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 | 34 ++++++- .../apache/catalina/core/LocalStrings.properties | 2 + .../core/TestApplicationServletRegistration.java | 107 ++++++++++++++++++++- 3 files changed, 137 insertions(+), 6 deletions(-) diff --git a/java/org/apache/catalina/core/ApplicationServletRegistration.java b/java/org/apache/catalina/core/ApplicationServletRegistration.java index 56fe2b5d2b..c4448b8f5f 100644 --- a/java/org/apache/catalina/core/ApplicationServletRegistration.java +++ b/java/org/apache/catalina/core/ApplicationServletRegistration.java @@ -93,6 +93,7 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam throw new IllegalArgumentException( sm.getString("applicationServletRegistration.nullInitParam", name, value)); } + checkState(); if (getInitParameter(name) != null) { return false; } @@ -117,6 +118,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()) { @@ -130,6 +133,7 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam @Override public void setAsyncSupported(boolean asyncSupported) { + checkState(); wrapper.setAsyncSupported(asyncSupported); } @@ -167,17 +171,23 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam @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())); + } + + for (String urlPattern : urlPatterns) { + if (urlPattern == null || urlPattern.isEmpty()) { + throw new IllegalArgumentException(sm.getString("applicationServletRegistration.nullUrlPattern")); + } } + checkState(); + Set<String> conflicts = new HashSet<>(); Set<String> overrides = new HashSet<>(); for (int i = 0; i < urlPatterns.length; i++) { - if (urlPatterns[i] == null) { - throw new IllegalArgumentException(sm.getString("applicationServletRegistration.nullUrlPattern")); - } String wrapperName = context.findServletMapping(urlPatterns[i]); if (wrapperName != null) { Wrapper wrapper = (Wrapper) context.findChild(wrapperName); @@ -233,4 +243,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 a86586e737..3d075b011e 100644 --- a/java/org/apache/catalina/core/LocalStrings.properties +++ b/java/org/apache/catalina/core/LocalStrings.properties @@ -73,6 +73,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 c7362b899a..11902b0df7 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 testUrlPatternsAreTreatedAsUrlDecoded() { @@ -36,4 +43,102 @@ public class TestApplicationServletRegistration { // Ensure pattern has not been decoded Assert.assertEquals("servlet", context.findServletMapping("/servlet%25")); } + + + @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]
