Skip to content

Commit

Permalink
Merge pull request #1653 from fl4via/backport-fixes_2.3.x
Browse files Browse the repository at this point in the history
 [UNDERTOW-2312][UNDERTOW-2425][UNDERTOW-2424] Backport fixes
  • Loading branch information
fl4via authored Aug 22, 2024
2 parents 2b6c7e5 + a499660 commit 9d49c81
Show file tree
Hide file tree
Showing 18 changed files with 586 additions and 145 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,9 @@ int doWrite(final ByteBuffer src) throws IOException {

@Override
public void truncateWrites() throws IOException {
if (anyAreSet(state, FLAG_FINISHED)) {
return;
}
try {
if (lastChunkBuffer != null) {
lastChunkBuffer.close();
Expand Down Expand Up @@ -259,6 +262,9 @@ public long transferFrom(final StreamSourceChannel source, final long count, fin

@Override
public boolean flush() throws IOException {
if (anyAreSet(state, FLAG_FINISHED)) {
return true;
}
this.state |= FLAG_FIRST_DATA_WRITTEN;
if (anyAreSet(state, FLAG_WRITES_SHUTDOWN)) {
if (anyAreSet(state, FLAG_NEXT_SHUTDOWN)) {
Expand Down
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
71 changes: 60 additions & 11 deletions core/src/main/java/io/undertow/server/Connectors.java
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 @@ -474,16 +477,44 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
/**
* Sets the request path and query parameters, decoding to the requested charset.
*
* @param exchange The exchange
* @param encodedPath The encoded path
* @param charset The charset
* @param exchange the exchange
* @param encodedPath the encoded path
* @param decode indicates if the request path should be decoded
* @param decodeSlashFlag indicates if slash characters contained in the encoded path should be decoded
* @param decodeBuffer the buffer used for decoding
* @param maxParameters maximum number of parameters allowed in the path
* @param charset the charset
* @throws BadRequestException if there is something wrong with the request, such as non-allowed characters
*/
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 {
setExchangeRequestPath(exchange, encodedPath, charset, decode, decode, decodeSlashFlag, decodeBuffer, maxParameters);
}

/**
* Sets the request path and query parameters, decoding to the requested charset.
*
* @param exchange the exchange
* @param encodedPath the encoded path
* @param decode indicates if the request path should be decoded, apart from the query string part of the
* request (see next parameter)
* @param decodeQueryString indicates if the query string of the path, when present, should be decoded
* @param decodeSlashFlag indicates if slash characters contained in the request path should be decoded
* @param decodeBuffer the buffer used for decoding
* @param maxParameters maximum number of parameters allowed in the path
* @param charset the charset
* @throws BadRequestException if there is something wrong with the request, such as non-allowed characters
*/
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, boolean decodeQueryString, 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,10 +527,22 @@ 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);
URLUtils.parseQueryString(qs, exchange, charset, decode, maxParameters);
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, decodeQueryString, maxParameters);
return;
} else if(c == ';') {
String part;
Expand All @@ -510,10 +553,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
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 @@ -268,7 +269,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 @@ -446,8 +447,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 @@ -580,16 +587,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
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
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,19 @@ 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, 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
* @param channel the channel that received the request
* @param data any extra data read by the channel that has not been parsed yet
* @param decode indicates if the request path should be decoded, apart from the query string
* @param decodeQueryString indicates if the query string of the path, when present, should be decoded
*/
void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte[] data, boolean decode, boolean decodeQueryString) {
//we have a request
Http2HeadersStreamSinkChannel sink = channel.createInitialUpgradeResponseStream();
final Http2ServerConnection connection = new Http2ServerConnection(channel, sink, undertowOptions, bufferSize, rootHandler);
Expand All @@ -243,8 +257,8 @@ void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte
Connectors.terminateRequest(exchange);
String uri = exchange.getQueryString().isEmpty() ? initial.getRequestURI() : initial.getRequestURI() + '?' + exchange.getQueryString();
try {
Connectors.setExchangeRequestPath(exchange, uri, encoding, decode, slashDecodingFlag, decodeBuffer, maxParameters);
} catch (ParameterLimitException e) {
Connectors.setExchangeRequestPath(exchange, uri, encoding, decode, decodeQueryString, slashDecodingFlag, decodeBuffer, maxParameters);
} catch (ParameterLimitException | BadRequestException e) {
exchange.setStatusCode(StatusCodes.BAD_REQUEST);
exchange.endExchange();
return;
Expand Down
Loading

0 comments on commit 9d49c81

Please sign in to comment.