Skip to content

Commit

Permalink
SLING-7323 - Optimise URL handling
Browse files Browse the repository at this point in the history
* added better decoding for URLs
  • Loading branch information
raducotescu committed Dec 28, 2017
1 parent 11011d5 commit ec6764d
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 33 deletions.
47 changes: 16 additions & 31 deletions src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@

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;
Expand All @@ -32,7 +29,6 @@
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;
Expand Down Expand Up @@ -228,33 +224,22 @@ private String mangleNamespaces(String absPath) {
@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
Expand Down
8 changes: 6 additions & 2 deletions src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,13 @@ public void testGetValidHref() {
String[][] testData = {
// 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",
Expand All @@ -229,7 +233,7 @@ public void testGetValidHref() {
"&#x6a;&#x61;&#x76;&#x61;&#x73;&#x63;&#x72;&#x69;&#x70;&#x74;&#x3a;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, ""},
Expand Down

0 comments on commit ec6764d

Please sign in to comment.