Skip to content

Commit 428b79b

Browse files
gregwgregw
authored
Do not decode path in EE9 Dispatcher (#12796)
Fix #12796 * Do not decode path in EE9 Dispatcher
1 parent 9a3abc3 commit 428b79b

File tree

4 files changed

+112
-10
lines changed

4 files changed

+112
-10
lines changed

‎jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/CrossContextDispatcherTest.java

+55-2
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@
5151
import jakarta.servlet.http.HttpServletResponseWrapper;
5252
import jakarta.servlet.http.Part;
5353
import org.eclipse.jetty.http.HttpTester;
54+
import org.eclipse.jetty.http.UriCompliance;
5455
import org.eclipse.jetty.server.HttpConfiguration;
56+
import org.eclipse.jetty.server.HttpConnectionFactory;
5557
import org.eclipse.jetty.server.LocalConnector;
5658
import org.eclipse.jetty.server.Server;
5759
import org.eclipse.jetty.server.handler.ContextHandler;
@@ -60,6 +62,7 @@
6062
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
6163
import org.eclipse.jetty.util.MultiMap;
6264
import org.eclipse.jetty.util.StringUtil;
65+
import org.eclipse.jetty.util.URIUtil;
6366
import org.eclipse.jetty.util.UrlEncoded;
6467
import org.eclipse.jetty.util.resource.ResourceFactory;
6568
import org.junit.jupiter.api.AfterEach;
@@ -295,6 +298,56 @@ public void testSimpleCrossContextForward() throws Exception
295298
assertThat(content, containsString("REQUEST_URI=/foreign/verify/pinfo"));
296299
}
297300

