diff --git a/src/main/java/org/apache/sling/engine/impl/parameters/ParameterMap.java b/src/main/java/org/apache/sling/engine/impl/parameters/ParameterMap.java index 7f886c07..884fd5bb 100644 --- a/src/main/java/org/apache/sling/engine/impl/parameters/ParameterMap.java +++ b/src/main/java/org/apache/sling/engine/impl/parameters/ParameterMap.java @@ -40,6 +40,7 @@ public class ParameterMap extends LinkedHashMap impl static final int DEFAULT_MAX_PARAMS = 10000; private static int maxParameters = DEFAULT_MAX_PARAMS; + private static boolean failOnParameterLimit = false; private Map stringParameterMap; @@ -49,6 +50,10 @@ static void setMaxParameters(final int maxParameters) { ParameterMap.maxParameters = (maxParameters > 0) ? maxParameters : -1; } + static void setFailOnParameterLimit(final boolean fail) { + ParameterMap.failOnParameterLimit = fail; + } + public RequestParameter getValue(String name) { RequestParameter[] params = getValues(name); return (params != null && params.length > 0) ? params[0] : null; @@ -71,8 +76,10 @@ void renameParameter(String oldName, String newName) { void addParameter(RequestParameter parameter, boolean prependNew) { // check number of parameters - if (this.requestParameters.size() == maxParameters) { - // TODO: how to handle this situation ?? just ignore or throw or what ?? + if (maxParameters > -1 && this.requestParameters.size() >= maxParameters) { + if (failOnParameterLimit) { + throw new IllegalStateException("Too many name/value pairs, limit is " + maxParameters); + } LoggerFactory.getLogger(Util.class) .warn("Too many name/value pairs, stopped processing after " + maxParameters + " entries"); return; diff --git a/src/main/java/org/apache/sling/engine/impl/parameters/RequestParameterSupportConfigurer.java b/src/main/java/org/apache/sling/engine/impl/parameters/RequestParameterSupportConfigurer.java index 78586a36..7421b46e 100644 --- a/src/main/java/org/apache/sling/engine/impl/parameters/RequestParameterSupportConfigurer.java +++ b/src/main/java/org/apache/sling/engine/impl/parameters/RequestParameterSupportConfigurer.java @@ -128,6 +128,13 @@ public void destroy() {} description = "The maximum number of files allowed for multipart/form-data requests in a single request. The default is 50.") long request_max_file_count() default 50; + + @AttributeDefinition( + name = "Fail on Parameter Limit", + description = "Whether to throw an exception when the maximum number of parameters is exceeded. " + + "If false (default), a warning is logged and processing continues with truncated parameters. " + + "If true, an IllegalStateException is thrown.") + boolean sling_default_parameter_fail_on_limit() default false; } static final String PID = "org.apache.sling.engine.parameters"; @@ -147,6 +154,7 @@ private void configure(final Config config) { final long maxFileSize = config.file_max(); final int fileSizeThreshold = config.file_threshold(); final boolean checkAddParameters = config.sling_default_parameter_checkForAdditionalContainerParameters(); + final boolean failOnParameterLimit = config.sling_default_parameter_fail_on_limit(); if (log.isInfoEnabled()) { log.info("Default Character Encoding: {}", fixEncoding); @@ -157,10 +165,12 @@ private void configure(final Config config) { log.info("Tempory File Creation Threshold: {}", fileSizeThreshold); log.info("Check for additional container parameters: {}", checkAddParameters); log.info("Maximum File Count: {}", config.request_max_file_count()); + log.info("Fail on Parameter Limit: {}", failOnParameterLimit); } Util.setDefaultFixEncoding(fixEncoding); ParameterMap.setMaxParameters(maxParams); + ParameterMap.setFailOnParameterLimit(failOnParameterLimit); ParameterSupport.configure( maxRequestSize, fileLocation, diff --git a/src/test/java/org/apache/sling/engine/impl/parameters/ParameterMapTest.java b/src/test/java/org/apache/sling/engine/impl/parameters/ParameterMapTest.java new file mode 100644 index 00000000..a89e2799 --- /dev/null +++ b/src/test/java/org/apache/sling/engine/impl/parameters/ParameterMapTest.java @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sling.engine.impl.parameters; + +import org.apache.sling.api.request.RequestParameter; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import static org.junit.Assert.assertEquals; + +public class ParameterMapTest { + + private static final int ORIGINAL_MAX_PARAMS = ParameterMap.DEFAULT_MAX_PARAMS; + + @Rule + public ExpectedException exception = ExpectedException.none(); + + private void resetToDefaults() { + ParameterMap.setMaxParameters(ORIGINAL_MAX_PARAMS); + ParameterMap.setFailOnParameterLimit(false); + } + + @Before + public void setUp() { + resetToDefaults(); + } + + @After + public void tearDown() { + resetToDefaults(); + } + + @Test + public void testDefaultBehavior() { + ParameterMap pm = new ParameterMap(); + ParameterMap.setMaxParameters(2); + + // Should work normally within limit + pm.addParameter(createTestParameter("param1", "value1"), false); + pm.addParameter(createTestParameter("param2", "value2"), false); + assertEquals(2, pm.size()); + + // Should log warning and continue when exceeding limit + pm.addParameter(createTestParameter("param3", "value3"), false); + assertEquals(2, pm.size()); // Should still be 2, param3 ignored + } + + @Test + public void testFailOnParameterLimit() { + ParameterMap pm = new ParameterMap(); + ParameterMap.setMaxParameters(2); + ParameterMap.setFailOnParameterLimit(true); + + // Should work normally within limit + pm.addParameter(createTestParameter("param1", "value1"), false); + pm.addParameter(createTestParameter("param2", "value2"), false); + assertEquals(2, pm.size()); + + // Should throw exception when exceeding limit + exception.expect(IllegalStateException.class); + exception.expectMessage("Too many name/value pairs"); + exception.expectMessage("2"); + pm.addParameter(createTestParameter("param3", "value3"), false); + } + + @Test + public void testParameterLimitExactlyAtBoundary() { + ParameterMap pm = new ParameterMap(); + ParameterMap.setMaxParameters(1); + ParameterMap.setFailOnParameterLimit(false); + + // Add exactly at limit + pm.addParameter(createTestParameter("param1", "value1"), false); + assertEquals(1, pm.size()); + + // Next addition should trigger warning and be ignored + pm.addParameter(createTestParameter("param2", "value2"), false); + assertEquals(1, pm.size()); // Should remain 1 + } + + @Test + public void testUnlimitedParameters() { + ParameterMap pm = new ParameterMap(); + ParameterMap.setMaxParameters(-1); // Unlimited + ParameterMap.setFailOnParameterLimit(false); + + // Should allow unlimited parameters + for (int i = 1; i <= 100; i++) { + pm.addParameter(createTestParameter("param" + i, "value" + i), false); + } + assertEquals(100, pm.size()); + } + + @Test + public void testFailOnLimitWithLargeLimit() { + ParameterMap pm = new ParameterMap(); + ParameterMap.setMaxParameters(5); + ParameterMap.setFailOnParameterLimit(true); + + // Add up to limit + for (int i = 1; i <= 5; i++) { + pm.addParameter(createTestParameter("param" + i, "value" + i), false); + } + assertEquals(5, pm.size()); + + // Next should fail + exception.expect(IllegalStateException.class); + exception.expectMessage("Too many name/value pairs"); + exception.expectMessage("5"); + pm.addParameter(createTestParameter("param6", "value6"), false); + } + + @Test + public void testFailOnLimitDisabledWithZeroLimit() { + ParameterMap pm = new ParameterMap(); + ParameterMap.setMaxParameters(0); // Becomes -1 (unlimited) + ParameterMap.setFailOnParameterLimit(true); // Shouldn't matter since unlimited + + // Should allow parameters despite failOnLimit=true + pm.addParameter(createTestParameter("param1", "value1"), false); + assertEquals(1, pm.size()); + } + + private RequestParameter createTestParameter(String name, String value) { + return new ContainerRequestParameter(name, value, "UTF-8"); + } +}