Skip to content

Commit 4ca8afb

Browse files
sbordetsbordet
authored
Fixes #8014 - Review HttpRequest URI construction. (#8146)
Now always adding a "/" before the path, if not already present. Parse CONNECT URIs as Authority Co-authored-by: Greg Wilkins <[email protected]> (cherry picked from commit d1e64f4)
1 parent 18653c4 commit 4ca8afb

File tree

6 files changed

+169
-7
lines changed

6 files changed

+169
-7
lines changed

‎jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ public Request path(String path)
197197
String rawPath = uri.getRawPath();
198198
if (rawPath == null)
199199
rawPath = "";
200+
if (!rawPath.startsWith("/"))
201+
rawPath = "/" + rawPath;
200202
this.path = rawPath;
201203
String query = uri.getRawQuery();
202204
if (query != null)
@@ -925,14 +927,14 @@ private URI buildURI(boolean withQuery)
925927
return result;
926928
}
927929

928-
private URI newURI(String uri)
930+
private URI newURI(String path)
929931
{
930932
try
931933
{
932934
// Handle specially the "OPTIONS *" case, since it is possible to create a URI from "*" (!).
933-
if ("*".equals(uri))
935+
if ("*".equals(path))
934936
return null;
935-
URI result = new URI(uri);
937+
URI result = new URI(path);
936938
return result.isOpaque() ? null : result;
937939
}
938940
catch (URISyntaxException x)

‎jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java

+45
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@
2929
import java.net.URLEncoder;
3030
import java.nio.charset.StandardCharsets;
3131
import java.util.Locale;
32+
import java.util.concurrent.CountDownLatch;
3233
import java.util.concurrent.ExecutionException;
3334
import java.util.concurrent.TimeUnit;
35+
import java.util.concurrent.atomic.AtomicReference;
3436
import javax.servlet.ServletException;
3537
import javax.servlet.http.HttpServletRequest;
3638
import javax.servlet.http.HttpServletResponse;
@@ -82,6 +84,49 @@ public void testIPv6Host(Scenario scenario) throws Exception
8284
assertEquals(HttpStatus.OK_200, request.send().getStatus());
8385
}
8486

87+
@ParameterizedTest
88+
@ArgumentsSource(ScenarioProvider.class)
89+
public void testPathWithPathParameter(Scenario scenario) throws Exception
90+
{
91+
AtomicReference<CountDownLatch> serverLatchRef = new AtomicReference<>();
92+
start(scenario, new EmptyServerHandler()
93+
{
94+
@Override
95+
protected void service(String target, org.eclipse.jetty.server.Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
96+
{
97+
if (jettyRequest.getHttpURI().hasAmbiguousEmptySegment())
98+
response.setStatus(400);
99+
serverLatchRef.get().countDown();
100+
}
101+
});
102+
103+
serverLatchRef.set(new CountDownLatch(1));
104+
ContentResponse response1 = client.newRequest("localhost", connector.getLocalPort())
105+
.scheme(scenario.getScheme())
106+
.path("/url;p=v")
107+
.send();
108+
assertEquals(HttpStatus.OK_200, response1.getStatus());
109+
assertTrue(serverLatchRef.get().await(5, TimeUnit.SECONDS));
110+
111+
// Ambiguous empty segment.
112+
serverLatchRef.set(new CountDownLatch(1));
113+
ContentResponse response2 = client.newRequest("localhost", connector.getLocalPort())
114+
.scheme(scenario.getScheme())
115+
.path(";p=v/url")
116+
.send();
117+
assertEquals(HttpStatus.BAD_REQUEST_400, response2.getStatus());
118+
assertTrue(serverLatchRef.get().await(5, TimeUnit.SECONDS));
119+
120+
// Ambiguous empty segment.
121+
serverLatchRef.set(new CountDownLatch(1));
122+
ContentResponse response3 = client.newRequest("localhost", connector.getLocalPort())
123+
.scheme(scenario.getScheme())
124+
.path(";@host.org/url")
125+
.send();
126+
assertEquals(HttpStatus.BAD_REQUEST_400, response3.getStatus());
127+
assertTrue(serverLatchRef.get().await(5, TimeUnit.SECONDS));
128+
}
129+
85130
@ParameterizedTest
86131
@ArgumentsSource(ScenarioProvider.class)
87132
public void testIDNHost(Scenario scenario) throws Exception

‎jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java

+21-1
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ public void parseRequestTarget(String method, String uri)
308308
_uri = uri;
309309

310310
if (HttpMethod.CONNECT.is(method))
311-
_path = uri;
311+
parse(State.HOST, uri, 0, uri.length());
312312
else
313313
parse(uri.startsWith("/") ? State.PATH : State.START, uri, 0, uri.length());
314314
}
@@ -1032,16 +1032,29 @@ public void setScheme(String scheme)
10321032
*/
10331033
public void setAuthority(String host, int port)
10341034
{
1035+
if (host != null && !isPathValidForAuthority(_path))
1036+
throw new IllegalArgumentException("Relative path with authority");
10351037
_host = host;
10361038
_port = port;
10371039
_uri = null;
10381040
}
10391041

