Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP [UNDERTOW-2312] multibytes language in URL request to http/https are … #1516

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

WIP [UNDERTOW-2312] multibytes language in URL request to http/https are … #1516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f409c1e
[UNDERTOW-2312] multibytes language in URL request to http/https are …
baranowb Sep 25, 2023
45d8df5
[UNDERTOW-2312] Add test for access log with unescaped characters. Al…
fl4via Oct 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/main/java/io/undertow/protocols/http2/HpackDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ private String readHpackString(ByteBuffer buffer) throws HpackException {
return readHuffmanString(length, buffer);
}
for (int i = 0; i < length; ++i) {
stringBuilder.append((char) buffer.get());
stringBuilder.append((char) (buffer.get() & 0xFF));
}
String ret = stringBuilder.toString();
stringBuilder.setLength(0);
Expand Down
41 changes: 34 additions & 7 deletions core/src/main/java/io/undertow/server/Connectors.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import io.undertow.UndertowMessages;
import io.undertow.UndertowOptions;
import io.undertow.server.handlers.Cookie;
import io.undertow.server.protocol.http.HttpRequestParser;
import io.undertow.util.BadRequestException;
import io.undertow.util.DateUtils;
import io.undertow.util.HeaderMap;
import io.undertow.util.HeaderValues;
Expand Down Expand Up @@ -446,7 +448,7 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
try {
final boolean slashDecodingFlag = URLUtils.getSlashDecodingFlag(allowEncodedSlash, exchange.getConnection().getUndertowOptions().get(UndertowOptions.DECODE_SLASH));
setExchangeRequestPath(exchange, encodedPath, charset, decode, slashDecodingFlag, decodeBuffer, exchange.getConnection().getUndertowOptions().get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS));
} catch (ParameterLimitException e) {
} catch (ParameterLimitException | BadRequestException e) {
throw new RuntimeException(e);
}
}
Expand All @@ -459,8 +461,9 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
* @param encodedPath The encoded path to decode
* @param decodeBuffer The decode buffer to use
* @throws ParameterLimitException
* @throws BadRequestException
*/
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, StringBuilder decodeBuffer) throws ParameterLimitException {
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, StringBuilder decodeBuffer) throws ParameterLimitException, BadRequestException {
final OptionMap options = exchange.getConnection().getUndertowOptions();
boolean slashDecodingFlag = URLUtils.getSlashDecodingFlag(options);
setExchangeRequestPath(exchange, encodedPath,
Expand All @@ -477,13 +480,19 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
* @param exchange The exchange
* @param encodedPath The encoded path
* @param charset The charset
* @throws BadRequestException
*/
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, final boolean decodeSlashFlag, StringBuilder decodeBuffer, int maxParameters) throws ParameterLimitException {
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, final boolean decodeSlashFlag, StringBuilder decodeBuffer, int maxParameters) throws ParameterLimitException, BadRequestException {
final OptionMap options = exchange.getConnection().getUndertowOptions();
final boolean allowUnescapedCharactersInUrl = options.get(UndertowOptions.ALLOW_UNESCAPED_CHARACTERS_IN_URL, false);
boolean requiresDecode = false;
final StringBuilder pathBuilder = new StringBuilder();
int currentPathPartIndex = 0;
for (int i = 0; i < encodedPath.length(); ++i) {
char c = encodedPath.charAt(i);
if(!allowUnescapedCharactersInUrl && !HttpRequestParser.isTargetCharacterAllowed(c)) {
throw new BadRequestException(UndertowMessages.MESSAGES.invalidCharacterInRequestTarget(c));
}
if (c == '?') {
String part;
String encodedPart = encodedPath.substring(currentPathPartIndex, i);
Expand All @@ -496,9 +505,21 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
part = pathBuilder.toString();
exchange.setRequestPath(part);
exchange.setRelativePath(part);
exchange.setRequestURI(encodedPath.substring(0, i));
if(requiresDecode && allowUnescapedCharactersInUrl) {
final String uri = URLUtils.decode(encodedPath.substring(0, i), charset, decodeSlashFlag,false, decodeBuffer);
exchange.setRequestURI(uri);
} else {
exchange.setRequestURI(encodedPath.substring(0, i));
}

final String qs = encodedPath.substring(i + 1);
exchange.setQueryString(qs);
if(requiresDecode && allowUnescapedCharactersInUrl) {
final String decodedQS = URLUtils.decode(qs, charset, decodeSlashFlag,false, decodeBuffer);
exchange.setQueryString(decodedQS);
} else {
exchange.setQueryString(qs);
}

URLUtils.parseQueryString(qs, exchange, charset, decode, maxParameters);
return;
} else if(c == ';') {
Expand All @@ -510,10 +531,16 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
part = encodedPart;
}
pathBuilder.append(part);
exchange.setRequestURI(encodedPath);
if(requiresDecode && allowUnescapedCharactersInUrl) {
final String uri = URLUtils.decode(encodedPath, charset, decodeSlashFlag,false, decodeBuffer);
exchange.setRequestURI(uri);
} else {
exchange.setRequestURI(encodedPath);
}

currentPathPartIndex = i + 1 + URLUtils.parsePathParams(encodedPath.substring(i + 1), exchange, charset, decode, maxParameters);
i = currentPathPartIndex -1 ;
} else if(c == '%' || c == '+') {
} else if(decode && (c == '+' || c == '%' || c > 127)) {
requiresDecode = decode;
}
}
Expand Down
19 changes: 13 additions & 6 deletions core/src/main/java/io/undertow/server/protocol/ajp/AjpRequestParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import io.undertow.util.HttpString;
import io.undertow.util.ParameterLimitException;
import io.undertow.util.URLUtils;
import io.undertow.util.UrlDecodeException;

/**
* @author Stuart Douglas
Expand Down Expand Up @@ -269,7 +270,7 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final
int colon = result.value.indexOf(';');
if (colon == -1) {
String res = decode(result.value, result.containsUrlCharacters);
if(result.containsUnencodedCharacters) {
if(result.containsUnencodedCharacters || (allowUnescapedCharactersInUrl && result.containsUrlCharacters)) {
//we decode if the URL was non-compliant, and contained incorrectly encoded characters
//there is not really a 'correct' thing to do in this situation, but this seems the least incorrect
exchange.setRequestURI(res);
Expand Down Expand Up @@ -447,8 +448,14 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final
state.state = AjpRequestParseState.READING_ATTRIBUTES;
return;
}
if(resultHolder.containsUnencodedCharacters) {
result = decode(resultHolder.value, true);
if(resultHolder.containsUnencodedCharacters || (resultHolder.containsUrlCharacters && allowUnescapedCharactersInUrl)) {
try {
result = decode(resultHolder.value, true);
} catch (UrlDecodeException | UnsupportedEncodingException e) {
UndertowLogger.REQUEST_IO_LOGGER.failedToParseRequest(e);
state.badRequest = true;
result = resultHolder.value;
}
decodingAlreadyDone = true;
} else {
result = resultHolder.value;
Expand Down Expand Up @@ -583,16 +590,16 @@ protected StringHolder parseString(ByteBuffer buf, AjpRequestParseState state, S
return new StringHolder(null, false, false, false);
}
byte c = buf.get();
if(type == StringType.QUERY_STRING && (c == '+' || c == '%' || c < 0 )) {
if (c < 0) {
if(type == StringType.QUERY_STRING && (c == '+' || c == '%' || c < 0 || c > 127 )) {
if (c < 0 || c > 127) {
if (!allowUnescapedCharactersInUrl) {
throw new BadRequestException();
} else {
containsUnencodedUrlCharacters = true;
}
}
containsUrlCharacters = true;
} else if(type == StringType.URL && (c == '%' || c < 0 )) {
} else if(type == StringType.URL && (c == '%' || c < 0 || c > 127 )) {
if(c < 0 ) {
if(!allowUnescapedCharactersInUrl) {
throw new BadRequestException();
Expand Down
35 changes: 24 additions & 11 deletions core/src/main/java/io/undertow/server/protocol/http/HttpRequestParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,10 +497,15 @@ private void parsePathComplete(ParseState state, HttpServerExchange exchange, in
exchange.setRelativePath("/");
exchange.setRequestURI(path, true);
} else if (parseState < HOST_DONE && state.canonicalPath.length() == 0) {
String decodedPath = decode(path, urlDecodeRequired, state, slashDecodingFlag, false);
exchange.setRequestPath(decodedPath);
exchange.setRelativePath(decodedPath);
exchange.setRequestURI(path, false);
final String decodedRequestPath = decode(path, urlDecodeRequired, state, slashDecodingFlag, false);
exchange.setRequestPath(decodedRequestPath);
exchange.setRelativePath(decodedRequestPath);
if(urlDecodeRequired && allowUnescapedCharactersInUrl) {
final String uri = decode(path, urlDecodeRequired, state, slashDecodingFlag, false);
exchange.setRequestURI(uri);
} else {
exchange.setRequestURI(path);
}
} else {
handleFullUrl(state, exchange, canonicalPathStart, urlDecodeRequired, path, parseState);
}
Expand All @@ -520,10 +525,15 @@ private void beginQueryParameters(ByteBuffer buffer, ParseState state, HttpServe

private void handleFullUrl(ParseState state, HttpServerExchange exchange, int canonicalPathStart, boolean urlDecodeRequired, String path, int parseState) {
state.canonicalPath.append(path.substring(canonicalPathStart));
String thePath = decode(state.canonicalPath.toString(), urlDecodeRequired, state, slashDecodingFlag, false);
exchange.setRequestPath(thePath);
exchange.setRelativePath(thePath);
exchange.setRequestURI(path, parseState == HOST_DONE);
final String requestPath = decode(state.canonicalPath.toString(), urlDecodeRequired, state, slashDecodingFlag, false);
exchange.setRequestPath(requestPath);
exchange.setRelativePath(requestPath);
if(urlDecodeRequired && allowUnescapedCharactersInUrl) {
final String uri = decode(path, urlDecodeRequired, state, slashDecodingFlag, false);
exchange.setRequestURI(uri, parseState == HOST_DONE);
} else {
exchange.setRequestURI(path, parseState == HOST_DONE);
}
}


Expand All @@ -537,7 +547,7 @@ private void handleFullUrl(ParseState state, HttpServerExchange exchange, int ca
*/
@SuppressWarnings("unused")
final void handleQueryParameters(ByteBuffer buffer, ParseState state, HttpServerExchange exchange) throws BadRequestException {
StringBuilder stringBuilder = state.stringBuilder;
final StringBuilder stringBuilder = state.stringBuilder;
int queryParamPos = state.pos;
int mapCount = state.mapCount;
boolean urlDecodeRequired = state.urlDecodeRequired;
Expand All @@ -551,12 +561,15 @@ final void handleQueryParameters(ByteBuffer buffer, ParseState state, HttpServer
//we encounter an encoded character

while (buffer.hasRemaining()) {
char next = (char) (buffer.get() & 0xFF);
final char next = (char) (buffer.get() & 0xFF);
if(!allowUnescapedCharactersInUrl && !ALLOWED_TARGET_CHARACTER[next]) {
throw new BadRequestException(UndertowMessages.MESSAGES.invalidCharacterInRequestTarget(next));
}
if (next == ' ' || next == '\t') {
final String queryString = stringBuilder.toString();
String queryString = stringBuilder.toString();
if(urlDecodeRequired && this.allowUnescapedCharactersInUrl) {
queryString = decode(queryString, urlDecodeRequired, state, slashDecodingFlag, false);
}
exchange.setQueryString(queryString);
if (nextQueryParam == null) {
if (queryParamPos != stringBuilder.length()) {
Expand Down
14 changes: 12 additions & 2 deletions core/src/main/java/io/undertow/server/protocol/http2/Http2ReceiveListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import io.undertow.server.protocol.http.HttpAttachments;
import io.undertow.server.protocol.http.HttpContinue;
import io.undertow.server.protocol.http.HttpRequestParser;
import io.undertow.util.BadRequestException;
import io.undertow.util.ConduitFactory;
import io.undertow.util.HeaderMap;
import io.undertow.util.HeaderValues;
Expand Down Expand Up @@ -193,7 +194,7 @@ public void handleEvent(Http2StreamSourceChannel channel) {

try {
Connectors.setExchangeRequestPath(exchange, path, encoding, decode, slashDecodingFlag, decodeBuffer, maxParameters);
} catch (ParameterLimitException e) {
} catch (ParameterLimitException | BadRequestException e) {
//this can happen if max parameters is exceeded
UndertowLogger.REQUEST_IO_LOGGER.debug("Failed to set request path", e);
exchange.setStatusCode(StatusCodes.BAD_REQUEST);
Expand All @@ -217,6 +218,15 @@ public void handleEvent(Http2StreamSourceChannel channel) {
* @param initial The initial upgrade request that started the HTTP2 connection
*/
void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte[] data) {
handleInitialRequest(initial, channel, data, this.decode);
}

/**
* Handles the initial request when the exchange was started by a HTTP upgrade.
*
* @param initial The initial upgrade request that started the HTTP2 connection
*/
void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte[] data, boolean decode) {
//we have a request
Http2HeadersStreamSinkChannel sink = channel.createInitialUpgradeResponseStream();
final Http2ServerConnection connection = new Http2ServerConnection(channel, sink, undertowOptions, bufferSize, rootHandler);
Expand Down Expand Up @@ -244,7 +254,7 @@ void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte
String uri = exchange.getQueryString().isEmpty() ? initial.getRequestURI() : initial.getRequestURI() + '?' + exchange.getQueryString();
try {
Connectors.setExchangeRequestPath(exchange, uri, encoding, decode, slashDecodingFlag, decodeBuffer, maxParameters);
} catch (ParameterLimitException e) {
} catch (ParameterLimitException | BadRequestException e) {
exchange.setStatusCode(StatusCodes.BAD_REQUEST);
exchange.endExchange();
return;
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/io/undertow/server/protocol/http2/Http2ServerConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import io.undertow.server.ServerConnection;
import io.undertow.util.AttachmentKey;
import io.undertow.util.AttachmentList;
import io.undertow.util.BadRequestException;
import io.undertow.util.HeaderMap;
import io.undertow.util.HttpString;
import io.undertow.util.StatusCodes;
Expand Down Expand Up @@ -442,7 +443,7 @@ public boolean pushResource(String path, HttpString method, HeaderMap requestHea
exchange.setRequestScheme(this.exchange.getRequestScheme());
try {
Connectors.setExchangeRequestPath(exchange, path, getUndertowOptions().get(UndertowOptions.URL_CHARSET, StandardCharsets.UTF_8.name()), getUndertowOptions().get(UndertowOptions.DECODE_URL, true), URLUtils.getSlashDecodingFlag(getUndertowOptions()), new StringBuilder(), getUndertowOptions().get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_HEADERS));
} catch (ParameterLimitException e) {
} catch (ParameterLimitException | BadRequestException e) {
UndertowLogger.REQUEST_IO_LOGGER.debug("Too many parameters in HTTP/2 request", e);
exchange.setStatusCode(StatusCodes.BAD_REQUEST);
exchange.endExchange();
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/io/undertow/server/protocol/http2/Http2UpgradeHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ public void handleRequest(HttpServerExchange exchange) throws Exception {
}
}, undertowOptions, exchange.getConnection().getBufferSize(), null);
channel.getReceiveSetter().set(receiveListener);
receiveListener.handleInitialRequest(exchange, channel, data);
// don't decode requests from upgrade, they are already decoded by the parser for protocol HTTP 1.1 (HttpRequestParser)
receiveListener.handleInitialRequest(exchange, channel, data, false);
channel.resumeReceives();
}
});
Expand Down