Merge pull request from GHSA-v7ff-8wcx-gmc5

Always normalize ambiguous URIs

Signed-off-by: Greg Wilkins <gregw@webtide.com>

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

@@ -1824,7 +1824,8 @@ public class Request implements HttpServletRequest
         setMethod(request.getMethod());
         HttpURI uri = request.getURI();
 
-        if (uri.isAmbiguous())
+        boolean ambiguous = uri.isAmbiguous();
+        if (ambiguous)
         {
             // replaced in jetty-10 with URICompliance from the HttpConfiguration
             Connection connection = _channel == null ? null : _channel.getConnection();
@@ -1852,6 +1853,13 @@ public class Request implements HttpServletRequest
         else if (encoded.startsWith("/"))
         {
             path = (encoded.length() == 1) ? "/" : uri.getDecodedPath();
+
+            // Strictly speaking if a URI is legal and encodes ambiguous segments, then they should be
+            // reflected in the decoded string version.  However, previous behaviour was to always normalize
+            // so we will continue to do so.  If an application wishes to see ambiguous URIs, then they can look
+            // at the encoded form of the URI
+            if (ambiguous)
+                path = URIUtil.canonicalPath(path);
         }
         else if ("*".equals(encoded) || HttpMethod.CONNECT.is(getMethod()))
         {

jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java

@@ -33,8 +33,10 @@ import javax.servlet.ServletContext;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 
+import org.eclipse.jetty.http.HttpCompliance;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpTester;
+import org.eclipse.jetty.server.HttpConnectionFactory;
 import org.eclipse.jetty.server.LocalConnector;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
@@ -247,6 +249,42 @@ public class WebAppContextTest
         assertFalse(context.isProtectedTarget("/something-else/web-inf"));
     }
 
+    @Test
+    public void testProtectedTarget() throws Exception
+    {
+        Server server = newServer();
+        server.getConnectors()[0].getConnectionFactory(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.LEGACY);
+
+        HandlerList handlers = new HandlerList();
+        ContextHandlerCollection contexts = new ContextHandlerCollection();
+        WebAppContext context = new WebAppContext();
+        Path testWebapp = MavenTestingUtils.getProjectDirPath("src/test/webapp");
+        context.setBaseResource(new PathResource(testWebapp));
+        context.setContextPath("/");
+        server.setHandler(handlers);
+        handlers.addHandler(contexts);
+        contexts.addHandler(context);
+
+        LocalConnector connector = new LocalConnector(server);
+        server.addConnector(connector);
+
+        server.start();
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET /test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200));
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2e/%2e/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200));
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET /foo/%2e%2e/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200));
+
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404));
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF/ HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404));
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404));
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET /web-inf/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404));
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2e/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404));
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2e/%2e/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404));
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET /foo/%2e%2e/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404));
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2E/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404));
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET //WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404));
+        assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF%2ftest.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404));
+    }
+
     @Test
     public void testNullPath() throws Exception
     {

jetty-webapp/src/test/webapp/WEB-INF/test.xml

@@ -0,0 +1 @@
+test

jetty-webapp/src/test/webapp/test.xml

@@ -0,0 +1 @@
+test