Skip to content

Commit 0672cfb

Browse files
michpetrovmichpetrov
committed
[UNDERTOW-2395] ensure proper encoding in RelativePathAttribute
1 parent 6d05233 commit 0672cfb

2 files changed

Lines changed: 93 additions & 27 deletions

File tree

‎core/src/main/java/io/undertow/attribute/RelativePathAttribute.java‎

Lines changed: 43 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@
2121
import io.undertow.server.HttpServerExchange;
2222
import io.undertow.util.QueryParameterUtils;
2323

24+
import java.io.UnsupportedEncodingException;
25+
import java.net.URI;
26+
import java.net.URISyntaxException;
27+
import java.net.URLDecoder;
28+
import java.util.Arrays;
29+
import java.util.stream.Collectors;
30+
2431
/**
2532
* The relative path
2633
*
@@ -33,6 +40,8 @@ public class RelativePathAttribute implements ExchangeAttribute {
3340

3441
public static final ExchangeAttribute INSTANCE = new RelativePathAttribute();
3542

43+
private static final String SLASH = "/";
44+
3645
private RelativePathAttribute() {
3746

3847
}
@@ -45,36 +54,47 @@ public String readAttribute(final HttpServerExchange exchange) {
4554
@Override
4655
public void writeAttribute(final HttpServerExchange exchange, final String newValue) throws ReadOnlyAttributeException {
4756
int pos = newValue.indexOf('?');
48-
if (pos == -1) {
49-
exchange.setRelativePath(newValue);
50-
String requestURI = exchange.getResolvedPath() + newValue;
51-
if(requestURI.contains("%")) {
52-
//as the request URI is supposed to be encoded we need to replace
53-
//percent characters with their encoded form, otherwise we can run into issues
54-
//where the percent will be taked to be a encoded character
55-
//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
56-
exchange.setRequestURI(requestURI.replaceAll("%", "%25"));
57-
} else {
58-
exchange.setRequestURI(requestURI);
59-
}
60-
exchange.setRequestPath(requestURI);
61-
} else {
62-
final String path = newValue.substring(0, pos);
63-
exchange.setRelativePath(path);
64-
String requestURI = exchange.getResolvedPath() + path;
65-
if(requestURI.contains("%")) {
66-
exchange.setRequestURI(requestURI.replaceAll("%", "%25"));
67-
} else {
68-
exchange.setRequestURI(requestURI);
69-
}
70-
exchange.setRequestPath(requestURI);
57+
String encoding = QueryParameterUtils.getQueryParamEncoding(exchange);
58+
final String path = pos == -1 ? newValue : newValue.substring(0, pos);
59+
final String decoded = decode(path, encoding);
60+
exchange.setRelativePath(decoded);
61+
final String requestURI = decode(exchange.getResolvedPath(), encoding) + decoded;
62+
exchange.setRequestURI(reencode(requestURI));
63+
exchange.setRequestPath(requestURI);
7164

65+
if (pos != -1) {
7266
final String newQueryString = newValue.substring(pos);
7367
exchange.setQueryString(newQueryString);
7468
exchange.getQueryParameters().putAll(QueryParameterUtils.parseQueryString(newQueryString.substring(1), QueryParameterUtils.getQueryParamEncoding(exchange)));
7569
}
7670
}
7771

72+
// the path might have inconsistent encoding so we try to decode it segment by segment
73+
private static String decode(String path, String encoding) {
74+
if (encoding == null) {
75+
return path;
76+
}
77+
return Arrays.stream(path.split(SLASH)).map(segment -> {
78+
try {
79+
return URLDecoder.decode(segment, encoding);
80+
} catch (IllegalArgumentException | UnsupportedEncodingException e) {
81+
return segment;
82+
}
83+
}).collect(Collectors.joining(SLASH));
84+
}
85+
86+
// re-encode the previously decoded path, this ensures the segments aren't encoded twice
87+
private static String reencode(String decodedPath) {
88+
return Arrays.stream(decodedPath.split(SLASH)).map(segment -> {
89+
try {
90+
// URI.toASCIIString does percent-encoding " " -> "%20", whereas URIEncoder does " " -> "+"
91+
return new URI(null, null, segment, null).toASCIIString();
92+
} catch (URISyntaxException e) {
93+
return segment;
94+
}
95+
}).collect(Collectors.joining(SLASH));
96+
}
97+
7898
@Override
7999
public String toString() {
80100
return RELATIVE_PATH;

‎core/src/test/java/io/undertow/server/handlers/SetAttributeTestCase.java‎

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
package io.undertow.server.handlers;
2020

21-
import io.undertow.Handlers;
2221
import io.undertow.server.HttpHandler;
2322
import io.undertow.server.HttpServerExchange;
2423
import io.undertow.testutils.DefaultServer;
@@ -37,6 +36,7 @@
3736

3837
import static io.undertow.Handlers.path;
3938
import static io.undertow.Handlers.rewrite;
39+
import static io.undertow.Handlers.setAttribute;
4040

4141

4242
/**
@@ -49,7 +49,7 @@ public class SetAttributeTestCase {
4949

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

5454
TestHttpClient client = new TestHttpClient();
5555
try {
@@ -81,10 +81,10 @@ public void testSettingHeader() throws IOException {
8181
@Test
8282
public void testRewrite() throws IOException {
8383
DefaultServer.setRootHandler(
84-
rewrite("regex['/somePath/(.*)']", "/otherPath/$1", getClass().getClassLoader(), path()
84+
rewrite("regex('/somePath/(.*)')", "/otherPath/$1", getClass().getClassLoader(), path()
8585
.addPrefixPath("/otherPath", new InfoHandler())
8686
.addPrefixPath("/relative",
87-
rewrite("path-template['/foo/{bar}/{woz}']", "/foo?bar=${bar}&woz=${woz}", getClass().getClassLoader(), new InfoHandler()))
87+
rewrite("path-template('/foo/{bar}/{woz}')", "/foo?bar=${bar}&woz=${woz}", getClass().getClassLoader(), new InfoHandler()))
8888
));
8989

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

115+
@Test
116+
public void testRewriteWithEncoding() throws IOException {
117+
DefaultServer.setRootHandler(
118+
rewrite("true", "/otherPath%{REQUEST_URL}", getClass().getClassLoader(), path()
119+
.addPrefixPath("/otherPath", new InfoHandler())));
120+
121+
TestHttpClient client = new TestHttpClient();
122+
try {
123+
HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/foobar");
124+
HttpResponse result = client.execute(get);
125+
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
126+
String response = HttpClientUtils.readResponse(result);
127+
Assert.assertEquals("URI: /otherPath/foobar relative: /foobar QS:", response);
128+
129+
get = new HttpGet(DefaultServer.getDefaultServerURL() + "/foo%20bar");
130+
result = client.execute(get);
131+
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
132+
response = HttpClientUtils.readResponse(result);
133+
Assert.assertEquals("URI: /otherPath/foo%20bar relative: /foo bar QS:", response);
134+
} finally {
135+
client.getConnectionManager().shutdown();
136+
}
137+
}
138+
139+
@Test
140+
public void testRewriteWithEncodingNoPathMatching() throws IOException {
141+
DefaultServer.setRootHandler(
142+
rewrite("true", "/other%%Path%{REQUEST_URL}", getClass().getClassLoader(), new InfoHandler()));
143+
144+
TestHttpClient client = new TestHttpClient();
145+
try {
146+
HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/foobar");
147+
HttpResponse result = client.execute(get);
148+
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
149+
String response = HttpClientUtils.readResponse(result);
150+
Assert.assertEquals("URI: /other%25Path/foobar relative: /other%Path/foobar QS:", response);
151+
152+
get = new HttpGet(DefaultServer.getDefaultServerURL() + "/foo%20bar");
153+
result = client.execute(get);
154+
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
155+
response = HttpClientUtils.readResponse(result);
156+
Assert.assertEquals("URI: /other%25Path/foo%20bar relative: /other%Path/foo bar QS:", response);
111157
} finally {
112158
client.getConnectionManager().shutdown();
113159
}

0 commit comments

Comments
 (0)