This is an automated email from the ASF dual-hosted git repository.
joerghoh pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-engine.git
The following commit(s) were added to refs/heads/master by this push:
new 7b6c7b5 SLING-13362 fix parameter parsing (#94)
7b6c7b5 is described below
commit 7b6c7b56c606a5e8aabbef3f0c1c561409315ee6
Author: Jörg Hoh <[email protected]>
AuthorDate: Thu Sep 24 17:36:46 2026 +0200
SLING-13362 fix parameter parsing (#94)
---
.../engine/impl/parameters/ParameterSupport.java | 24 ++--
.../apache/sling/engine/impl/parameters/Util.java | 52 ++++++--
.../impl/parameters/ParameterSupportTest.java | 140 +++++++++++++++++++++
.../sling/engine/impl/parameters/UtilTest.java | 45 +++++++
4 files changed, 240 insertions(+), 21 deletions(-)
diff --git
a/src/main/java/org/apache/sling/engine/impl/parameters/ParameterSupport.java
b/src/main/java/org/apache/sling/engine/impl/parameters/ParameterSupport.java
index 967b9ff..98bed3c 100644
---
a/src/main/java/org/apache/sling/engine/impl/parameters/ParameterSupport.java
+++
b/src/main/java/org/apache/sling/engine/impl/parameters/ParameterSupport.java
@@ -265,7 +265,11 @@ public class ParameterSupport {
if (query != null) {
try {
InputStream input = Util.toInputStream(query);
- Util.parseQueryString(input, encoding, parameters, false);
+ // the query string is always decoded with the
byte-preserving
+ // ISO-8859-1 encoding and later fixed up via the
'_charset_'
+ // parameter or the configured default encoding (see
+ // Util.fixEncoding).
+ Util.parseQueryString(input, Util.ENCODING_DIRECT,
parameters, false);
addContainerParameters = checkForAdditionalParameters;
} catch (IllegalArgumentException e) {
this.log.error("getRequestParameterMapInternal: Error
parsing request", e);
@@ -379,15 +383,17 @@ public class ParameterSupport {
return false;
}
- // This check assumes the content type ends after the WWW_FORM_URL_ENC
- // or continues with blank or semicolon. It will probably break if
- // the content type is some string extension of WWW_FORM_URL_ENC
- // such as "application/x-www-form-urlencoded-bla"
- if
(contentType.toLowerCase(Locale.ENGLISH).startsWith(WWW_FORM_URL_ENC)) {
- return true;
+ // only the media type is relevant, optional parameters (such as
+ // charset) may follow separated by a semicolon. An exact match is
+ // required: a prefix match would treat unrelated media types such as
+ // "application/x-www-form-urlencoded-bla" as form encoded content and
+ // parse a request body which container level inspection does not
+ // consider to carry parameters
+ final int semi = contentType.indexOf(';');
+ if (semi >= 0) {
+ contentType = contentType.substring(0, semi);
}
-
- return false;
+ return
WWW_FORM_URL_ENC.equals(contentType.trim().toLowerCase(Locale.ENGLISH));
}
private RequestContext getMultiPartContext() {
diff --git a/src/main/java/org/apache/sling/engine/impl/parameters/Util.java
b/src/main/java/org/apache/sling/engine/impl/parameters/Util.java
index 4734ab8..03d0852 100644
--- a/src/main/java/org/apache/sling/engine/impl/parameters/Util.java
+++ b/src/main/java/org/apache/sling/engine/impl/parameters/Util.java
@@ -281,12 +281,7 @@ public class Util {
case ESC_NAME:
chCode[subState++] = ch;
if (subState == chCode.length) {
- String code = new String(chCode);
- try {
- keyBuffer.write(Integer.parseInt(code, 16));
- } catch (NumberFormatException e) {
- throw new IllegalArgumentException("Bad escape
sequence: %" + code);
- }
+ keyBuffer.write(decodePercentEscape(chCode));
state = INSIDE_NAME;
}
break;
@@ -328,12 +323,7 @@ public class Util {
case ESC_VALUE:
chCode[subState++] = ch;
if (subState == chCode.length) {
- String code = new String(chCode);
- try {
- valueBuffer.write(Integer.parseInt(code, 16));
- } catch (NumberFormatException e) {
- throw new IllegalArgumentException("Bad escape
sequence: %" + code);
- }
+ valueBuffer.write(decodePercentEscape(chCode));
state = INSIDE_VALUE;
}
break;
@@ -345,11 +335,49 @@ public class Util {
}
}
+ // a truncated escape sequence at the end of the input is malformed;
+ // reject it instead of silently dropping it, so that the resulting
+ // parameter values never differ from a standards compliant decoder
+ if (state == ESC_NAME || state == ESC_VALUE) {
+ throw new IllegalArgumentException("Bad escape sequence:
unexpected end of input after '%'");
+ }
+
if (keyBuffer.size() > 0) {
addNVPair(map, keyBuffer, valueBuffer, encoding, prependNew);
}
}
+ /**
+ * Decodes a two character percent escape sequence, accepting only ASCII
+ * hexadecimal digits as mandated by RFC 3986.
+ *
+ * @param chCode the two escape characters following '%'. Callers must
+ * guarantee that this array has a length of exactly 2
+ * @return the decoded byte value (0..255)
+ * @throws IllegalArgumentException if a character is not a hex digit
+ */
+ private static int decodePercentEscape(final char[] chCode) {
+ final int hi = hexDigit(chCode[0]);
+ final int lo = hexDigit(chCode[1]);
+ if (hi < 0 || lo < 0) {
+ throw new IllegalArgumentException("Bad escape sequence: %" + new
String(chCode));
+ }
+ return (hi << 4) + lo;
+ }
+
+ private static int hexDigit(final char c) {
+ if (c >= '0' && c <= '9') {
+ return c - '0';
+ }
+ if (c >= 'a' && c <= 'f') {
+ return c - 'a' + 10;
+ }
+ if (c >= 'A' && c <= 'F') {
+ return c - 'A' + 10;
+ }
+ return -1;
+ }
+
private static void addNVPair(
ParameterMap map,
ByteArrayOutputStream keyBuffer,
diff --git
a/src/test/java/org/apache/sling/engine/impl/parameters/ParameterSupportTest.java
b/src/test/java/org/apache/sling/engine/impl/parameters/ParameterSupportTest.java
new file mode 100644
index 0000000..3a7b241
--- /dev/null
+++
b/src/test/java/org/apache/sling/engine/impl/parameters/ParameterSupportTest.java
@@ -0,0 +1,140 @@
+/*
+ * 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 java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.util.Collections;
+
+import jakarta.servlet.ReadListener;
+import jakarta.servlet.ServletInputStream;
+import jakarta.servlet.http.HttpServletRequest;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Regression tests for SLING-13362 (f010): the engine's request parameter
+ * decoding must not diverge from a standards compliant (container/WAF)
+ * decoder, otherwise a client can smuggle data past inspection that the
+ * engine still acts on.
+ */
+public class ParameterSupportTest {
+
+ @Test
+ public void
testQueryStringDecodingIgnoresClientControlledCharacterEncoding() {
+ // the client claims (via Content-Type) that the request uses UTF-16BE.
+ // Per HTTP this only applies to the request body, not the query
+ // string. If the engine honored it for the query string anyway, the
+ // two raw bytes 0x00 0x41 would be read as a single UTF-16BE
+ // character ('A'), rather than as two ISO-8859-1 bytes/characters -
+ // exactly the kind of divergence a container or WAF (which decodes
+ // the query string as its own configured/default charset) would not
+ // see.
+ final HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getMethod()).thenReturn("GET");
+ when(request.getCharacterEncoding()).thenReturn("UTF-16BE");
+ when(request.getQueryString()).thenReturn("a=%00A");
+
when(request.getAttribute(ParameterSupport.MARKER_IS_SERVICE_PROCESSING))
+ .thenReturn(null);
+ when(request.getParameterMap()).thenReturn(Collections.emptyMap());
+
+ final ParameterSupport parameterSupport =
ParameterSupport.getInstance(request);
+
+ assertEquals(
+ "query string must always be decoded byte-for-byte as
ISO-8859-1, "
+ + "never with the client supplied (body) character
encoding",
+ "\u0000A",
+ parameterSupport.getParameter("a"));
+ }
+
+ @Test
+ public void
testWwwFormEncodedContentTypeExactMatchIsParsedAsFormParameters() throws
Exception {
+ final HttpServletRequest request =
postRequest("application/x-www-form-urlencoded", "a=b");
+
+ final ParameterSupport parameterSupport =
ParameterSupport.getInstance(request);
+
+ assertEquals("b", parameterSupport.getParameter("a"));
+ }
+
+ @Test
+ public void
testWwwFormEncodedContentTypeWithCharsetParameterIsParsedAsFormParameters()
throws IOException {
+ final HttpServletRequest request =
postRequest("application/x-www-form-urlencoded; charset=UTF-8", "a=b");
+
+ final ParameterSupport parameterSupport =
ParameterSupport.getInstance(request);
+
+ assertEquals("b", parameterSupport.getParameter("a"));
+ }
+
+ @Test
+ public void
testContentTypeThatOnlyStartsWithFormEncodedMediaTypeIsNotParsedAsFormParameters()
throws IOException {
+ // a container/perimeter doing an exact media type match considers
+ // this request to carry no parameters at all; if the engine parsed
+ // the body anyway (prefix match) it would act on data invisible to
+ // that inspection.
+ final HttpServletRequest request =
postRequest("application/x-www-form-urlencoded-bla", "a=b");
+
+ final ParameterSupport parameterSupport =
ParameterSupport.getInstance(request);
+
+ assertNull(parameterSupport.getParameter("a"));
+ }
+
+ private static HttpServletRequest postRequest(final String contentType,
final String body) throws IOException {
+ final HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getMethod()).thenReturn("POST");
+ when(request.getCharacterEncoding()).thenReturn("UTF-8");
+ when(request.getContentType()).thenReturn(contentType);
+ when(request.getContentLength()).thenReturn(body.length());
+ when(request.getQueryString()).thenReturn(null);
+
when(request.getAttribute(ParameterSupport.MARKER_IS_SERVICE_PROCESSING))
+ .thenReturn(null);
+ when(request.getParameterMap()).thenReturn(Collections.emptyMap());
+ when(request.getInputStream()).thenReturn(toServletInputStream(body));
+ return request;
+ }
+
+ private static ServletInputStream toServletInputStream(final String
content) throws UnsupportedEncodingException {
+ final ByteArrayInputStream in = new
ByteArrayInputStream(content.getBytes(Util.ENCODING_DIRECT));
+ return new ServletInputStream() {
+ @Override
+ public boolean isFinished() {
+ return in.available() == 0;
+ }
+
+ @Override
+ public boolean isReady() {
+ return true;
+ }
+
+ @Override
+ public void setReadListener(ReadListener readListener) {
+ // not needed for these tests
+ }
+
+ @Override
+ public int read() {
+ return in.read();
+ }
+ };
+ }
+}
diff --git
a/src/test/java/org/apache/sling/engine/impl/parameters/UtilTest.java
b/src/test/java/org/apache/sling/engine/impl/parameters/UtilTest.java
index d10cda2..7bde849 100644
--- a/src/test/java/org/apache/sling/engine/impl/parameters/UtilTest.java
+++ b/src/test/java/org/apache/sling/engine/impl/parameters/UtilTest.java
@@ -127,4 +127,49 @@ public class UtilTest extends TestCase {
assertEquals("Some Page", map.getStringValue("title"));
assertEquals("/content/geometrixx", map.getStringValue("parentPath"));
}
+
+ public void test_decode_valid_escapes() throws Exception {
+ final ParameterMap map = new ParameterMap();
+ final String query = "a=%41%62c&b=x%2fy&c=%0d%0A";
+ Util.parseQueryString(
+ new
ByteArrayInputStream(query.getBytes(Util.ENCODING_DIRECT)),
Util.ENCODING_DIRECT, map, false);
+ assertEquals("Abc", map.getStringValue("a"));
+ assertEquals("x/y", map.getStringValue("b"));
+ assertEquals("\r\n", map.getStringValue("c"));
+ }
+
+ public void test_signed_escape_sequences_rejected() throws Exception {
+ // Integer.parseInt(s, 16) accepts a leading sign, a standards
compliant
+ // URL decoder does not: %+d/%+a would decode to CR/LF invisible to any
+ // RFC 3986 decoder (e.g. a WAF), %-1 to byte 0xFF
+ for (String query : new String[] {"r=x%+d%+ay", "r=x%-1", "%+d=x",
"r=%4+"}) {
+ try {
+ Util.parseQueryString(
+ new
ByteArrayInputStream(query.getBytes(Util.ENCODING_DIRECT)),
+ Util.ENCODING_DIRECT,
+ new ParameterMap(),
+ false);
+ fail("Expected IllegalArgumentException for query: " + query);
+ } catch (IllegalArgumentException expected) {
+ // expected: signed/non-hex escape sequences are malformed
+ }
+ }
+ }
+
+ public void test_trailing_incomplete_escape_rejected() throws Exception {
+ // a trailing "%2" was silently swallowed before, yielding value "b"
+ // for "a=b%2" - malformed input must be rejected, not truncated
+ for (String query : new String[] {"a=b%2", "a=b%", "a%2"}) {
+ try {
+ Util.parseQueryString(
+ new
ByteArrayInputStream(query.getBytes(Util.ENCODING_DIRECT)),
+ Util.ENCODING_DIRECT,
+ new ParameterMap(),
+ false);
+ fail("Expected IllegalArgumentException for query: " + query);
+ } catch (IllegalArgumentException expected) {
+ // expected: incomplete trailing escape sequence
+ }
+ }
+ }
}
