You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "alexkachanov (via GitHub)" <gi...@apache.org> on 2023/05/24 00:10:15 UTC
[GitHub] [tomcat] alexkachanov opened a new pull request, #621: Bug 66609 - invalid XML in directory listing with file names containing "&" and "'"
alexkachanov opened a new pull request, #621:
URL: https://github.com/apache/tomcat/pull/621
* fix and junit test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] markt-asf closed pull request #621: Bug 66609 - invalid XML in directory listing with file names containing "&" and "'"
Posted by "markt-asf (via GitHub)" <gi...@apache.org>.
markt-asf closed pull request #621: Bug 66609 - invalid XML in directory listing with file names containing "&" and "'"
URL: https://github.com/apache/tomcat/pull/621
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on pull request #621: Bug 66609 - invalid XML in directory listing with file names containing "&" and "'"
Posted by "markt-asf (via GitHub)" <gi...@apache.org>.
markt-asf commented on PR #621:
URL: https://github.com/apache/tomcat/pull/621#issuecomment-1560920115
(Ab)using the URLEncoder for this seems wrong. Tomcat has a class specifically for escaping values in content - `org.apache.tomcat.util.security.Escape`. The fix should use this.
Further, it appears that there are still multiple implementations of XML escaping in the Tomcat code base. These need to be rationalised to use `Escape` class.
I agree having a test case for this is an improvement. I'd like to see that test case cover all the XML characters that need to be escaped and also test the WebDAV case as well as the directory listing case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] alexkachanov commented on a diff in pull request #621: Bug 66609 - invalid XML in directory listing with file names containing "&" and "'"
Posted by "alexkachanov (via GitHub)" <gi...@apache.org>.
alexkachanov commented on code in PR #621:
URL: https://github.com/apache/tomcat/pull/621#discussion_r1203195738
##########
test/org/apache/catalina/servlets/TestDefaultServlet.java:
##########
@@ -578,6 +578,44 @@ public void testBug57601() throws Exception {
tomcat.stop();
}
+
+ @Test
+ public void testBug66609() throws Exception {
Review Comment:
added test to test the fix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] alexkachanov commented on pull request #621: Bug 66609 - invalid XML in directory listing with file names containing "&" and "'"
Posted by "alexkachanov (via GitHub)" <gi...@apache.org>.
alexkachanov commented on PR #621:
URL: https://github.com/apache/tomcat/pull/621#issuecomment-1560963105
I was using URLEncoder class because it was used in original rewriteUrl method. So I wanted to keep it consistent and keep the changes it to minimum. I see that Escape.xml(String content) makes sense here. Let me change the patch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] alexkachanov commented on a diff in pull request #621: Bug 66609 - invalid XML in directory listing with file names containing "&" and "'"
Posted by "alexkachanov (via GitHub)" <gi...@apache.org>.
alexkachanov commented on code in PR #621:
URL: https://github.com/apache/tomcat/pull/621#discussion_r1203192773
##########
java/org/apache/catalina/util/URLEncoder.java:
##########
@@ -95,6 +96,8 @@ public final class URLEncoder implements Cloneable {
QUERY.addSafeCharacter('_');
QUERY.addSafeCharacter('=');
QUERY.addSafeCharacter('&');
+
+ XML.addSafeCharacter('/');
Review Comment:
"/" is a valid symbol
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] alexkachanov commented on a diff in pull request #621: Bug 66609 - invalid XML in directory listing with file names containing "&" and "'"
Posted by "alexkachanov (via GitHub)" <gi...@apache.org>.
alexkachanov commented on code in PR #621:
URL: https://github.com/apache/tomcat/pull/621#discussion_r1203991514
##########
java/org/apache/catalina/servlets/DefaultServlet.java:
##########
@@ -1650,7 +1650,7 @@ protected InputStream renderXml(HttpServletRequest request, String contextPath,
.append('\'');
sb.append(" urlPath='")
.append(rewrittenContextPath)
- .append(rewriteUrl(directoryWebappPath + entry))
+ .append(Escape.xml(directoryWebappPath + entry))
Review Comment:
using Escape.xml instead of rewriteUrl
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] alexkachanov commented on a diff in pull request #621: Bug 66609 - invalid XML in directory listing with file names containing "&" and "'"
Posted by "alexkachanov (via GitHub)" <gi...@apache.org>.
alexkachanov commented on code in PR #621:
URL: https://github.com/apache/tomcat/pull/621#discussion_r1203195493
##########
java/org/apache/catalina/servlets/DefaultServlet.java:
##########
@@ -1650,7 +1654,7 @@ protected InputStream renderXml(HttpServletRequest request, String contextPath,
.append('\'');
sb.append(" urlPath='")
.append(rewrittenContextPath)
- .append(rewriteUrl(directoryWebappPath + entry))
+ .append(rewriteUrl(directoryWebappPath + entry, URLEncoder.XML))
Review Comment:
using XML encoder to encode path attribute which may contain "'" and "&" symbols
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] alexkachanov commented on a diff in pull request #621: Bug 66609 - invalid XML in directory listing with file names containing "&" and "'"
Posted by "alexkachanov (via GitHub)" <gi...@apache.org>.
alexkachanov commented on code in PR #621:
URL: https://github.com/apache/tomcat/pull/621#discussion_r1203192504
##########
java/org/apache/catalina/util/URLEncoder.java:
##########
@@ -40,6 +40,7 @@ public final class URLEncoder implements Cloneable {
public static final URLEncoder DEFAULT = new URLEncoder();
public static final URLEncoder QUERY = new URLEncoder();
+ public static final URLEncoder XML = new URLEncoder();
Review Comment:
added new XML encoder that encodes strings that are valid in XML
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on pull request #621: Bug 66609 - invalid XML in directory listing with file names containing "&" and "'"
Posted by "markt-asf (via GitHub)" <gi...@apache.org>.
markt-asf commented on PR #621:
URL: https://github.com/apache/tomcat/pull/621#issuecomment-1561136901
Thanks for the PR. Applied manually with a few tweaks (e.g. CheckStyle config).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org