XSS protection path mangling issue: Bug or not?

[Georg Köster]

Hi all,

I stumbled over this behavior: Last part in path gets prepended with an 
underscore if there is a colon in the query string. Test appended, to be 
applied on  https://github.com/apache/sling/tree/196dea678c6010

Test output:

Failed tests:
  XSSAPIImplTest.testGetValidHref:267 Requested 
'/content/items/searchpages.html?0_tag:id=geo' 
expected:</content/items/[searchpages.html?0_tag%3a]id=geo> but 
was:</content/items/[_searchpages.html?0_tag_]id=geo>

Is this a bug? Should I create an issue ?

Cheers,

Georg
From e00a12f145fbebb6439f49bae3c3ee913f9c0042 Mon Sep 17 00:00:00 2001
From: Georg Koester <m...@georgkoester.de>
Date: Thu, 19 Mar 2015 15:45:22 +0100
Subject: [PATCH] Add testcases for getValidHref showing problem in colon
 handling.

Last element in path (searchpages.html) is modified (to
_searchpages.html) when there is a colon in the query string.
---
 .../java/org/apache/sling/xss/impl/XSSAPIImplTest.java   | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/contrib/extensions/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java b/contrib/extensions/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
index f1347ec..9bfaad2 100644
--- a/contrib/extensions/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
+++ b/contrib/extensions/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
@@ -16,10 +16,7 @@
  ******************************************************************************/
 package org.apache.sling.xss.impl;
 
-import java.io.FileInputStream;
-import java.io.InputStream;
-import java.lang.reflect.Field;
-
+import junit.framework.TestCase;
 import org.apache.sling.api.SlingHttpServletRequest;
 import org.apache.sling.api.resource.ResourceResolver;
 import org.apache.sling.xss.XSSAPI;
@@ -31,7 +28,9 @@ import org.owasp.validator.html.AntiSamy;
 import org.owasp.validator.html.Policy;
 import org.powermock.reflect.Whitebox;
 
-import junit.framework.TestCase;
+import java.io.FileInputStream;
+import java.io.InputStream;
+import java.lang.reflect.Field;
 
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.anyString;
@@ -252,6 +251,13 @@ public class XSSAPIImplTest {
                 // `
                 {"/test/ab`cd", "/test/ab%60cd"},
                 {"http://localhost:4502/test/ab`cd";, "http://localhost:4502/test/ab%60cd"},
+                // colons in query string
+                {"/content/items/searchpages.html?0_tag:id=geo", "/content/items/searchpages.html?0_tag%3aid=geo"},
+                // JCR namespaces and colons in query string
+                {
+                        "/content/items/jcr:content/searchpages.html?0_tag:id=geo",
+                        "/content/items/_jcr_content/searchpages.html?0_tag%3aid=geo"
+                },
         };
 
         for (String[] aTestData : testData) {
-- 
2.2.1