Skip to content

Commit

Permalink
Check UriCompliance.Violation.ALLOW_BAD_UTF8 in EE9 query parameter e…
Browse files Browse the repository at this point in the history
…xtraction.
  • Loading branch information
gregw committed Jan 5, 2025
1 parent 818bd0e commit c1d9dbd
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.BiConsumer;
import java.util.function.Supplier;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -223,7 +224,7 @@ public static void decodeTo(String content, BiConsumer<String, String> adder, Ch

if (StandardCharsets.UTF_8.equals(charset))
{
decodeUtf8To(content, 0, content.length(), adder);
decodeUtf8To(content, 0, content.length(), adder, false);
return;
}

Expand Down Expand Up @@ -318,7 +319,7 @@ public static void decodeUtf8To(String query, Fields fields)
@Deprecated
public static void decodeUtf8To(String query, int offset, int length, MultiMap<String> map)
{
decodeUtf8To(query, offset, length, map::add);
decodeUtf8To(query, offset, length, map::add, false);
}

/**
Expand All @@ -331,28 +332,29 @@ public static void decodeUtf8To(String query, int offset, int length, MultiMap<S
*/
public static void decodeUtf8To(String uri, int offset, int length, Fields fields)
{
decodeUtf8To(uri, offset, length, fields::add);
decodeUtf8To(uri, offset, length, fields::add, false);
}

private static void decodeUtf8To(String query, int offset, int length, BiConsumer<String, String> adder)
public static void decodeUtf8To(String query, int offset, int length, BiConsumer<String, String> adder, boolean allowBadUtf8)
{
Utf8StringBuilder buffer = new Utf8StringBuilder();
String key = null;
String value;

Supplier<Utf8StringBuilder.Utf8IllegalArgumentException> onCodingError = allowBadUtf8 ? null : Utf8StringBuilder.Utf8IllegalArgumentException::new;
int end = offset + length;
for (int i = offset; i < end; i++)
{
char c = query.charAt(i);
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, "");
}
Expand All @@ -365,7 +367,7 @@ else if (value != null && value.length() > 0)
buffer.append(c);
break;
}
key = buffer.takeCompleteString(Utf8StringBuilder.Utf8IllegalArgumentException::new);
key = buffer.takeCompleteString(onCodingError);
break;

case '+':
Expand All @@ -379,6 +381,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();
Expand All @@ -393,7 +400,7 @@ 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -421,7 +422,16 @@ 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))
{
boolean allowBadUtf8 = getHttpChannel().getHttpConfiguration().getUriCompliance().allows(UriCompliance.Violation.BAD_UTF8_ENCODING);
UrlEncoded.decodeUtf8To(query, 0, query.length(), _queryParameters::add, allowBadUtf8);
}
else
{
UrlEncoded.decodeTo(query, _queryParameters::add, _queryEncoding);
}
}
catch (IllegalStateException | IllegalArgumentException e)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down

0 comments on commit c1d9dbd

Please sign in to comment.