This is an automated email from the ASF dual-hosted git repository. radu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-xss.git
commit ec6764d165abc4df8cffd8439761bb2228887db9 Author: Radu Cotescu <[email protected]> AuthorDate: Thu Dec 28 13:58:30 2017 +0100 SLING-7323 - Optimise URL handling * added better decoding for URLs --- .../java/org/apache/sling/xss/impl/XSSAPIImpl.java | 47 ++++++++-------------- .../org/apache/sling/xss/impl/XSSAPIImplTest.java | 8 +++- 2 files changed, 22 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java b/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java index 4c62098..f0d35e1 100644 --- a/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java +++ b/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java @@ -18,9 +18,6 @@ package org.apache.sling.xss.impl; import java.io.StringReader; import java.io.StringWriter; -import java.io.UnsupportedEncodingException; -import java.net.URLDecoder; -import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; import java.util.regex.Matcher; @@ -32,7 +29,6 @@ import javax.json.JsonReaderFactory; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; -import org.apache.commons.lang3.StringEscapeUtils; import org.apache.commons.lang3.StringUtils; import org.apache.sling.xss.ProtectionContext; import org.apache.sling.xss.XSSAPI; @@ -228,33 +224,22 @@ public class XSSAPIImpl implements XSSAPI { @Nonnull public String getValidHref(final String url) { if (StringUtils.isNotEmpty(url)) { - try { - String unescapedURL = URLDecoder.decode(url, StandardCharsets.UTF_8.name()); - /* - StringEscapeUtils is deprecated starting with version 3.6 of commons-lang3, however the indicated replacement comes from - commons-text, which is not an OSGi bundle - */ - unescapedURL = StringEscapeUtils.unescapeXml(unescapedURL); - // Percent-encode characters that are not allowed in unquoted - // HTML attributes: ", ', >, <, ` and space. We don't encode = - // since this would break links with query parameters. - String encodedUrl = unescapedURL.replaceAll("\"", "%22") - .replaceAll("'", "%27") - .replaceAll(">", "%3E") - .replaceAll("<", "%3C") - .replaceAll("`", "%60") - .replaceAll(" ", "%20"); - int qMarkIx = encodedUrl.indexOf('?'); - if (qMarkIx > 0) { - encodedUrl = encodedUrl.substring(0, qMarkIx) + encodedUrl.substring(qMarkIx).replaceAll(":", "%3A"); - } - - encodedUrl = mangleNamespaces(encodedUrl); - if (xssFilter.isValidHref(encodedUrl)) { - return encodedUrl; - } - } catch (UnsupportedEncodingException e) { - LOGGER.error("Unable to decode url: {}.", url); + // Percent-encode characters that are not allowed in unquoted + // HTML attributes: ", ', >, <, ` and space. We don't encode = + // since this would break links with query parameters. + String encodedUrl = url.replaceAll("\"", "%22") + .replaceAll("'", "%27") + .replaceAll(">", "%3E") + .replaceAll("<", "%3C") + .replaceAll("`", "%60") + .replaceAll(" ", "%20"); + int qMarkIx = encodedUrl.indexOf('?'); + if (qMarkIx > 0) { + encodedUrl = encodedUrl.substring(0, qMarkIx) + encodedUrl.substring(qMarkIx).replaceAll(":", "%3A"); + } + encodedUrl = mangleNamespaces(encodedUrl); + if (xssFilter.isValidHref(encodedUrl)) { + return encodedUrl; } } // fall through to empty string diff --git a/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java b/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java index e0a8ba4..889fc1c 100644 --- a/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java +++ b/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java @@ -218,8 +218,12 @@ public class XSSAPIImplTest { // Href Expected Result // { + "test?discount=25%25", + "test?discount=25%25" + }, + { "/base?backHref=%26%23x6a%3b%26%23x61%3b%26%23x76%3b%26%23x61%3b%26%23x73%3b%26%23x63%3b%26%23x72%3b%26%23x69%3b%26%23x70%3b%26%23x74%3b%26%23x3a%3balert%281%29", - "/base?backHref=javascript%3Aalert(1)" + "" }, { "%26%23x6a%3b%26%23x61%3b%26%23x76%3b%26%23x61%3b%26%23x73%3b%26%23x63%3b%26%23x72%3b%26%23x69%3b%26%23x70%3b%26%23x74%3b%26%23x3a%3balert%281%29", @@ -229,7 +233,7 @@ public class XSSAPIImplTest { "javascript:alert(1)", "" }, - {"%2Fscripts%2Ftest.js", "/scripts/test.js"}, + {"%2Fscripts%2Ftest.js", "%2Fscripts%2Ftest.js"}, {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"}, {"/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995", "/etc/commerce/collections/\u09aa\u09b0\u09c0\u0995\u09cd\u09b7\u09be\u09ae\u09c2\u09b2\u0995"}, {null, ""}, -- To stop receiving notification emails like this one, please contact "[email protected]" <[email protected]>.