1042+
private boolean isPathValidForAuthority(String path)
1043+
{
1044+
if (path == null)
1045+
return true;
1046+
if (path.isEmpty() || "*".equals(path))
1047+
return true;
1048+
return path.startsWith("/");
1049+
}
1050+
10401051
/**
10411052
* @param path the path
10421053
*/
10431054
public void setPath(String path)
10441055
{
1056+
if (hasAuthority() && !isPathValidForAuthority(path))
1057+
throw new IllegalArgumentException("Relative path with authority");
10451058
_uri = null;
10461059
_path = null;
10471060
if (path != null)
@@ -1050,6 +1063,8 @@ public void setPath(String path)
10501063

10511064
public void setPathQuery(String pathQuery)
10521065
{
1066+
if (hasAuthority() && !isPathValidForAuthority(pathQuery))
1067+
throw new IllegalArgumentException("Relative path with authority");
10531068
_uri = null;
10541069
_path = null;
10551070
_decodedPath = null;
@@ -1063,6 +1078,11 @@ public void setPathQuery(String pathQuery)
10631078
parse(State.PATH, pathQuery, 0, pathQuery.length());
10641079
}
10651080

1081+
private boolean hasAuthority()
1082+
{
1083+
return _host != null;
1084+
}
1085+
10661086
public void setQuery(String query)
10671087
{
10681088
_query = query;

‎jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java

+86
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,32 @@ public void testParseRequestTarget()
116116
assertThat(uri.getPath(), is("/bar"));
117117
}
118118

119+
@Test
120+
public void testCONNECT()
121+
{
122+
HttpURI uri = new HttpURI();
123+
124+
uri.parseRequestTarget("CONNECT", "host:80");
125+
assertThat(uri.getHost(), is("host"));
126+
assertThat(uri.getPort(), is(80));
127+
assertThat(uri.getPath(), nullValue());
128+
129+
uri.parseRequestTarget("CONNECT", "host");
130+
assertThat(uri.getHost(), is("host"));
131+
assertThat(uri.getPort(), is(-1));
132+
assertThat(uri.getPath(), nullValue());
133+
134+
uri.parseRequestTarget("CONNECT", "192.168.0.1:8080");
135+
assertThat(uri.getHost(), is("192.168.0.1"));
136+
assertThat(uri.getPort(), is(8080));
137+
assertThat(uri.getPath(), nullValue());
138+
139+
uri.parseRequestTarget("CONNECT", "[::1]:8080");
140+
assertThat(uri.getHost(), is("[::1]"));
141+
assertThat(uri.getPort(), is(8080));
142+
assertThat(uri.getPath(), nullValue());
143+
}
144+
119145
@Test
120146
public void testExtB() throws Exception
121147
{
@@ -789,4 +815,64 @@ public void testEncodedQuery(String input, String expectedQuery)
789815
HttpURI httpURI = new HttpURI(input);
790816
assertThat("[" + input + "] .query", httpURI.getQuery(), is(expectedQuery));
791817
}
818+
819+
@Test
820+
public void testRelativePathWithAuthority()
821+
{
822+
assertThrows(IllegalArgumentException.class, () ->
823+
{
824+
HttpURI httpURI = new HttpURI();
825+
httpURI.setAuthority("host", 0);
826+
httpURI.setPath("path");
827+
});
828+
assertThrows(IllegalArgumentException.class, () ->
829+
{
830+
HttpURI httpURI = new HttpURI();
831+
httpURI.setAuthority("host", 8080);
832+
httpURI.setPath(";p=v/url");
833+
});
834+
assertThrows(IllegalArgumentException.class, () ->
835+
{
836+
HttpURI httpURI = new HttpURI();
837+
httpURI.setAuthority("host", 0);
838+
httpURI.setPath(";");
839+
});
840+
841+
assertThrows(IllegalArgumentException.class, () ->
842+
{
843+
HttpURI httpURI = new HttpURI();
844+
httpURI.setPath("path");
845+
httpURI.setAuthority("host", 0);
846+
});
847+
assertThrows(IllegalArgumentException.class, () ->
848+
{
849+
HttpURI httpURI = new HttpURI();
850+
httpURI.setPath(";p=v/url");
851+
httpURI.setAuthority("host", 8080);
852+
});
853+
assertThrows(IllegalArgumentException.class, () ->
854+
{
855+
HttpURI httpURI = new HttpURI();
856+
httpURI.setPath(";");
857+
httpURI.setAuthority("host", 0);
858+
});
859+
860+
HttpURI uri = new HttpURI();
861+
uri.setPath("*");
862+
uri.setAuthority("host", 0);
863+
assertEquals("//host*", uri.toString());
864+
uri = new HttpURI();
865+
uri.setAuthority("host", 0);
866+
uri.setPath("*");
867+
assertEquals("//host*", uri.toString());
868+
869+
uri = new HttpURI();
870+
uri.setPath("");
871+
uri.setAuthority("host", 0);
872+
assertEquals("//host", uri.toString());
873+
uri = new HttpURI();
874+
uri.setAuthority("host", 0);
875+
uri.setPath("");
876+
assertEquals("//host", uri.toString());
877+
}
792878
}