301+
@Test
302+
public void testEncodedCrossContextForward() throws Exception
303+
{
304+
_server.stop();
305+
_targetServletContextHandler.addServlet(VerifyForwardServlet.class, "/verify/*");
306+
_targetServletContextHandler.getServletHandler().setDecodeAmbiguousURIs(true);
307+
_contextHandler.addServlet(CrossContextDispatchServlet.class, "/dispatch/*");
308+
_contextHandler.getServletHandler().setDecodeAmbiguousURIs(true);
309+
_server.getContainedBeans(HttpConnectionFactory.class).forEach(f -> f.getHttpConfiguration().setUriCompliance(UriCompliance.DEFAULT.with("test", UriCompliance.Violation.AMBIGUOUS_PATH_ENCODING)));
310+
_server.start();
311+
312+
String rawResponse = _connector.getResponse("""
313+
GET /context/dispatch/?forward=/verify/%25%20test HTTP/1.1\r
314+
Host: localhost\r
315+
Connection: close\r
316+
\r
317+
""");
318+
319+
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
320+
321+
String content = response.getContent();
322+
String[] contentLines = content.split("\\n");
323+
324+
//verify forward attributes
325+
assertThat(content, containsString("Verified!"));
326+
assertThat(content, containsString("jakarta.servlet.forward.context_path=/context"));
327+
assertThat(content, containsString("jakarta.servlet.forward.servlet_path=/dispatch"));
328+
assertThat(content, containsString("jakarta.servlet.forward.path_info=/"));
329+
330+
String forwardMapping = extractLine(contentLines, "jakarta.servlet.forward.mapping=");
331+
assertNotNull(forwardMapping);
332+
assertThat(forwardMapping, containsString("CrossContextDispatchServlet"));
333+
assertThat(content, containsString("jakarta.servlet.forward.query_string=forward=/verify"));
334+
assertThat(content, containsString("jakarta.servlet.forward.request_uri=/context/dispatch/"));
335+
//verify request values
336+
assertThat(content, containsString("REQUEST_URL=http://localhost/foreign/"));
337+
assertThat(content, containsString("CONTEXT_PATH=/foreign"));
338+
assertThat(content, containsString("SERVLET_PATH=/verify"));
339+
assertThat(content, containsString("PATH_INFO=/% test/pinfo"));
340+
String mapping = extractLine(contentLines, "MAPPING=");
341+
assertNotNull(mapping);
342+
assertThat(mapping, containsString("VerifyForwardServlet"));
343+
String params = extractLine(contentLines, "PARAMS=");
344+
assertNotNull(params);
345+
params = params.substring(params.indexOf("=") + 1);
346+
params = params.substring(1, params.length() - 1); //dump leading, trailing [ ]
347+
assertThat(Arrays.asList(StringUtil.csvSplit(params)), containsInAnyOrder("a", "forward"));
348+
assertThat(content, containsString("REQUEST_URI=/foreign/verify/%25%20test/pinfo"));
349+
}
350+
298351
@Test
299352
public void testSimpleCrossContextInclude() throws Exception
300353
{
@@ -831,7 +884,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t
831884
{
832885
ServletContext foreign = getServletContext().getContext(ctx);
833886
assertNotNull(foreign);
834-
dispatcher = foreign.getRequestDispatcher(request.getParameter("forward") + "/pinfo?a=b");
887+
dispatcher = foreign.getRequestDispatcher(URIUtil.encodePath(request.getParameter("forward")) + "/pinfo?a=b");
835888

836889
if (dispatcher == null)
837890
response.sendError(404, "No dispatcher for forward");
@@ -994,7 +1047,7 @@ public void service(ServletRequest req, ServletResponse res) throws ServletExcep
9941047
res.getWriter().println("----------- FORWARD ATTRIBUTES");
9951048
res.getWriter().println(RequestDispatcher.FORWARD_CONTEXT_PATH + "=" + req.getAttribute(RequestDispatcher.FORWARD_CONTEXT_PATH));
9961049
res.getWriter().println(RequestDispatcher.FORWARD_SERVLET_PATH + "=" + req.getAttribute(RequestDispatcher.FORWARD_SERVLET_PATH));
997-
res.getWriter().println(RequestDispatcher.FORWARD_PATH_INFO + "=" + req.getAttribute(RequestDispatcher.FORWARD_PATH_INFO));
1050+
res.getWriter().println(RequestDispatcher.FORWARD_PATH_INFO + "=" + req.getAttribute(RequestDispatcher.FORWARD_PATH_INFO));
9981051
res.getWriter().println(RequestDispatcher.FORWARD_MAPPING + "=" + req.getAttribute(RequestDispatcher.FORWARD_MAPPING));
9991052
res.getWriter().println(RequestDispatcher.FORWARD_QUERY_STRING + "=" + req.getAttribute(RequestDispatcher.FORWARD_QUERY_STRING));
10001053
res.getWriter().println(RequestDispatcher.FORWARD_REQUEST_URI + "=" + req.getAttribute(RequestDispatcher.FORWARD_REQUEST_URI));

‎jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -1915,14 +1915,14 @@ public RequestDispatcher getRequestDispatcher(String uriInContext)
19151915
String contextPath = getContextPath();
19161916
// uriInContext is canonicalized by HttpURI.
19171917
HttpURI.Mutable uri = HttpURI.build(uriInContext);
1918-
String pathInfo = uri.getCanonicalPath();
1918+
String pathInfo = uri.getDecodedPath();
19191919
if (StringUtil.isEmpty(pathInfo))
19201920
return null;
19211921

19221922
if (!StringUtil.isEmpty(contextPath))
19231923
{
19241924
uri.path(URIUtil.addPaths(contextPath, uri.getPath()));
1925-
pathInfo = uri.getCanonicalPath().substring(contextPath.length());
1925+
pathInfo = uri.getDecodedPath().substring(contextPath.length());
19261926
}
19271927
return new Dispatcher(ContextHandler.this, uri, pathInfo);
19281928
}

‎jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Dispatcher.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import org.eclipse.jetty.http.UriCompliance;
3232
import org.eclipse.jetty.util.Attributes;
3333
import org.eclipse.jetty.util.Fields;
34-
import org.eclipse.jetty.util.URIUtil;
3534
import org.slf4j.Logger;
3635
import org.slf4j.LoggerFactory;
3736

@@ -187,9 +186,8 @@ protected void forward(ServletRequest request, ServletResponse response, Dispatc
187186
if (query == null)
188187
query = old_uri.getQuery();
189188

190-
String decodedPathInContext = URIUtil.decodePath(_pathInContext);
191189
baseRequest.setHttpURI(HttpURI.build(old_uri, _uri.getPath(), _uri.getParam(), query));
192-
baseRequest.setContext(_contextHandler.getServletContext(), decodedPathInContext);
190+
baseRequest.setContext(_contextHandler.getServletContext(), _pathInContext);
193191

194192
if (_uri.getQuery() != null || old_uri.getQuery() != null)
195193
{
@@ -212,7 +210,7 @@ protected void forward(ServletRequest request, ServletResponse response, Dispatc
212210
}
213211
}
214212

215-
_contextHandler.handle(decodedPathInContext, baseRequest, (HttpServletRequest)request, (HttpServletResponse)response);
213+
_contextHandler.handle(_pathInContext, baseRequest, (HttpServletRequest)request, (HttpServletResponse)response);
216214

217215
// If we are not async and not closed already, then close via the possibly wrapped response.
218216
if (!baseRequest.getHttpChannelState().isAsync() && !baseResponse.getHttpOutput().isClosed())

‎jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/CrossContextDispatcherTest.java

+53-2
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@
5353
import org.eclipse.jetty.ee9.nested.Dispatcher;
5454
import org.eclipse.jetty.ee9.nested.Request;
5555
import org.eclipse.jetty.http.HttpTester;
56+
import org.eclipse.jetty.http.UriCompliance;
5657
import org.eclipse.jetty.server.HttpConfiguration;
58+
import org.eclipse.jetty.server.HttpConnectionFactory;
5759
import org.eclipse.jetty.server.LocalConnector;
5860
import org.eclipse.jetty.server.Server;
5961
import org.eclipse.jetty.server.handler.ContextHandler;
@@ -62,6 +64,7 @@
6264
import org.eclipse.jetty.toolchain.test.MavenPaths;
6365
import org.eclipse.jetty.util.MultiMap;
6466
import org.eclipse.jetty.util.StringUtil;
67+
import org.eclipse.jetty.util.URIUtil;
6568
import org.eclipse.jetty.util.UrlEncoded;
6669
import org.eclipse.jetty.util.resource.ResourceFactory;
6770
import org.junit.jupiter.api.AfterEach;
@@ -298,6 +301,54 @@ public void testSimpleCrossContextForward() throws Exception
298301
assertThat(content, containsString("TYPE=org.eclipse.jetty.ee9.servlet.CrossContextDispatcherTest$MyHttpServletRequestWrapper"));
299302
}
300303

304+
@Test
305+
public void testEncodedCrossContextForward() throws Exception
306+
{
307+
_server.stop();
308+
_targetServletContextHandler.addServlet(VerifyForwardServlet.class, "/verify/*");
309+
_contextHandler.addServlet(CrossContextDispatchServlet.class, "/dispatch/*");
310+
_server.getContainedBeans(HttpConnectionFactory.class).forEach(f -> f.getHttpConfiguration().setUriCompliance(UriCompliance.DEFAULT.with("test", UriCompliance.Violation.AMBIGUOUS_PATH_ENCODING)));
311+
_server.start();
312+
313+
String rawResponse = _connector.getResponse("""
314+
GET /context/dispatch/?forward=/verify/%25%20test HTTP/1.1\r
315+
Host: localhost\r
316+
Connection: close\r
317+
\r
318+
""");
319+
320+
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
321+
322+
String content = response.getContent();
323+
String[] contentLines = content.split("\\n");
324+
325+
//verify forward attributes
326+
assertThat(content, containsString("Verified!"));
327+
assertThat(content, containsString("jakarta.servlet.forward.context_path=/context"));
328+
assertThat(content, containsString("jakarta.servlet.forward.servlet_path=/dispatch"));
329+
assertThat(content, containsString("jakarta.servlet.forward.path_info=/"));
330+
331+
String forwardMapping = extractLine(contentLines, "jakarta.servlet.forward.mapping=");
332+
assertNotNull(forwardMapping);
333+
assertThat(forwardMapping, containsString("CrossContextDispatchServlet"));
334+
assertThat(content, containsString("jakarta.servlet.forward.query_string=forward=/verify"));
335+
assertThat(content, containsString("jakarta.servlet.forward.request_uri=/context/dispatch/"));
336+
//verify request values
337+
assertThat(content, containsString("REQUEST_URL=http://localhost/foreign/"));
338+
assertThat(content, containsString("CONTEXT_PATH=/foreign"));
339+
assertThat(content, containsString("SERVLET_PATH=/verify"));
340+
assertThat(content, containsString("PATH_INFO=/% test/pinfo"));
341+
String mapping = extractLine(contentLines, "MAPPING=");
342+
assertNotNull(mapping);
343+
assertThat(mapping, containsString("VerifyForwardServlet"));
344+
String params = extractLine(contentLines, "PARAMS=");
345+
assertNotNull(params);
346+
params = params.substring(params.indexOf("=") + 1);
347+
params = params.substring(1, params.length() - 1); //dump leading, trailing [ ]
348+
assertThat(Arrays.asList(StringUtil.csvSplit(params)), containsInAnyOrder("a", "forward"));
349+
assertThat(content, containsString("REQUEST_URI=/foreign/verify/%25%20test/pinfo"));
350+
}
351+
301352
@Test
302353
public void testSimpleCrossContextInclude() throws Exception
303354
{
@@ -847,7 +898,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t
847898
{
848899
ServletContext foreign = getServletContext().getContext(ctx);
849900
assertNotNull(foreign);
850-
dispatcher = foreign.getRequestDispatcher(request.getParameter("forward") + "/pinfo?a=b");
901+
dispatcher = foreign.getRequestDispatcher(URIUtil.encodePath(request.getParameter("forward")) + "/pinfo?a=b");
851902

852903
if (dispatcher == null)
853904
response.sendError(404, "No dispatcher for forward");
@@ -1020,7 +1071,7 @@ public void service(ServletRequest req, ServletResponse res) throws ServletExcep
10201071
res.getWriter().println("----------- FORWARD ATTRIBUTES");
10211072
res.getWriter().println(RequestDispatcher.FORWARD_CONTEXT_PATH + "=" + req.getAttribute(RequestDispatcher.FORWARD_CONTEXT_PATH));
10221073
res.getWriter().println(RequestDispatcher.FORWARD_SERVLET_PATH + "=" + req.getAttribute(RequestDispatcher.FORWARD_SERVLET_PATH));
1023-
res.getWriter().println(RequestDispatcher.FORWARD_PATH_INFO + "=" + req.getAttribute(RequestDispatcher.FORWARD_PATH_INFO));
1074+
res.getWriter().println(RequestDispatcher.FORWARD_PATH_INFO + "=" + req.getAttribute(RequestDispatcher.FORWARD_PATH_INFO));
10241075
res.getWriter().println(RequestDispatcher.FORWARD_MAPPING + "=" + req.getAttribute(RequestDispatcher.FORWARD_MAPPING));
10251076
res.getWriter().println(RequestDispatcher.FORWARD_QUERY_STRING + "=" + req.getAttribute(RequestDispatcher.FORWARD_QUERY_STRING));
10261077
res.getWriter().println(RequestDispatcher.FORWARD_REQUEST_URI + "=" + req.getAttribute(RequestDispatcher.FORWARD_REQUEST_URI));

0 commit comments

Comments
 (0)