This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 7bacd86a631f858a6ff3c83f08fb5a2a6367735d Author: opencode <[email protected]> AuthorDate: Mon Oct 12 11:54:11 2026 +0200 Do not apply partial ParameterLimitValve configuration when parsing fails setUrlPatternLimits() parsed the configuration line by line directly into the shared map of enforced limits. An invalid line, an invalid number of limits or a non-numeric limit propagated an exception after the lines parsed so far had already been applied, leaving the valve enforcing a partial, inconsistent limit set. The String overload made this worse by clearing the previously configured limits before parsing, so a failed runtime update through the management operation both lost the old configuration and enforced a partial one. An IO error mid-read was logged but likewise left partial state in effect. Parse into a temporary map and only merge it into the enforced limits once the complete input has been parsed successfully. On a parse exception the previously configured limits are now left untouched, and on an IO error the input read so far is discarded. The append semantics of the BufferedReader overload and the replace semantics of the String overload are unchanged. Also use the parameterLimitValve.invalidLimits message key that is actually defined in the bundles for the invalid limit count error, instead of the undefined parameterLimitValve.invalidLimitsString, so the message is no longer rendered as a missing-resource placeholder. --- .../catalina/valves/ParameterLimitValve.java | 36 +++++++++++++++++---- .../catalina/valves/TestParameterLimitValve.java | 37 ++++++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/java/org/apache/catalina/valves/ParameterLimitValve.java b/java/org/apache/catalina/valves/ParameterLimitValve.java index b0f2748bae..2c54f2a6f8 100644 --- a/java/org/apache/catalina/valves/ParameterLimitValve.java +++ b/java/org/apache/catalina/valves/ParameterLimitValve.java @@ -187,18 +187,25 @@ public class ParameterLimitValve extends ValveBase { /** * Set the mapping of URL patterns to their corresponding parameter limits from a string containing the - * configuration. + * configuration. The new limits replace any previously configured limits. The limits are only applied if the + * complete configuration is parsed successfully. If parsing fails, the exception is propagated and any previously + * configured limits are left unchanged. * * @param urlPatternConfig The URL pattern to parameter limit mappings */ public void setUrlPatternLimits(String urlPatternConfig) { + Map<Pattern,Integer[]> newLimits = + parseUrlPatternLimits(new BufferedReader(new StringReader(urlPatternConfig))); urlPatternLimits.clear(); - setUrlPatternLimits(new BufferedReader(new StringReader(urlPatternConfig))); + urlPatternLimits.putAll(newLimits); } /** * Set the mapping of URL patterns to their corresponding parameter limits. The input should be provided line by - * line, where each line contains a pattern and a limit, separated by the last '='. + * line, where each line contains a pattern and a limit, separated by the last '='. The parsed limits are only + * applied if the complete input is parsed successfully. If parsing fails, the exception is propagated and any + * previously configured limits are left unchanged. If a read error occurs, it is logged and the input read so far + * is discarded without modifying the configured limits. * <p> * Example: * @@ -213,9 +220,23 @@ public class ParameterLimitValve extends ValveBase { * line. */ public void setUrlPatternLimits(BufferedReader reader) { + urlPatternLimits.putAll(parseUrlPatternLimits(reader)); + } + + /** + * Parse URL pattern to parameter limit mappings without modifying the limits that are currently in effect. This + * avoids enforcing a partial, inconsistent limit set when parsing fails part way through. + * + * @param reader A BufferedReader containing URL pattern to parameter limit mappings, with each pair on a separate + * line. + * + * @return The parsed mappings + */ + private Map<Pattern,Integer[]> parseUrlPatternLimits(BufferedReader reader) { if (containerLog == null && getContainer() != null) { containerLog = LogFactory.getLog(getContainer().getLogName() + ".parameterLimit"); } + Map<Pattern,Integer[]> newLimits = new ConcurrentHashMap<>(); try { String line; while ((line = reader.readLine()) != null) { @@ -237,13 +258,13 @@ public class ParameterLimitValve extends ValveBase { Pattern pattern = Pattern.compile(UDecoder.URLDecode(patternString, StandardCharsets.UTF_8)); String[] limits = limitsString.split(","); if (limits.length == 1) { - urlPatternLimits.put(pattern, new Integer[] { Integer.valueOf(limits[0]), null, null }); + newLimits.put(pattern, new Integer[] { Integer.valueOf(limits[0]), null, null }); } else if (limits.length == 3) { - urlPatternLimits.put(pattern, new Integer[] { Integer.valueOf(limits[0]), + newLimits.put(pattern, new Integer[] { Integer.valueOf(limits[0]), Integer.valueOf(limits[1]), Integer.valueOf(limits[2]) }); } else { throw new IllegalArgumentException( - sm.getString("parameterLimitValve.invalidLimitsString", limitsString)); + sm.getString("parameterLimitValve.invalidLimits", limitsString)); } if (containerLog != null && containerLog.isTraceEnabled()) { containerLog.trace("Add pattern " + pattern + " and limit(s) " + limitsString); @@ -253,7 +274,10 @@ public class ParameterLimitValve extends ValveBase { if (containerLog != null) { containerLog.error(sm.getString("parameterLimitValve.readError"), ioe); } + // Do not apply the limits parsed before the error + return Map.of(); } + return newLimits; } @Override diff --git a/test/org/apache/catalina/valves/TestParameterLimitValve.java b/test/org/apache/catalina/valves/TestParameterLimitValve.java index 5f5aab6eeb..5c34d94cb5 100644 --- a/test/org/apache/catalina/valves/TestParameterLimitValve.java +++ b/test/org/apache/catalina/valves/TestParameterLimitValve.java @@ -592,6 +592,43 @@ public class TestParameterLimitValve extends TomcatBaseTest { } + @Test + public void testInvalidConfigLeavesLimitsUnchanged() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + StandardContext ctx = (StandardContext) getProgrammaticRootContext(); + + ParameterLimitValve parameterLimitValve = new ParameterLimitValve(); + ctx.getPipeline().addValve(parameterLimitValve); + parameterLimitValve.setUrlPatternLimits("/special/.*=2"); + + // An invalid configuration update must fail and must leave the + // previously configured limits in effect. Previously, the limits were + // cleared first and the lines parsed before the failure remained. + try { + parameterLimitValve.setUrlPatternLimits( + "/other/.*=5" + CRLF + "invalid-line-without-equals"); + Assert.fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException expected) { + // Expected + } + + Tomcat.addServlet(ctx, "snoop", new SnoopServlet()); + ctx.addServletMapping("/special/endpoint", "snoop"); + + tomcat.start(); + + int rc = getUrl("http://localhost:"; + getPort() + + "/special/endpoint?param1=value1¶m2=value2¶m3=value3", + new ByteChunk(), null); + Assert.assertEquals(400, rc); + + rc = getUrl("http://localhost:"; + getPort() + "/special/endpoint?param1=value1¶m2=value2", + new ByteChunk(), null); + Assert.assertEquals(200, rc); + } + + private void doTestMaxParameterCountLimitExceeded(int maxParameterCount, int textPartCount, int filePartCount, int queryStringCount, boolean okExpected) throws Exception { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
