This is an automated email from the ASF dual-hosted git repository. joerghoh pushed a commit to branch SLING-13365 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-engine.git
commit 18e2f69da6afed8e5d4d292b0f96063e4e035886 Author: Joerg Hoh <[email protected]> AuthorDate: Fri Sep 25 13:15:47 2026 +0200 SLING-13365 fail by default when providing more than 10'000 parameters --- .../engine/impl/SlingRequestProcessorImpl.java | 8 ++ .../sling/engine/impl/parameters/ParameterMap.java | 2 +- .../impl/parameters/ParameterParseException.java | 41 +++++++ .../RequestParameterSupportConfigurer.java | 7 +- .../engine/impl/SlingRequestProcessorImplTest.java | 127 +++++++++++++++++++++ .../engine/impl/parameters/ParameterMapTest.java | 4 +- 6 files changed, 183 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java b/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java index 27451b6..cd6bb96 100644 --- a/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java +++ b/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java @@ -66,6 +66,7 @@ import org.apache.sling.engine.impl.filter.ServletFilterManager; import org.apache.sling.engine.impl.filter.ServletFilterManager.FilterChainType; import org.apache.sling.engine.impl.filter.SlingComponentFilterChain; import org.apache.sling.engine.impl.helper.SlingServletContext; +import org.apache.sling.engine.impl.parameters.ParameterParseException; import org.apache.sling.engine.impl.parameters.ParameterSupport; import org.apache.sling.engine.impl.request.ContentData; import org.apache.sling.engine.impl.request.DispatchingInfo; @@ -311,6 +312,13 @@ public class SlingRequestProcessorImpl implements SlingRequestProcessor { log.debug("service: Resource {} not found", rnfe.getResource()); handleError(HttpServletResponse.SC_NOT_FOUND, rnfe.getMessage(), request, response); + } catch (final ParameterParseException ppe) { + // the request parameters could not be processed (e.g. too many + // parameters); the request itself is malformed/excessive, not the + // server, so send this exception as a 400 status + log.debug("service: Failed processing request parameters", ppe); + handleError(HttpServletResponse.SC_BAD_REQUEST, ppe.getMessage(), request, response); + } catch (final SlingException se) { // send this exception as is (albeit unwrapping and wrapped // exception. 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 884fd5b..611456f 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 @@ -78,7 +78,7 @@ public class ParameterMap extends LinkedHashMap<String, RequestParameter[]> impl // check number of parameters if (maxParameters > -1 && this.requestParameters.size() >= maxParameters) { if (failOnParameterLimit) { - throw new IllegalStateException("Too many name/value pairs, limit is " + maxParameters); + throw new ParameterParseException("Too many name/value pairs, limit is " + maxParameters); } LoggerFactory.getLogger(Util.class) .warn("Too many name/value pairs, stopped processing after " + maxParameters + " entries"); diff --git a/src/main/java/org/apache/sling/engine/impl/parameters/ParameterParseException.java b/src/main/java/org/apache/sling/engine/impl/parameters/ParameterParseException.java new file mode 100644 index 0000000..f47ae7e --- /dev/null +++ b/src/main/java/org/apache/sling/engine/impl/parameters/ParameterParseException.java @@ -0,0 +1,41 @@ +/* + * 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; + +/** + * Unchecked exception indicating that the request parameters could not be + * processed as-is, e.g. because the request exceeds the configured maximum + * number of parameters. This is a client-caused, request-fatal condition: the + * request must be refused (mapped to an HTTP 400 response by + * {@code SlingRequestProcessorImpl}) rather than processed with a silently + * truncated parameter map, which would let an attacker hide parameters from + * Sling-side consumers while other parsers of the same request (edge WAFs, + * the container) still see them. + * <p> + * Extends {@link IllegalStateException} to remain compatible with callers + * that only checked for that (more generic) exception type. + */ +public class ParameterParseException extends IllegalStateException { + + private static final long serialVersionUID = 1L; + + public ParameterParseException(final String message) { + super(message); + } +} 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 7421b46..f1e6f20 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 @@ -132,9 +132,10 @@ public class RequestParameterSupportConfigurer implements Filter { @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; + + "If true (default), an exception is thrown and the request is rejected with a 400 Bad " + + "Request response. If false, a warning is logged and processing continues with the " + + "parameter map silently truncated after the configured limit.") + boolean sling_default_parameter_fail_on_limit() default true; } static final String PID = "org.apache.sling.engine.parameters"; diff --git a/src/test/java/org/apache/sling/engine/impl/SlingRequestProcessorImplTest.java b/src/test/java/org/apache/sling/engine/impl/SlingRequestProcessorImplTest.java new file mode 100644 index 0000000..b07f3cc --- /dev/null +++ b/src/test/java/org/apache/sling/engine/impl/SlingRequestProcessorImplTest.java @@ -0,0 +1,127 @@ +/* + * 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; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.lang.reflect.Field; + +import jakarta.servlet.Servlet; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.apache.sling.api.SlingJakartaHttpServletRequest; +import org.apache.sling.api.resource.Resource; +import org.apache.sling.api.resource.ResourceMetadata; +import org.apache.sling.api.resource.ResourceResolver; +import org.apache.sling.api.servlets.ServletResolver; +import org.apache.sling.engine.impl.filter.FilterHandle; +import org.apache.sling.engine.impl.filter.ServletFilterManager; +import org.apache.sling.engine.impl.filter.ServletFilterManager.FilterChainType; +import org.apache.sling.engine.impl.parameters.ParameterParseException; +import org.jetbrains.annotations.NotNull; +import org.junit.Before; +import org.junit.Test; + +import static jakarta.servlet.http.HttpServletResponse.SC_BAD_REQUEST; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Tests for {@link SlingRequestProcessorImpl}, in particular the + * {@code ParameterParseException} to HTTP 400 mapping performed in + * {@code doProcessRequest} (regression test for SLING-13138/f018: a + * request refused for exceeding the configured parameter limit must + * surface as a 400 Bad Request, not an uncaught 500). + */ +public class SlingRequestProcessorImplTest { + + private SlingRequestProcessorImpl processor; + private ServletFilterManager filterManager; + + @Before + public void setup() throws Exception { + processor = new SlingRequestProcessorImpl(); + + filterManager = mock(ServletFilterManager.class); + when(filterManager.getFilters(FilterChainType.ERROR)).thenReturn(new FilterHandle[0]); + setField(processor, "filterManager", filterManager); + } + + private static void setField(final Object target, final String name, final Object value) throws Exception { + Field field = target.getClass().getDeclaredField(name); + field.setAccessible(true); + field.set(target, value); + } + + @Test + public void testDoProcessRequestMapsParameterParseExceptionToBadRequest() throws Exception { + final Servlet servlet = mock(Servlet.class); + doThrow(new ParameterParseException("Too many name/value pairs, limit is 10000")) + .when(servlet) + .service(any(ServletRequest.class), any(ServletResponse.class)); + + final Resource resource = getMockedResource("/content/test"); + + final ResourceResolver resourceResolver = mock(ResourceResolver.class); + when(resourceResolver.resolve(any(HttpServletRequest.class), anyString())) + .thenReturn(resource); + + final ServletResolver servletResolver = mock(ServletResolver.class); + when(servletResolver.resolve(any(SlingJakartaHttpServletRequest.class))).thenReturn(servlet); + setField(processor, "servletResolver", servletResolver); + + // no request/component filters: the chain falls straight through to + // the resolved servlet, whose service() call raises the exception + when(filterManager.getFilters(FilterChainType.REQUEST)).thenReturn(new FilterHandle[0]); + when(filterManager.getFilters(FilterChainType.COMPONENT)).thenReturn(new FilterHandle[0]); + + final HttpServletRequest httpServletRequest = mock(HttpServletRequest.class); + when(httpServletRequest.getRequestURI()).thenReturn("/content/test"); + when(httpServletRequest.getRequestURL()).thenReturn(new StringBuffer("http://localhost/content/test";)); + when(httpServletRequest.getContextPath()).thenReturn(""); + when(httpServletRequest.getServletPath()).thenReturn(""); + when(httpServletRequest.getMethod()).thenReturn("GET"); + + final HttpServletResponse httpServletResponse = mock(HttpServletResponse.class); + final StringWriter writer = new StringWriter(); + when(httpServletResponse.getWriter()).thenReturn(new PrintWriter(writer)); + + processor.doProcessRequest(httpServletRequest, httpServletResponse, resourceResolver); + + verify(httpServletResponse).setStatus(SC_BAD_REQUEST); + writer.flush(); + assertTrue(writer.toString().contains("Too many name/value pairs")); + } + + private static @NotNull Resource getMockedResource(final @NotNull String path) { + final Resource resource = mock(Resource.class); + when(resource.getPath()).thenReturn(path); + final ResourceMetadata resourceMetadata = mock(ResourceMetadata.class); + when(resource.getResourceMetadata()).thenReturn(resourceMetadata); + when(resourceMetadata.getResolutionPathInfo()).thenReturn(path); + return resource; + } +} 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 index a89e279..0387db7 100644 --- a/src/test/java/org/apache/sling/engine/impl/parameters/ParameterMapTest.java +++ b/src/test/java/org/apache/sling/engine/impl/parameters/ParameterMapTest.java @@ -76,7 +76,7 @@ public class ParameterMapTest { assertEquals(2, pm.size()); // Should throw exception when exceeding limit - exception.expect(IllegalStateException.class); + exception.expect(ParameterParseException.class); exception.expectMessage("Too many name/value pairs"); exception.expectMessage("2"); pm.addParameter(createTestParameter("param3", "value3"), false); @@ -123,7 +123,7 @@ public class ParameterMapTest { assertEquals(5, pm.size()); // Next should fail - exception.expect(IllegalStateException.class); + exception.expect(ParameterParseException.class); exception.expectMessage("Too many name/value pairs"); exception.expectMessage("5"); pm.addParameter(createTestParameter("param6", "value6"), false);
