Skip to content

Commit 9d49c81

Browse files
fl4viafl4via
authored
Merge pull request #1653 from fl4via/backport-fixes_2.3.x
 [UNDERTOW-2312][UNDERTOW-2425][UNDERTOW-2424] Backport fixes
2 parents 2b6c7e5 + a499660 commit 9d49c81

18 files changed

+586
-145
lines changed

‎core/src/main/java/io/undertow/conduits/ChunkedStreamSinkConduit.java

+6
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,9 @@ int doWrite(final ByteBuffer src) throws IOException {
200200

201201
@Override
202202
public void truncateWrites() throws IOException {
203+
if (anyAreSet(state, FLAG_FINISHED)) {
204+
return;
205+
}
203206
try {
204207
if (lastChunkBuffer != null) {
205208
lastChunkBuffer.close();
@@ -259,6 +262,9 @@ public long transferFrom(final StreamSourceChannel source, final long count, fin
259262

260263
@Override
261264
public boolean flush() throws IOException {
265+
if (anyAreSet(state, FLAG_FINISHED)) {
266+
return true;
267+
}
262268
this.state |= FLAG_FIRST_DATA_WRITTEN;
263269
if (anyAreSet(state, FLAG_WRITES_SHUTDOWN)) {
264270
if (anyAreSet(state, FLAG_NEXT_SHUTDOWN)) {

‎core/src/main/java/io/undertow/protocols/http2/HpackDecoder.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ private String readHpackString(ByteBuffer buffer) throws HpackException {
248248
return readHuffmanString(length, buffer);
249249
}
250250
for (int i = 0; i < length; ++i) {
251-
stringBuilder.append((char) buffer.get());
251+
stringBuilder.append((char) (buffer.get() & 0xFF));
252252
}
253253
String ret = stringBuilder.toString();
254254
stringBuilder.setLength(0);

‎core/src/main/java/io/undertow/server/Connectors.java

+60-11
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import io.undertow.UndertowMessages;
2323
import io.undertow.UndertowOptions;
2424
import io.undertow.server.handlers.Cookie;
25+
import io.undertow.server.protocol.http.HttpRequestParser;
26+
import io.undertow.util.BadRequestException;
2527
import io.undertow.util.DateUtils;
2628
import io.undertow.util.HeaderMap;
2729
import io.undertow.util.HeaderValues;
@@ -446,7 +448,7 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
446448
try {
447449
final boolean slashDecodingFlag = URLUtils.getSlashDecodingFlag(allowEncodedSlash, exchange.getConnection().getUndertowOptions().get(UndertowOptions.DECODE_SLASH));
448450
setExchangeRequestPath(exchange, encodedPath, charset, decode, slashDecodingFlag, decodeBuffer, exchange.getConnection().getUndertowOptions().get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS));
449-
} catch (ParameterLimitException e) {
451+
} catch (ParameterLimitException | BadRequestException e) {
450452
throw new RuntimeException(e);
451453
}
452454
}
@@ -459,8 +461,9 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
459461
* @param encodedPath The encoded path to decode
460462
* @param decodeBuffer The decode buffer to use
461463
* @throws ParameterLimitException
464+
* @throws BadRequestException
462465
*/
463-
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, StringBuilder decodeBuffer) throws ParameterLimitException {
466+
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, StringBuilder decodeBuffer) throws ParameterLimitException, BadRequestException {
464467
final OptionMap options = exchange.getConnection().getUndertowOptions();
465468
boolean slashDecodingFlag = URLUtils.getSlashDecodingFlag(options);
466469
setExchangeRequestPath(exchange, encodedPath,
@@ -474,16 +477,44 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
474477
/**
475478
* Sets the request path and query parameters, decoding to the requested charset.
476479
*
477-
* @param exchange The exchange
478-
* @param encodedPath The encoded path
479-
* @param charset The charset
480+
* @param exchange the exchange
481+
* @param encodedPath the encoded path
482+
* @param decode indicates if the request path should be decoded
483+
* @param decodeSlashFlag indicates if slash characters contained in the encoded path should be decoded
484+
* @param decodeBuffer the buffer used for decoding
485+
* @param maxParameters maximum number of parameters allowed in the path
486+
* @param charset the charset
487+
* @throws BadRequestException if there is something wrong with the request, such as non-allowed characters
488+
*/
489+
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 {
490+
setExchangeRequestPath(exchange, encodedPath, charset, decode, decode, decodeSlashFlag, decodeBuffer, maxParameters);
491+
}
492+
493+
/**
494+
* Sets the request path and query parameters, decoding to the requested charset.
495+
*
496+
* @param exchange the exchange
497+
* @param encodedPath the encoded path
498+
* @param decode indicates if the request path should be decoded, apart from the query string part of the
499+
* request (see next parameter)
500+
* @param decodeQueryString indicates if the query string of the path, when present, should be decoded
501+
* @param decodeSlashFlag indicates if slash characters contained in the request path should be decoded
502+
* @param decodeBuffer the buffer used for decoding
503+
* @param maxParameters maximum number of parameters allowed in the path
504+
* @param charset the charset
505+
* @throws BadRequestException if there is something wrong with the request, such as non-allowed characters
480506
*/
481-
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, final boolean decodeSlashFlag, StringBuilder decodeBuffer, int maxParameters) throws ParameterLimitException {
507+
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 {
508+
final OptionMap options = exchange.getConnection().getUndertowOptions();
509+
final boolean allowUnescapedCharactersInUrl = options.get(UndertowOptions.ALLOW_UNESCAPED_CHARACTERS_IN_URL, false);
482510
boolean requiresDecode = false;
483511
final StringBuilder pathBuilder = new StringBuilder();
484512
int currentPathPartIndex = 0;
485513
for (int i = 0; i < encodedPath.length(); ++i) {
486514
char c = encodedPath.charAt(i);
515+
if(!allowUnescapedCharactersInUrl && !HttpRequestParser.isTargetCharacterAllowed(c)) {
516+
throw new BadRequestException(UndertowMessages.MESSAGES.invalidCharacterInRequestTarget(c));
517+
}
487518
if (c == '?') {
488519
String part;
489520
String encodedPart = encodedPath.substring(currentPathPartIndex, i);
@@ -496,10 +527,22 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
496527
part = pathBuilder.toString();
497528
exchange.setRequestPath(part);
498529
exchange.setRelativePath(part);
499-
exchange.setRequestURI(encodedPath.substring(0, i));
530+
if(requiresDecode && allowUnescapedCharactersInUrl) {
531+
final String uri = URLUtils.decode(encodedPath.substring(0, i), charset, decodeSlashFlag,false, decodeBuffer);
532+
exchange.setRequestURI(uri);
533+
} else {
534+
exchange.setRequestURI(encodedPath.substring(0, i));
535+
}
536+
500537
final String qs = encodedPath.substring(i + 1);
501-
exchange.setQueryString(qs);
502-
URLUtils.parseQueryString(qs, exchange, charset, decode, maxParameters);
538+
if(requiresDecode && allowUnescapedCharactersInUrl) {
539+
final String decodedQS = URLUtils.decode(qs, charset, decodeSlashFlag,false, decodeBuffer);
540+
exchange.setQueryString(decodedQS);
541+
} else {
542+
exchange.setQueryString(qs);
543+
}
544+
545+
URLUtils.parseQueryString(qs, exchange, charset, decodeQueryString, maxParameters);
503546
return;
504547
} else if(c == ';') {
505548
String part;
@@ -510,10 +553,16 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
510553
part = encodedPart;
511554
}
512555
pathBuilder.append(part);
513-
exchange.setRequestURI(encodedPath);
556+
if(requiresDecode && allowUnescapedCharactersInUrl) {
557+
final String uri = URLUtils.decode(encodedPath, charset, decodeSlashFlag,false, decodeBuffer);
558+
exchange.setRequestURI(uri);
559+
} else {
560+
exchange.setRequestURI(encodedPath);
561+
}
562+
514563
currentPathPartIndex = i + 1 + URLUtils.parsePathParams(encodedPath.substring(i + 1), exchange, charset, decode, maxParameters);
515564
i = currentPathPartIndex -1 ;
516-
} else if(c == '%' || c == '+') {
565+
} else if(decode && (c == '+' || c == '%' || c > 127)) {
517566
requiresDecode = decode;
518567
}
519568
}

‎core/src/main/java/io/undertow/server/protocol/ajp/AjpRequestParser.java

+13-6
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
import io.undertow.util.HttpString;
6767
import io.undertow.util.ParameterLimitException;
6868
import io.undertow.util.URLUtils;
69+
import io.undertow.util.UrlDecodeException;
6970

7071
/**
7172
* @author Stuart Douglas
@@ -268,7 +269,7 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final
268269
int colon = result.value.indexOf(';');
269270
if (colon == -1) {
270271
String res = decode(result.value, result.containsUrlCharacters);
271-
if(result.containsUnencodedCharacters) {
272+
if(result.containsUnencodedCharacters || (allowUnescapedCharactersInUrl && result.containsUrlCharacters)) {
272273
//we decode if the URL was non-compliant, and contained incorrectly encoded characters
273274
//there is not really a 'correct' thing to do in this situation, but this seems the least incorrect
274275
exchange.setRequestURI(res);
@@ -446,8 +447,14 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final
446447
state.state = AjpRequestParseState.READING_ATTRIBUTES;
447448
return;
448449
}
449-
if(resultHolder.containsUnencodedCharacters) {
450-
result = decode(resultHolder.value, true);
450+
if(resultHolder.containsUnencodedCharacters || (resultHolder.containsUrlCharacters && allowUnescapedCharactersInUrl)) {
451+
try {
452+
result = decode(resultHolder.value, true);
453+
} catch (UrlDecodeException | UnsupportedEncodingException e) {
454+
UndertowLogger.REQUEST_IO_LOGGER.failedToParseRequest(e);
455+
state.badRequest = true;
456+
result = resultHolder.value;
457+
}
451458
decodingAlreadyDone = true;
452459
} else {
453460
result = resultHolder.value;
@@ -580,16 +587,16 @@ protected StringHolder parseString(ByteBuffer buf, AjpRequestParseState state, S
580587
return new StringHolder(null, false, false, false);
581588
}
582589
byte c = buf.get();
583-
if(type == StringType.QUERY_STRING && (c == '+' || c == '%' || c < 0 )) {
584-
if (c < 0) {
590+
if(type == StringType.QUERY_STRING && (c == '+' || c == '%' || c < 0 || c > 127 )) {
591+
if (c < 0 || c > 127) {
585592
if (!allowUnescapedCharactersInUrl) {
586593
throw new BadRequestException();
587594
} else {
588595
containsUnencodedUrlCharacters = true;
589596
}
590597
}
591598
containsUrlCharacters = true;
592-
} else if(type == StringType.URL && (c == '%' || c < 0 )) {
599+
} else if(type == StringType.URL && (c == '%' || c < 0 || c > 127 )) {
593600
if(c < 0 ) {
594601
if(!allowUnescapedCharactersInUrl) {
595602
throw new BadRequestException();

‎core/src/main/java/io/undertow/server/protocol/http/HttpRequestParser.java

+24-11
Original file line numberDiff line numberDiff line change
@@ -497,10 +497,15 @@ private void parsePathComplete(ParseState state, HttpServerExchange exchange, in
497497
exchange.setRelativePath("/");
498498
exchange.setRequestURI(path, true);
499499
} else if (parseState < HOST_DONE && state.canonicalPath.length() == 0) {
500-
String decodedPath = decode(path, urlDecodeRequired, state, slashDecodingFlag, false);
501-
exchange.setRequestPath(decodedPath);
502-
exchange.setRelativePath(decodedPath);
503-
exchange.setRequestURI(path, false);
500+
final String decodedRequestPath = decode(path, urlDecodeRequired, state, slashDecodingFlag, false);
501+
exchange.setRequestPath(decodedRequestPath);
502+
exchange.setRelativePath(decodedRequestPath);
503+
if(urlDecodeRequired && allowUnescapedCharactersInUrl) {
504+
final String uri = decode(path, urlDecodeRequired, state, slashDecodingFlag, false);
505+
exchange.setRequestURI(uri);
506+
} else {
507+
exchange.setRequestURI(path);
508+
}
504509
} else {
505510
handleFullUrl(state, exchange, canonicalPathStart, urlDecodeRequired, path, parseState);
506511
}
@@ -520,10 +525,15 @@ private void beginQueryParameters(ByteBuffer buffer, ParseState state, HttpServe
520525

521526
private void handleFullUrl(ParseState state, HttpServerExchange exchange, int canonicalPathStart, boolean urlDecodeRequired, String path, int parseState) {
522527
state.canonicalPath.append(path.substring(canonicalPathStart));
523-
String thePath = decode(state.canonicalPath.toString(), urlDecodeRequired, state, slashDecodingFlag, false);
524-
exchange.setRequestPath(thePath);
525-
exchange.setRelativePath(thePath);
526-
exchange.setRequestURI(path, parseState == HOST_DONE);
528+
final String requestPath = decode(state.canonicalPath.toString(), urlDecodeRequired, state, slashDecodingFlag, false);
529+
exchange.setRequestPath(requestPath);
530+
exchange.setRelativePath(requestPath);
531+
if(urlDecodeRequired && allowUnescapedCharactersInUrl) {
532+
final String uri = decode(path, urlDecodeRequired, state, slashDecodingFlag, false);
533+
exchange.setRequestURI(uri, parseState == HOST_DONE);
534+
} else {
535+
exchange.setRequestURI(path, parseState == HOST_DONE);
536+
}
527537
}
528538

529539

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

553563
while (buffer.hasRemaining()) {
554-
char next = (char) (buffer.get() & 0xFF);
564+
final char next = (char) (buffer.get() & 0xFF);
555565
if(!allowUnescapedCharactersInUrl && !ALLOWED_TARGET_CHARACTER[next]) {
556566
throw new BadRequestException(UndertowMessages.MESSAGES.invalidCharacterInRequestTarget(next));
557567
}
558568
if (next == ' ' || next == '\t') {
559-
final String queryString = stringBuilder.toString();
569+
String queryString = stringBuilder.toString();
570+
if(urlDecodeRequired && this.allowUnescapedCharactersInUrl) {
571+
queryString = decode(queryString, urlDecodeRequired, state, slashDecodingFlag, false);
572+
}
560573
exchange.setQueryString(queryString);
561574
if (nextQueryParam == null) {
562575
if (queryParamPos != stringBuilder.length()) {

‎core/src/main/java/io/undertow/server/protocol/http2/Http2ReceiveListener.java

+17-3
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import io.undertow.server.protocol.http.HttpAttachments;
4444
import io.undertow.server.protocol.http.HttpContinue;
4545
import io.undertow.server.protocol.http.HttpRequestParser;
46+
import io.undertow.util.BadRequestException;
4647
import io.undertow.util.ConduitFactory;
4748
import io.undertow.util.HeaderMap;
4849
import io.undertow.util.HeaderValues;
@@ -193,7 +194,7 @@ public void handleEvent(Http2StreamSourceChannel channel) {
193194

194195
try {
195196
Connectors.setExchangeRequestPath(exchange, path, encoding, decode, slashDecodingFlag, decodeBuffer, maxParameters);
196-
} catch (ParameterLimitException e) {
197+
} catch (ParameterLimitException | BadRequestException e) {
197198
//this can happen if max parameters is exceeded
198199
UndertowLogger.REQUEST_IO_LOGGER.debug("Failed to set request path", e);
199200
exchange.setStatusCode(StatusCodes.BAD_REQUEST);
@@ -217,6 +218,19 @@ public void handleEvent(Http2StreamSourceChannel channel) {
217218
* @param initial The initial upgrade request that started the HTTP2 connection
218219
*/
219220
void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte[] data) {
221+
handleInitialRequest(initial, channel, data, this.decode, this.decode);
222+
}
223+
224+
/**
225+
* Handles the initial request when the exchange was started by a HTTP upgrade.
226+
*
227+
* @param initial the initial upgrade request that started the HTTP2 connection
228+
* @param channel the channel that received the request
229+
* @param data any extra data read by the channel that has not been parsed yet
230+
* @param decode indicates if the request path should be decoded, apart from the query string
231+
* @param decodeQueryString indicates if the query string of the path, when present, should be decoded
232+
*/
233+
void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte[] data, boolean decode, boolean decodeQueryString) {
220234
//we have a request
221235
Http2HeadersStreamSinkChannel sink = channel.createInitialUpgradeResponseStream();
222236
final Http2ServerConnection connection = new Http2ServerConnection(channel, sink, undertowOptions, bufferSize, rootHandler);
@@ -243,8 +257,8 @@ void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte
243257
Connectors.terminateRequest(exchange);
244258
String uri = exchange.getQueryString().isEmpty() ? initial.getRequestURI() : initial.getRequestURI() + '?' + exchange.getQueryString();
245259
try {
246-
Connectors.setExchangeRequestPath(exchange, uri, encoding, decode, slashDecodingFlag, decodeBuffer, maxParameters);
247-
} catch (ParameterLimitException e) {
260+
Connectors.setExchangeRequestPath(exchange, uri, encoding, decode, decodeQueryString, slashDecodingFlag, decodeBuffer, maxParameters);
261+
} catch (ParameterLimitException | BadRequestException e) {
248262
exchange.setStatusCode(StatusCodes.BAD_REQUEST);
249263
exchange.endExchange();
250264
return;

0 commit comments

Comments
 (0)