‎jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
195195
{
196196
if (HttpMethod.CONNECT.is(request.getMethod()))
197197
{
198-
String serverAddress = request.getRequestURI();
198+
String serverAddress = baseRequest.getHttpURI().getAuthority();
199199
if (LOG.isDebugEnabled())
200200
LOG.debug("CONNECT request for {}", serverAddress);
201201

‎jetty-server/src/main/java/org/eclipse/jetty/server/Request.java

+11-2
Original file line numberDiff line numberDiff line change
@@ -1836,9 +1836,18 @@ public void setMetaData(org.eclipse.jetty.http.MetaData.Request request)
18361836
throw new BadMessageException(badMessage);
18371837
}
18381838

1839-
_originalURI = uri.isAbsolute() && request.getHttpVersion() != HttpVersion.HTTP_2 ? uri.toString() : uri.getPathQuery();
1839+
String encoded;
1840+
if (HttpMethod.CONNECT.is(request.getMethod()))
1841+
{
1842+
_originalURI = uri.getAuthority();
1843+
encoded = "/";
1844+
}
1845+
else
1846+
{
1847+
_originalURI = uri.isAbsolute() && request.getHttpVersion() != HttpVersion.HTTP_2 ? uri.toString() : uri.getPathQuery();
1848+
encoded = uri.getPath();
1849+
}
18401850

1841-
String encoded = uri.getPath();
18421851
String path;
18431852
if (encoded == null)
18441853
{

0 commit comments

Comments
 (0)