Skip to content

[UNDERTOW-2395] ensure proper encoding in RelativePathAttribute #1848

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

Open
michpetrov wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[UNDERTOW-2395] ensure proper encoding in RelativePathAttribute #1848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 43 additions & 23 deletions core/src/main/java/io/undertow/attribute/RelativePathAttribute.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@
import io.undertow.server.HttpServerExchange;
import io.undertow.util.QueryParameterUtils;

import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URLDecoder;
import java.util.Arrays;
import java.util.stream.Collectors;

/**
* The relative path
*
Expand All @@ -33,6 +40,8 @@ public class RelativePathAttribute implements ExchangeAttribute {

public static final ExchangeAttribute INSTANCE = new RelativePathAttribute();

private static final String SLASH = "/";

private RelativePathAttribute() {

}
Expand All @@ -45,36 +54,47 @@ public String readAttribute(final HttpServerExchange exchange) {
@Override
public void writeAttribute(final HttpServerExchange exchange, final String newValue) throws ReadOnlyAttributeException {
int pos = newValue.indexOf('?');
if (pos == -1) {
exchange.setRelativePath(newValue);
String requestURI = exchange.getResolvedPath() + newValue;
if(requestURI.contains("%")) {
//as the request URI is supposed to be encoded we need to replace
//percent characters with their encoded form, otherwise we can run into issues
//where the percent will be taked to be a encoded character
//TODO: should we fully encode this? It seems like it also has the potential to cause issues, and encoding the percent character is probably enough
exchange.setRequestURI(requestURI.replaceAll("%", "%25"));
} else {
exchange.setRequestURI(requestURI);
}
exchange.setRequestPath(requestURI);
} else {
final String path = newValue.substring(0, pos);
exchange.setRelativePath(path);
String requestURI = exchange.getResolvedPath() + path;
if(requestURI.contains("%")) {
exchange.setRequestURI(requestURI.replaceAll("%", "%25"));
} else {
exchange.setRequestURI(requestURI);
}
exchange.setRequestPath(requestURI);
String encoding = QueryParameterUtils.getQueryParamEncoding(exchange);
final String path = pos == -1 ? newValue : newValue.substring(0, pos);
final String decoded = decode(path, encoding);
exchange.setRelativePath(decoded);
final String requestURI = decode(exchange.getResolvedPath(), encoding) + decoded;
exchange.setRequestURI(reencode(requestURI));
exchange.setRequestPath(requestURI);

if (pos != -1) {
final String newQueryString = newValue.substring(pos);
exchange.setQueryString(newQueryString);
exchange.getQueryParameters().putAll(QueryParameterUtils.parseQueryString(newQueryString.substring(1), QueryParameterUtils.getQueryParamEncoding(exchange)));
}
}

// the path might have inconsistent encoding so we try to decode it segment by segment
private static String decode(String path, String encoding) {
if (encoding == null) {
return path;
}
return Arrays.stream(path.split(SLASH)).map(segment -> {
try {
return URLDecoder.decode(segment, encoding);
} catch (IllegalArgumentException | UnsupportedEncodingException e) {
return segment;
}
}).collect(Collectors.joining(SLASH));
}

// re-encode the previously decoded path, this ensures the segments aren't encoded twice
private static String reencode(String decodedPath) {
return Arrays.stream(decodedPath.split(SLASH)).map(segment -> {
try {
// URI.toASCIIString does percent-encoding " " -> "%20", whereas URIEncoder does " " -> "+"
return new URI(null, null, segment, null).toASCIIString();
} catch (URISyntaxException e) {
return segment;
}
}).collect(Collectors.joining(SLASH));
}

@Override
public String toString() {
return RELATIVE_PATH;
Expand Down
54 changes: 50 additions & 4 deletions core/src/test/java/io/undertow/server/handlers/SetAttributeTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

package io.undertow.server.handlers;

import io.undertow.Handlers;
import io.undertow.server.HttpHandler;
import io.undertow.server.HttpServerExchange;
import io.undertow.testutils.DefaultServer;
Expand All @@ -37,6 +36,7 @@

import static io.undertow.Handlers.path;
import static io.undertow.Handlers.rewrite;
import static io.undertow.Handlers.setAttribute;


/**
Expand All @@ -49,7 +49,7 @@ public class SetAttributeTestCase {

@Test
public void testSettingHeader() throws IOException {
DefaultServer.setRootHandler(Handlers.setAttribute(ResponseCodeHandler.HANDLE_200, "%{o,Foo}", "%U-%{q,p1}", SetAttributeHandler.class.getClassLoader()));
DefaultServer.setRootHandler(setAttribute(ResponseCodeHandler.HANDLE_200, "%{o,Foo}", "%U-%{q,p1}", SetAttributeHandler.class.getClassLoader()));

TestHttpClient client = new TestHttpClient();
try {
Expand Down Expand Up @@ -81,10 +81,10 @@ public void testSettingHeader() throws IOException {
@Test
public void testRewrite() throws IOException {
DefaultServer.setRootHandler(
rewrite("regex['/somePath/(.*)']", "/otherPath/$1", getClass().getClassLoader(), path()
rewrite("regex('/somePath/(.*)')", "/otherPath/$1", getClass().getClassLoader(), path()
.addPrefixPath("/otherPath", new InfoHandler())
.addPrefixPath("/relative",
rewrite("path-template['/foo/{bar}/{woz}']", "/foo?bar=${bar}&woz=${woz}", getClass().getClassLoader(), new InfoHandler()))
rewrite("path-template('/foo/{bar}/{woz}')", "/foo?bar=${bar}&woz=${woz}", getClass().getClassLoader(), new InfoHandler()))
));

TestHttpClient client = new TestHttpClient();
Expand All @@ -107,7 +107,53 @@ public void testRewrite() throws IOException {
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
response = HttpClientUtils.readResponse(result);
Assert.assertEquals("URI: /otherPath/foo relative: /foo QS:a=b a: b", response);
} finally {
client.getConnectionManager().shutdown();
}
}

@Test
public void testRewriteWithEncoding() throws IOException {
DefaultServer.setRootHandler(
rewrite("true", "/otherPath%{REQUEST_URL}", getClass().getClassLoader(), path()
.addPrefixPath("/otherPath", new InfoHandler())));

TestHttpClient client = new TestHttpClient();
try {
HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/foobar");
HttpResponse result = client.execute(get);
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
String response = HttpClientUtils.readResponse(result);
Assert.assertEquals("URI: /otherPath/foobar relative: /foobar QS:", response);

get = new HttpGet(DefaultServer.getDefaultServerURL() + "/foo%20bar");
result = client.execute(get);
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
response = HttpClientUtils.readResponse(result);
Assert.assertEquals("URI: /otherPath/foo%20bar relative: /foo bar QS:", response);
} finally {
client.getConnectionManager().shutdown();
}
}

@Test
public void testRewriteWithEncodingNoPathMatching() throws IOException {
DefaultServer.setRootHandler(
rewrite("true", "/other%%Path%{REQUEST_URL}", getClass().getClassLoader(), new InfoHandler()));

TestHttpClient client = new TestHttpClient();
try {
HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/foobar");
HttpResponse result = client.execute(get);
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
String response = HttpClientUtils.readResponse(result);
Assert.assertEquals("URI: /other%25Path/foobar relative: /other%Path/foobar QS:", response);

get = new HttpGet(DefaultServer.getDefaultServerURL() + "/foo%20bar");
result = client.execute(get);
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
response = HttpClientUtils.readResponse(result);
Assert.assertEquals("URI: /other%25Path/foo%20bar relative: /other%Path/foo bar QS:", response);
} finally {
client.getConnectionManager().shutdown();
}
Expand Down
Loading