This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fee1f457f287a56d3d490a5ab5b3f643d280ecf5 Author: Mark Thomas <[email protected]> AuthorDate: Wed Oct 13 18:29:30 2021 +0100 Add tests cases for URI canonicalization from the Servlet spec --- .../TestCoyoteAdapterCanonicalization.java | 235 +++++++++++++++++++++ .../apache/catalina/startup/SimpleHttpClient.java | 17 +- 2 files changed, 247 insertions(+), 5 deletions(-) diff --git a/test/org/apache/catalina/connector/TestCoyoteAdapterCanonicalization.java b/test/org/apache/catalina/connector/TestCoyoteAdapterCanonicalization.java new file mode 100644 index 0000000..710e0c7 --- /dev/null +++ b/test/org/apache/catalina/connector/TestCoyoteAdapterCanonicalization.java @@ -0,0 +1,235 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.connector; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; + +import static org.apache.catalina.startup.SimpleHttpClient.CRLF; +import org.apache.catalina.Context; +import org.apache.catalina.startup.SimpleHttpClient; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; + +/* + * Tests for the canonicalization clarifications in Servlet 6.0 + */ +@RunWith(Parameterized.class) +public class TestCoyoteAdapterCanonicalization extends TomcatBaseTest { + + @Parameterized.Parameters(name = "{index}: requestURI[{0}]") + public static Collection<Object[]> parameters() { + List<Object[]> parameterSets = new ArrayList<>(); + + // This should be consistent with the table in the Servlet specification + parameterSets.add(new Object[] { "foo/bar", "/foo/bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/bar", "/foo/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/bar;jsessionid=1234", "/foo/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/bar/", "/foo/bar/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/bar/;jsessionid=1234", "/foo/bar/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo;/bar;", "/foo/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo;/bar;/;", "/foo/bar/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo%00/bar/", "/foo\000/bar/", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo%7Fbar", "/foo\177bar", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo%2Fbar", "/foo%2Fbar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo%2Fb%25r", "/foo%2Fb%25r", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/b%25r", "/foo/b%r", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo\\bar", "/foo\\bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo%5Cbar", "/foo\\bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo;%2F/bar", "/foo/bar", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/./bar", "/foo/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/././bar", "/foo/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/./foo/bar", "/foo/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/%2e/bar", "/foo/bar", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/.;/bar", "/foo/bar", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/%2e;/bar", "/foo/bar", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/.%2Fbar", "/foo/.%2Fbar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/.%5Cbar", "/foo/.\\bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/bar/.", "/foo/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/bar/./", "/foo/bar/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/bar/.;", "/foo/bar", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/bar/./;", "/foo/bar/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/.bar", "/foo/.bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/../bar", "/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/../../bar", "/../bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/../foo/bar", "/../foo/bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/%2e%2E/bar", "/bar", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/%2e%2e/%2E%2E/bar", "/../bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/./../bar", "/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/..;/bar", "/bar", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/%2e%2E;/bar", "/bar", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/..%2Fbar", "/foo/..%2Fbar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/..%5Cbar", "/foo/..\\bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/bar/..", "/foo", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/bar/../", "/foo/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/bar/..;", "/foo", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/bar/../;", "/foo/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/..bar", "/foo/..bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/.../bar", "/foo/.../bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo//bar", "/foo/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "//foo//bar//", "/foo/bar/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/;/foo;/;/bar/;/;", "/foo/bar/", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo//../bar", "/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/;/../bar", "/bar", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo%E2%82%ACbar", "/foo€bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo%20bar", "/foo bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo%E2%82", "/foo%E2%82", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo%E2%82bar", "/foo%E2%82bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo%-1/bar", "/foo%-1/bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo%XX/bar", "/foo%XX/bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo%/bar", "/foo%/bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/bar%0", "/foo/bar%0", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/good%20/bad%/%20mix%", "/good /bad%/%20mix%", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/bar?q", "/foo/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/bar#f", "/foo/bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/bar?q#f", "/foo/bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/bar/?q", "/foo/bar/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/bar/#f", "/foo/bar/", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/bar/?q#f", "/foo/bar/", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/bar;?q", "/foo/bar", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/foo/bar;#f", "/foo/bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/foo/bar;?q#f", "/foo/bar", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/", "/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "//", "/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/;/", "/", Boolean.TRUE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/.", "/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/..", "/..", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/./", "/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "/../", "/../", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "foo/bar/", "/foo/bar/", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "./foo/bar/", "/foo/bar/", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "%2e/foo/bar/", "/foo/bar/", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "../foo/bar/", "/../foo/bar/", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { ".%2e/foo/bar/", "/../foo/bar/", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { ";/foo/bar/", "/foo/bar/", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/#f", "/", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "#f", "/", Boolean.TRUE, Boolean.TRUE }); + parameterSets.add(new Object[] { "/?q", "/", Boolean.FALSE, Boolean.FALSE }); + parameterSets.add(new Object[] { "?q", "/", Boolean.TRUE, Boolean.TRUE }); + + return parameterSets; + } + + @Parameter(0) + public String requestURI; + + @Parameter(1) + public String canonicalizedURI; + + @Parameter(2) + public boolean badRequestServlet; + + @Parameter(3) + public boolean badRequestTomcat; + + + @Test + public void testCanonicalizationSpecification() throws Exception { + doTestCanonicalization(true, badRequestServlet); + } + + @Test + public void testCanonicalizationTomcat() throws Exception { + doTestCanonicalization(false, badRequestTomcat); + } + + + private void doTestCanonicalization(boolean specCompliant, boolean expectBadRequest) throws Exception { + Tomcat tomcat = getTomcatInstance(); + // ROOT web application so context path is "" + Context root = tomcat.addContext("", TEMP_DIR); + Tomcat.addServlet(root, "EchoServletPath", new EchoServletPath()); + // Map as default servlet so servlet path should be URI less context + // path. Since the content path is "" the servlet path should be the + // URI. + root.addServletMappingDecoded("/", "EchoServletPath"); + + if (specCompliant) { + // Enabled options for stricter checking + tomcat.getConnector().setRejectSuspiciousURIs(true); + } + + tomcat.start(); + + Client client = new Client(tomcat.getConnector().getLocalPort(), canonicalizedURI); + client.setRequest(new String[] { + "GET " + requestURI + " HTTP/1.1" + CRLF + + "Host: localhost" + CRLF + + CRLF + }); + client.setResponseBodyEncoding(StandardCharsets.UTF_8); + + client.connect(); + client.processRequest(); + + // Expected response + String line = client.getResponseLine(); + String body = client.getResponseBody(); + if (expectBadRequest) { + Assert.assertTrue(line + CRLF + body, line.startsWith("HTTP/1.1 " + HttpServletResponse.SC_BAD_REQUEST)); + } else { + Assert.assertTrue(line + CRLF + body, line.startsWith("HTTP/1.1 " + HttpServletResponse.SC_OK)); + Assert.assertTrue(line + CRLF + body, body.equals(canonicalizedURI)); + } + } + + + public class EchoServletPath extends HttpServlet { + + private static final long serialVersionUID = 1L; + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + resp.setContentType("text/plain"); + resp.setCharacterEncoding("UTF-8"); + resp.getWriter().write(req.getServletPath()); + } + } + + + private static final class Client extends SimpleHttpClient { + + private final String expected; + + public Client(int port, String expected) { + this.expected = expected; + setPort(port); + setRequestPause(0); + setUseContentLength(true); + } + + @Override + public boolean isResponseBodyOK() { + return expected.equals(getResponseBody()); + } + } +} diff --git a/test/org/apache/catalina/startup/SimpleHttpClient.java b/test/org/apache/catalina/startup/SimpleHttpClient.java index b0ba161..2282823 100644 --- a/test/org/apache/catalina/startup/SimpleHttpClient.java +++ b/test/org/apache/catalina/startup/SimpleHttpClient.java @@ -29,6 +29,8 @@ import java.net.Socket; import java.net.SocketAddress; import java.net.SocketException; import java.net.UnknownHostException; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import java.util.Locale; @@ -96,6 +98,7 @@ public abstract class SimpleHttpClient { private boolean useCookies = true; private boolean useHttp09 = false; private int requestPause = 1000; + private Charset requestBodyEncoding = StandardCharsets.ISO_8859_1; private String responseLine; private List<String> responseHeaders = new ArrayList<>(); @@ -106,6 +109,7 @@ public abstract class SimpleHttpClient { private String responseBody; private List<String> bodyUriElements = null; + private Charset responseBodyEncoding = StandardCharsets.ISO_8859_1; public void setPort(int thePort) { port = thePort; @@ -150,6 +154,10 @@ public abstract class SimpleHttpClient { return responseHeaders; } + public void setResponseBodyEncoding(Charset charset) { + responseBodyEncoding = charset; + } + public String getResponseBody() { return responseBody; } @@ -187,15 +195,14 @@ public abstract class SimpleHttpClient { public void connect(int connectTimeout, int soTimeout) throws UnknownHostException, IOException { - final String encoding = "ISO-8859-1"; SocketAddress addr = new InetSocketAddress("localhost", port); socket = new Socket(); socket.setSoTimeout(soTimeout); socket.connect(addr,connectTimeout); OutputStream os = createOutputStream(socket); - writer = new OutputStreamWriter(os, encoding); + writer = new OutputStreamWriter(os, requestBodyEncoding); InputStream is = socket.getInputStream(); - Reader r = new InputStreamReader(is, encoding); + Reader r = new InputStreamReader(is, responseBodyEncoding); reader = new BufferedReader(r); } public void connect() throws UnknownHostException, IOException { @@ -316,8 +323,8 @@ public abstract class SimpleHttpClient { if (useContentLength && (contentLength > -1)) { char[] body = new char[contentLength]; int read = reader.read(body); - Assert.assertEquals(contentLength, read); - builder.append(body); + builder.append(body, 0 , read); + Assert.assertEquals(contentLength, builder.toString().getBytes(responseBodyEncoding).length); } else { // not using content length, so just read it line by line String line = null; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
