diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 1250165a1a78..30247aeaaf45 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -20,6 +20,7 @@ import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.nio.charset.IllegalCharsetNameException; +import java.nio.charset.StandardCharsets; import java.nio.charset.UnsupportedCharsetException; import java.nio.file.Path; import java.security.Principal; @@ -46,6 +47,7 @@ import org.eclipse.jetty.http.MultiPartCompliance; import org.eclipse.jetty.http.MultiPartConfig; import org.eclipse.jetty.http.Trailers; +import org.eclipse.jetty.http.UriCompliance; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.server.internal.CompletionStreamWrapper; @@ -552,21 +554,31 @@ static Charset getCharset(Request request) throws IllegalCharsetNameException, U static Fields extractQueryParameters(Request request) { - String query = request.getHttpURI().getQuery(); - if (StringUtil.isBlank(query)) - return Fields.EMPTY; - Fields fields = new Fields(true); - if (StringUtil.isNotBlank(query)) - UrlEncoded.decodeUtf8To(query, fields); - return fields; + return extractQueryParameters(request, null); } static Fields extractQueryParameters(Request request, Charset charset) { - Fields fields = new Fields(true); String query = request.getHttpURI().getQuery(); - if (StringUtil.isNotBlank(query)) + if (StringUtil.isBlank(query)) + return Fields.EMPTY; + Fields fields = new Fields(true); + + if (charset == null || StandardCharsets.UTF_8.equals(charset)) + { + UriCompliance uriCompliance = request.getConnectionMetaData().getHttpConfiguration().getUriCompliance(); + boolean allowBadUtf8 = uriCompliance.allows(UriCompliance.Violation.BAD_UTF8_ENCODING); + if (!UrlEncoded.decodeUtf8To(query, 0, query.length(), fields::add, allowBadUtf8)) + { + HttpChannel httpChannel = HttpChannel.from(request); + if (httpChannel != null && httpChannel.getComplianceViolationListener() != null) + httpChannel.getComplianceViolationListener().onComplianceViolation(new ComplianceViolation.Event(uriCompliance, UriCompliance.Violation.BAD_UTF8_ENCODING, "query=" + query)); + } + } + else + { UrlEncoded.decodeTo(query, fields::add, charset); + } return fields; } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 03a8e2763b50..95c265e5c99c 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -19,9 +19,11 @@ import java.util.EnumSet; import java.util.List; import java.util.Locale; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; import java.util.stream.Stream; +import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpHeader; @@ -33,6 +35,7 @@ import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.DumpHandler; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.Fields; import org.eclipse.jetty.util.component.LifeCycle; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -40,6 +43,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; @@ -49,6 +53,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; public class RequestTest { @@ -541,4 +546,75 @@ private static void checkCookieResult(String containedCookie, String[] notContai } } } + + /** + * Test to ensure that response.write() will add Content-Length on HTTP/1.1 responses. + */ + @ParameterizedTest + @ValueSource(strings = { "true", "false"}) + public void testBadUtf8Query(boolean allowBadUtf8) throws Exception + { + server.stop(); + List events = new CopyOnWriteArrayList<>(); + ComplianceViolation.Listener listener = new ComplianceViolation.Listener() + { + @Override + public void onComplianceViolation(ComplianceViolation.Event event) + { + events.add(event); + } + }; + + if (allowBadUtf8) + { + for (Connector connector : server.getConnectors()) + { + HttpConnectionFactory httpConnectionFactory = connector.getConnectionFactory(HttpConnectionFactory.class); + if (httpConnectionFactory != null) + { + HttpConfiguration httpConfiguration = httpConnectionFactory.getHttpConfiguration(); + httpConfiguration.setUriCompliance(UriCompliance.DEFAULT.with("test", UriCompliance.Violation.BAD_UTF8_ENCODING)); + httpConfiguration.addComplianceViolationListener(listener); + } + } + } + + server.setHandler(new Handler.Abstract.NonBlocking() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + if (allowBadUtf8) + { + Fields fields = Request.extractQueryParameters(request); + assertThat(fields.getValue("param"), is("bad_�")); + assertThat(fields.getValue("other"), is("short�")); + } + else + { + assertThrows(IllegalArgumentException.class, () -> Request.extractQueryParameters(request)); + } + callback.succeeded(); + return true; + } + }); + server.start(); + + String request = """ + GET /foo?param=bad_%e0%b8&other=short%a HTTP/1.1\r + Host: local\r + \r + """; + HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(request)); + assertEquals(HttpStatus.OK_200, response.getStatus()); + if (allowBadUtf8) + { + assertThat(events.size(), is(1)); + assertThat(events.get(0).violation(), is(UriCompliance.Violation.BAD_UTF8_ENCODING)); + } + else + { + assertThat(events.size(), is(0)); + } + } } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java index 0164a4673bd5..5d2df9bef405 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java @@ -22,8 +22,10 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiConsumer; +import java.util.function.Supplier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -223,7 +225,7 @@ public static void decodeTo(String content, BiConsumer adder, Ch if (StandardCharsets.UTF_8.equals(charset)) { - decodeUtf8To(content, 0, content.length(), adder); + decodeUtf8To(content, 0, content.length(), adder, false); return; } @@ -318,7 +320,7 @@ public static void decodeUtf8To(String query, Fields fields) @Deprecated public static void decodeUtf8To(String query, int offset, int length, MultiMap map) { - decodeUtf8To(query, offset, length, map::add); + decodeUtf8To(query, offset, length, map::add, false); } /** @@ -331,15 +333,45 @@ public static void decodeUtf8To(String query, int offset, int length, MultiMap adder) + /** + *

Decodes URI query parameters as UTF8 string

+ * + * @param query the URI string. + * @param offset the offset at which query parameters start. + * @param length the length of query parameters string to parse. + * @param adder the method to call to add decoded parameters. + * @param allowBadUtf8 if {@code true} allow bad UTF-8 and insert the replacement character. + * @return {@code true} if the string was decoded without any bad UTF-8 + * @throws org.eclipse.jetty.util.Utf8StringBuilder.Utf8IllegalArgumentException if there is illegal UTF-8 and `allowsBadUtf8` is {@code false} + */ + public static boolean decodeUtf8To(String query, int offset, int length, BiConsumer adder, boolean allowBadUtf8) + throws Utf8StringBuilder.Utf8IllegalArgumentException { Utf8StringBuilder buffer = new Utf8StringBuilder(); String key = null; String value; + AtomicBoolean badUtf8; + Supplier onCodingError; + + if (allowBadUtf8) + { + badUtf8 = new AtomicBoolean(false); + onCodingError = () -> + { + badUtf8.set(true); + return null; + }; + } + else + { + badUtf8 = null; + onCodingError = Utf8StringBuilder.Utf8IllegalArgumentException::new; + } + int end = offset + length; for (int i = offset; i < end; i++) { @@ -347,12 +379,12 @@ private static void decodeUtf8To(String query, int offset, int length, BiConsume switch (c) { case '&': - value = buffer.takeCompleteString(Utf8StringBuilder.Utf8IllegalArgumentException::new); + value = buffer.takeCompleteString(onCodingError); if (key != null) { adder.accept(key, value); } - else if (value != null && value.length() > 0) + else if (value != null && !value.isEmpty()) { adder.accept(value, ""); } @@ -365,7 +397,7 @@ else if (value != null && value.length() > 0) buffer.append(c); break; } - key = buffer.takeCompleteString(Utf8StringBuilder.Utf8IllegalArgumentException::new); + key = buffer.takeCompleteString(onCodingError); break; case '+': @@ -379,6 +411,11 @@ else if (value != null && value.length() > 0) char lo = query.charAt(++i); buffer.append(decodeHexByte(hi, lo)); } + else if (allowBadUtf8) + { + buffer.append(Utf8StringBuilder.REPLACEMENT); + i = end; + } else { throw new Utf8StringBuilder.Utf8IllegalArgumentException(); @@ -393,13 +430,15 @@ else if (value != null && value.length() > 0) if (key != null) { - value = buffer.takeCompleteString(Utf8StringBuilder.Utf8IllegalArgumentException::new); + value = buffer.takeCompleteString(onCodingError); adder.accept(key, value); } else if (buffer.length() > 0) { adder.accept(buffer.toCompleteString(), ""); } + + return badUtf8 == null || !badUtf8.get(); } /** diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java index c9560b6b4e76..fef96b697dd2 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java @@ -77,6 +77,7 @@ import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.MultiPartCompliance; import org.eclipse.jetty.http.SetCookieParser; +import org.eclipse.jetty.http.UriCompliance; import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.security.UserIdentity; @@ -421,7 +422,22 @@ private void extractQueryParameters() try { _queryParameters = new Fields(true); - UrlEncoded.decodeTo(query, _queryParameters::add, _queryEncoding); + + if (StandardCharsets.UTF_8.equals(_queryEncoding) || _queryEncoding == null && UrlEncoded.ENCODING.equals(StandardCharsets.UTF_8)) + { + UriCompliance uriCompliance = getHttpChannel().getHttpConfiguration().getUriCompliance(); + boolean allowBadUtf8 = uriCompliance.allows(UriCompliance.Violation.BAD_UTF8_ENCODING); + if (!UrlEncoded.decodeUtf8To(query, 0, query.length(), _queryParameters::add, allowBadUtf8)) + { + ComplianceViolation.Listener complianceViolationListener = getComplianceViolationListener(); + if (complianceViolationListener != null) + complianceViolationListener.onComplianceViolation(new ComplianceViolation.Event(uriCompliance, UriCompliance.Violation.BAD_UTF8_ENCODING, "query=" + query)); + } + } + else + { + UrlEncoded.decodeTo(query, _queryParameters::add, _queryEncoding); + } } catch (IllegalStateException | IllegalArgumentException e) { diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java index f274322b939d..fe560a2ff6c5 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java +++ b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java @@ -212,6 +212,34 @@ public void testRequestCharacterEncoding() throws Exception assertEquals("utf-8", result.get()); } + @Test + public void testBadUtf8Query() throws Exception + { + _server.stop(); + _connector.getConnectionFactory(HttpConnectionFactory.class) + .getHttpConfiguration().setUriCompliance(UriCompliance.DEFAULT.with("test", UriCompliance.Violation.BAD_UTF8_ENCODING)); + _server.start(); + + _handler._checker = (request, response) -> + { + String param = request.getParameter("param"); + String other = request.getParameter("other"); + return param != null && param.equals("bad_�") && other != null && other.equals("short�"); + }; + + //Send a request with query string with illegal hex code to cause + //an exception parsing the params + String request = """ + GET /?param=bad_%e0%b8&other=short%a HTTP/1.1\r + Host: whatever\r + Connection: close + + """; + + String responses = _connector.getResponse(request); + assertThat(responses, startsWith("HTTP/1.1 200")); + } + @Test public void testParamExtraction() throws Exception {