Skip to content

Commit

Permalink
[UNDERTOW-2312] Add test for access log with unescaped characters. Al…
Browse files Browse the repository at this point in the history
…so: make the test pass on the modes for HTTP2 upgrade and AJP

Signed-off-by: Flavia Rainone <frainone@redhat.com>
  • Loading branch information
fl4via committed Apr 18, 2024
1 parent f409c1e commit 45d8df5
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 7 deletions.
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
Original file line number Diff line number Diff line change
Expand Up @@ -218,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
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* JBoss, Home of Professional Open Source.
* Copyright 2024 Red Hat, Inc., and individual contributors
* as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.undertow.server.handlers.accesslog;

import io.undertow.UndertowOptions;
import io.undertow.server.HttpHandler;
import io.undertow.server.HttpServerExchange;
import io.undertow.testutils.DefaultServer;
import io.undertow.testutils.HttpClientUtils;
import io.undertow.testutils.TestHttpClient;
import io.undertow.util.CompletionLatchHandler;
import io.undertow.util.FileUtils;
import io.undertow.util.StatusCodes;
import org.apache.http.HttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.xnio.OptionMap;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

/**
* Tests writing the access log to a file
*
* @author Flavia Rainone
*/
@RunWith(DefaultServer.class)
public class AccessLogFileWithUnescapedCharactersTestCase {

private static final Path logDirectory = Paths.get(System.getProperty("java.io.tmpdir"), "logs");

private static final int NUM_THREADS = 10;
private static final int NUM_REQUESTS = 12;

@Before
public void before() throws IOException {
Files.createDirectories(logDirectory);
}

@After
public void after() throws IOException {
FileUtils.deleteRecursive(logDirectory);
}

private static final HttpHandler HELLO_HANDLER = new HttpHandler() {
@Override
public void handleRequest(final HttpServerExchange exchange) throws Exception {
exchange.getResponseSender().send("Hello");
}
};

@Test
public void testSingleLogMessageToFile() throws IOException, InterruptedException {
Path directory = logDirectory;
Path logFileName = directory.resolve("server1.log");
DefaultAccessLogReceiver logReceiver = new DefaultAccessLogReceiver(DefaultServer.getWorker(), directory, "server1.");
verifySingleLogMessageToFile(logFileName, logReceiver);
}

@Test
public void testSingleLogMessageToFileWithSuffix() throws IOException, InterruptedException {
Path directory = logDirectory;
Path logFileName = directory.resolve("server1.logsuffix");
DefaultAccessLogReceiver logReceiver = new DefaultAccessLogReceiver(DefaultServer.getWorker(), directory, "server1.", "logsuffix");
verifySingleLogMessageToFile(logFileName, logReceiver);
}

private void verifySingleLogMessageToFile(Path logFileName, DefaultAccessLogReceiver logReceiver) throws IOException, InterruptedException {
CompletionLatchHandler latchHandler;
DefaultServer.setRootHandler(latchHandler = new CompletionLatchHandler(new AccessLogHandler(HELLO_HANDLER, logReceiver,
"%h \"%r\" %s %b", AccessLogFileWithUnescapedCharactersTestCase.class.getClassLoader())));
DefaultServer.setUndertowOptions(
OptionMap.create(UndertowOptions.ALLOW_UNESCAPED_CHARACTERS_IN_URL, true));
DefaultServer.setServerOptions(OptionMap.create(UndertowOptions.ALLOW_UNESCAPED_CHARACTERS_IN_URL, true));
TestHttpClient client = new TestHttpClient();
try {
HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/helloworld/한글이름_test.html?param=한글이름_ahoy");
HttpResponse result = client.execute(get);
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
Assert.assertEquals("Hello", HttpClientUtils.readResponse(result));
latchHandler.await();
logReceiver.awaitWrittenForTest();
String written = new String(Files.readAllBytes(logFileName));
System.out.println("Look, this is written:\n" + written);
final String protocolVersion = DefaultServer.isH2()? "HTTP/2.0" : result.getProtocolVersion().toString();
Assert.assertEquals(DefaultServer.getDefaultServerAddress().getAddress().getHostAddress() + " \"GET " + "/helloworld/한글이름_test.html?param=한글이름_ahoy " + protocolVersion + "\" 200 5" + System.lineSeparator(), written);
} finally {
client.getConnectionManager().shutdown();
}
}
}

0 comments on commit 45d8df5

Please sign in to comment.