You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2023/05/24 13:19:47 UTC

[tomcat] branch main updated (63461b3bed -> 9a6bc65e50)

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


    from 63461b3bed Code clean-up. Formatting. No functional change.
     new bc161f3204 Fix BZ 66609. Correctly escape XML directory listings
     new 9a6bc65e50 Refactor WebDAV servlet escaping for XML. Add test case.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 build.xml                                          |  4 +-
 .../apache/catalina/servlets/DefaultServlet.java   |  2 +-
 .../apache/catalina/servlets/WebdavServlet.java    | 22 ------
 java/org/apache/catalina/util/XMLWriter.java       |  4 +-
 .../catalina/servlets/TestDefaultServlet.java      | 34 ++++++++
 .../catalina/servlets/TestWebdavServlet.java       | 48 ++++++++++++
 test/webapp/bug66609/_listing.xslt                 | 90 ++++++++++++++++++++++
 .../{bug53257/foo bar.txt => bug66609/a&a.txt}     |  0
 .../{bug53257/foo bar.txt => bug66609/b'b.txt}     |  0
 .../foo bar.txt => "test/webapp/bug66609/c\"c.txt" |  0
 .../{bug53257/foo bar.txt => bug66609/d<d.txt}     |  0
 .../{bug53257/foo bar.txt => bug66609/e>e.txt}     |  0
 webapps/docs/changelog.xml                         |  5 ++
 13 files changed, 184 insertions(+), 25 deletions(-)
 create mode 100644 test/webapp/bug66609/_listing.xslt
 copy test/webapp/{bug53257/foo bar.txt => bug66609/a&a.txt} (100%)
 copy test/webapp/{bug53257/foo bar.txt => bug66609/b'b.txt} (100%)
 copy test/webapp/bug53257/foo bar.txt => "test/webapp/bug66609/c\"c.txt" (100%)
 copy test/webapp/{bug53257/foo bar.txt => bug66609/d<d.txt} (100%)
 copy test/webapp/{bug53257/foo bar.txt => bug66609/e>e.txt} (100%)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 01/02: Fix BZ 66609. Correctly escape XML directory listings

Posted by ma...@apache.org.
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 bc161f3204b41f5f580d4ff4466b25aaea073c4e
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed May 24 14:11:32 2023 +0100

    Fix BZ 66609. Correctly escape XML directory listings
    
    Based on #621 by Alex Kachanov
---
 build.xml                                          |  4 +-
 .../apache/catalina/servlets/DefaultServlet.java   |  2 +-
 .../catalina/servlets/TestDefaultServlet.java      | 34 ++++++++
 test/webapp/bug66609/_listing.xslt                 | 90 ++++++++++++++++++++++
 test/webapp/bug66609/a&a.txt                       |  1 +
 test/webapp/bug66609/b'b.txt                       |  1 +
 "test/webapp/bug66609/c\"c.txt"                    |  1 +
 test/webapp/bug66609/d<d.txt                       |  1 +
 test/webapp/bug66609/e>e.txt                       |  1 +
 webapps/docs/changelog.xml                         |  5 ++
 10 files changed, 138 insertions(+), 2 deletions(-)

diff --git a/build.xml b/build.xml
index ab0021e3e2..f189b100f6 100644
--- a/build.xml
+++ b/build.xml
@@ -864,6 +864,7 @@
         <exclude name="test/webapp-fragments/WEB-INF/classes/*.txt"/>
         <exclude name="test/webapp/bug49nnn/*.txt"/>
         <exclude name="test/webapp/bug53257/**/*.txt"/>
+        <exclude name="test/webapp/bug66609/*.txt"/>
         <exclude name="test/webresources/**/*.txt"/>
         <exclude name="**/*.mdl"/>
         <exclude name="**/*.pem"/>
@@ -912,8 +913,9 @@
         <exclude name=".*/**"/>
         <exclude name="**/*.pem"/>
         <!-- Exclude simple test files -->
-        <exclude name="webapp/bug53257/**/*.txt"/>
         <exclude name="webapp/bug49nnn/bug49464*"/>
+        <exclude name="webapp/bug53257/**/*.txt"/>
+        <exclude name="webapp/bug66609/*.txt"/>
         <exclude name="webapp-fragments/WEB-INF/classes/*.txt"/>
         <exclude name="webresources/**"/>
         <!-- Exclude test files with unusual encodings -->
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java
index 83e0aa142c..5c86b21170 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -1650,7 +1650,7 @@ public class DefaultServlet extends HttpServlet {
               .append('\'');
             sb.append(" urlPath='")
               .append(rewrittenContextPath)
-              .append(rewriteUrl(directoryWebappPath + entry))
+              .append(Escape.xml(rewriteUrl(directoryWebappPath + entry)))
               .append(childResource.isDirectory()?"/":"")
               .append('\'');
             if (childResource.isFile()) {
diff --git a/test/org/apache/catalina/servlets/TestDefaultServlet.java b/test/org/apache/catalina/servlets/TestDefaultServlet.java
index a185aa9586..b67cc325d4 100644
--- a/test/org/apache/catalina/servlets/TestDefaultServlet.java
+++ b/test/org/apache/catalina/servlets/TestDefaultServlet.java
@@ -608,4 +608,38 @@ public class TestDefaultServlet extends TomcatBaseTest {
             return true;
         }
     }
+
+    /*
+     * Bug 66609
+     */
+    @Test
+    public void testXmlDirectoryListing() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = new File("test/webapp");
+        Context ctxt = tomcat.addContext("", appDir.getAbsolutePath());
+
+        Wrapper defaultServlet = Tomcat.addServlet(ctxt, "default", new DefaultServlet());
+        defaultServlet.addInitParameter("listings", "true");
+        defaultServlet.addInitParameter("localXsltFile", "_listing.xslt");
+
+        ctxt.addServletMappingDecoded("/", "default");
+
+        tomcat.start();
+
+        Map<String,List<String>> resHeaders= new HashMap<>();
+        String path = "http://localhost:" + getPort() + "/bug66609/";
+        ByteChunk out = new ByteChunk();
+
+        int rc = getUrl(path, out, resHeaders);
+        Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+        String length = resHeaders.get("Content-Length").get(0);
+        Assert.assertEquals(Long.parseLong(length), out.getLength());
+        out.recycle();
+
+        rc = headUrl(path, out, resHeaders);
+        Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+        Assert.assertEquals(0, out.getLength());
+        Assert.assertEquals(length, resHeaders.get("Content-Length").get(0));
+    }
 }
diff --git a/test/webapp/bug66609/_listing.xslt b/test/webapp/bug66609/_listing.xslt
new file mode 100644
index 0000000000..ee132052b1
--- /dev/null
+++ b/test/webapp/bug66609/_listing.xslt
@@ -0,0 +1,90 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+
+<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
+  version="3.0">
+
+  <xsl:output method="html" html-version="5.0"
+    encoding="UTF-8" indent="no"
+    doctype-system="about:legacy-compat"/>
+
+  <xsl:template match="listing">
+   <html>
+    <head>
+      <title>
+        Sample Directory Listing For
+        <xsl:value-of select="@directory"/>
+      </title>
+      <style>
+        h1 {color : white;background-color : #0086b2;}
+        h3 {color : white;background-color : #0086b2;}
+        body {font-family : sans-serif,Arial,Tahoma;
+             color : black;background-color : white;}
+        b {color : white;background-color : #0086b2;}
+        a {color : black;} HR{color : #0086b2;}
+        table td { padding: 5px; }
+      </style>
+    </head>
+    <body>
+      <h1>Sample Directory Listing For
+            <xsl:value-of select="@directory"/>
+      </h1>
+      <hr style="height: 1px;" />
+      <table style="width: 100%;">
+        <tr>
+          <th style="text-align: left;">Filename</th>
+          <th style="text-align: center;">Size</th>
+          <th style="text-align: right;">Last Modified</th>
+        </tr>
+        <xsl:apply-templates select="entries"/>
+        </table>
+      <xsl:apply-templates select="readme"/>
+      <hr style="height: 1px;" />
+      <h3>Apache Tomcat/11.0</h3>
+    </body>
+   </html>
+  </xsl:template>
+
+
+  <xsl:template match="entries">
+    <xsl:apply-templates select="entry"/>
+  </xsl:template>
+
+  <xsl:template match="readme">
+    <hr style="height: 1px;" />
+    <pre><xsl:apply-templates/></pre>
+  </xsl:template>
+
+  <xsl:template match="entry">
+    <tr>
+      <td style="text-align: left;">
+        <xsl:variable name="urlPath" select="@urlPath"/>
+        <a href="{$urlPath}">
+          <pre><xsl:apply-templates/></pre>
+        </a>
+      </td>
+      <td style="text-align: right;">
+        <pre><xsl:value-of select="@size"/></pre>
+      </td>
+      <td style="text-align: right;">
+        <pre><xsl:value-of select="@date"/></pre>
+      </td>
+    </tr>
+  </xsl:template>
+
+</xsl:stylesheet>
\ No newline at end of file
diff --git a/test/webapp/bug66609/a&a.txt b/test/webapp/bug66609/a&a.txt
new file mode 100644
index 0000000000..a0aba9318a
--- /dev/null
+++ b/test/webapp/bug66609/a&a.txt
@@ -0,0 +1 @@
+OK
\ No newline at end of file
diff --git a/test/webapp/bug66609/b'b.txt b/test/webapp/bug66609/b'b.txt
new file mode 100644
index 0000000000..a0aba9318a
--- /dev/null
+++ b/test/webapp/bug66609/b'b.txt
@@ -0,0 +1 @@
+OK
\ No newline at end of file
diff --git "a/test/webapp/bug66609/c\"c.txt" "b/test/webapp/bug66609/c\"c.txt"
new file mode 100644
index 0000000000..a0aba9318a
--- /dev/null
+++ "b/test/webapp/bug66609/c\"c.txt"
@@ -0,0 +1 @@
+OK
\ No newline at end of file
diff --git a/test/webapp/bug66609/d<d.txt b/test/webapp/bug66609/d<d.txt
new file mode 100644
index 0000000000..a0aba9318a
--- /dev/null
+++ b/test/webapp/bug66609/d<d.txt
@@ -0,0 +1 @@
+OK
\ No newline at end of file
diff --git a/test/webapp/bug66609/e>e.txt b/test/webapp/bug66609/e>e.txt
new file mode 100644
index 0000000000..a0aba9318a
--- /dev/null
+++ b/test/webapp/bug66609/e>e.txt
@@ -0,0 +1 @@
+OK
\ No newline at end of file
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index e37c67094b..98ae2b5b2d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -135,6 +135,11 @@
         Connectors to process requests received by those Connectors using
         virtual threads. (markt)
       </add>
+      <fix>
+        <bug>66609</bug>: Ensure that the default servlet correctly escapes
+        file names in directory listings when using XML output. Based on pull
+        request <pr>621</pr> by Alex Kachanov. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 02/02: Refactor WebDAV servlet escaping for XML. Add test case.

Posted by ma...@apache.org.
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 9a6bc65e50df5b8d138b5373e48575d043981c24
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed May 24 14:16:13 2023 +0100

    Refactor WebDAV servlet escaping for XML. Add test case.
---
 .../apache/catalina/servlets/WebdavServlet.java    | 22 ----------
 java/org/apache/catalina/util/XMLWriter.java       |  4 +-
 .../catalina/servlets/TestWebdavServlet.java       | 48 ++++++++++++++++++++++
 3 files changed, 51 insertions(+), 23 deletions(-)

diff --git a/java/org/apache/catalina/servlets/WebdavServlet.java b/java/org/apache/catalina/servlets/WebdavServlet.java
index 096ed5a84f..38448532aa 100644
--- a/java/org/apache/catalina/servlets/WebdavServlet.java
+++ b/java/org/apache/catalina/servlets/WebdavServlet.java
@@ -52,7 +52,6 @@ import jakarta.servlet.http.HttpServletResponse;
 import org.apache.catalina.WebResource;
 import org.apache.catalina.connector.RequestFacade;
 import org.apache.catalina.util.DOMWriter;
-import org.apache.catalina.util.URLEncoder;
 import org.apache.catalina.util.XMLWriter;
 import org.apache.tomcat.util.buf.HexUtils;
 import org.apache.tomcat.util.http.ConcurrentDateFormat;
@@ -140,15 +139,6 @@ public class WebdavServlet extends DefaultServlet {
 
     // -------------------------------------------------------------- Constants
 
-    private static final URLEncoder URL_ENCODER_XML;
-    static {
-        URL_ENCODER_XML = (URLEncoder) URLEncoder.DEFAULT.clone();
-        // Remove '&' from the safe character set since while it it permitted
-        // in a URI path, it is not permitted in XML and encoding it is a simple
-        // way to address this.
-        URL_ENCODER_XML.removeSafeCharacter('&');
-    }
-
     private static final String METHOD_PROPFIND = "PROPFIND";
     private static final String METHOD_PROPPATCH = "PROPPATCH";
     private static final String METHOD_MKCOL = "MKCOL";
@@ -390,18 +380,6 @@ public class WebdavServlet extends DefaultServlet {
     }
 
 
-    /**
-     * URL rewriter.
-     *
-     * @param path Path which has to be rewritten
-     * @return the rewritten path
-     */
-    @Override
-    protected String rewriteUrl(String path) {
-        return URL_ENCODER_XML.encode(path, StandardCharsets.UTF_8);
-    }
-
-
     /**
      * Override the DefaultServlet implementation and only use the PathInfo. If
      * the ServletPath is non-null, it will be because the WebDAV servlet has
diff --git a/java/org/apache/catalina/util/XMLWriter.java b/java/org/apache/catalina/util/XMLWriter.java
index 019c4b98cd..8290b73943 100644
--- a/java/org/apache/catalina/util/XMLWriter.java
+++ b/java/org/apache/catalina/util/XMLWriter.java
@@ -19,6 +19,8 @@ package org.apache.catalina.util;
 import java.io.IOException;
 import java.io.Writer;
 
+import org.apache.tomcat.util.security.Escape;
+
 /**
  * XMLWriter helper class.
  */
@@ -199,7 +201,7 @@ public class XMLWriter {
      * @param text Text to append
      */
     public void writeText(String text) {
-        buffer.append(text);
+        buffer.append(Escape.xml(text));
     }
 
 
diff --git a/test/org/apache/catalina/servlets/TestWebdavServlet.java b/test/org/apache/catalina/servlets/TestWebdavServlet.java
index fdf61a0e11..0ce1683917 100644
--- a/test/org/apache/catalina/servlets/TestWebdavServlet.java
+++ b/test/org/apache/catalina/servlets/TestWebdavServlet.java
@@ -18,18 +18,25 @@ package org.apache.catalina.servlets;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.StringReader;
 import java.util.List;
 import java.util.Map;
 
+import javax.xml.parsers.SAXParserFactory;
+
 import jakarta.servlet.http.HttpServletResponse;
 
 import org.junit.Assert;
 import org.junit.Test;
 
+import org.apache.catalina.Context;
+import org.apache.catalina.Wrapper;
+import org.apache.catalina.startup.SimpleHttpClient;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
 import org.apache.tomcat.util.buf.ByteChunk;
 import org.apache.tomcat.websocket.server.WsContextListener;
+import org.xml.sax.InputSource;
 
 public class TestWebdavServlet extends TomcatBaseTest {
 
@@ -149,4 +156,45 @@ public class TestWebdavServlet extends TomcatBaseTest {
         return TomcatBaseTest.getUrl(path, out, resHead);
     }
 
+    /*
+     * Bug 66609
+     */
+    @Test
+    public void testDirectoryListing() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = new File("test/webapp");
+        Context ctxt = tomcat.addContext("", appDir.getAbsolutePath());
+
+        Wrapper defaultServlet = Tomcat.addServlet(ctxt, "webdav", new WebdavServlet());
+        defaultServlet.addInitParameter("listings", "true");
+
+        ctxt.addServletMappingDecoded("/*", "webdav");
+        ctxt.addMimeMapping("html", "text/html");
+
+        tomcat.start();
+
+        Client client = new Client();
+        client.setPort(getPort());
+        client.setRequest(new String[] { "PROPFIND /bug66609/ HTTP/1.1" + SimpleHttpClient.CRLF +
+                                         "Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
+                                         SimpleHttpClient.CRLF});
+        client.connect();
+        client.sendRequest();
+
+        client.setUseContentLength(true);
+        client.readResponse(true);
+
+        // This will throw an exception if the XML is not valid
+        SAXParserFactory.newInstance().newSAXParser().getXMLReader().parse(new InputSource(new StringReader(client.getResponseBody())));
+    }
+
+
+    private static final class Client extends SimpleHttpClient {
+
+        @Override
+        public boolean isResponseBodyOK() {
+            return true;
+        }
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org