markt-asf commented on code in PR #923:
URL: https://github.com/apache/tomcat/pull/923#discussion_r2546571736


##########
test/org/apache/catalina/filters/TestCsrfPreventionFilter.java:
##########
@@ -196,6 +198,98 @@ public void testNoNonceMimeMatcher() {
         Assert.assertEquals("/foo/images/home.jpg", 
response.encodeURL("/foo/images/home.jpg"));
     }
 
+    @Test
+    public void testMultipleTokens() {
+        String nonce = "TESTNONCE";
+        String testURL = "/foo/bar?" + Constants.CSRF_NONCE_SESSION_ATTR_NAME 
+ "=sample";
+        CsrfPreventionFilter.CsrfResponseWrapper response = new 
CsrfPreventionFilter.CsrfResponseWrapper(new NonEncodingResponse(),
+                Constants.CSRF_NONCE_SESSION_ATTR_NAME, nonce, null);
+
+        Assert.assertTrue("Original URL does not contain CSRF token",
+                testURL.contains(Constants.CSRF_NONCE_SESSION_ATTR_NAME));
+
+        String result = response.encodeURL(testURL);
+
+        int pos = result.indexOf(Constants.CSRF_NONCE_SESSION_ATTR_NAME);
+        Assert.assertTrue("Result URL does not contain CSRF token",
+                pos >= 0);
+        pos = result.indexOf(Constants.CSRF_NONCE_SESSION_ATTR_NAME, pos + 1);
+        Assert.assertFalse("Result URL contains multiple CSRF tokens: " + 
result,
+                pos >= 0);
+    }
+
+    @Test
+    public void testURLCleansing() {
+        String[] urls = new String[] {
+                "/foo/bar",
+                "/foo/bar?",
+                "/foo/bar?csrf",
+                "/foo/bar?csrf&",
+                "/foo/bar?csrf=",
+                "/foo/bar?csrf=&",
+                "/foo/bar?csrf=abc",
+                "/foo/bar?csrf=abc&bar=foo",
+                "/foo/bar?bar=foo&csrf=abc",
+                "/foo/bar?bar=foo&csrf=abc&foo=bar",
+                "/foo/bar?csrfx=foil&bar=foo&csrf=abc&foo=bar",
+                "/foo/bar?csrfx=foil&bar=foo&csrf=abc&foo=bar&csrf=def",
+                "/foo/bar?csrf=&csrf&csrf&csrf&csrf=abc&csrf=",
+                "/foo/bar?xcsrf=&xcsrf&xcsrf&xcsrf&xcsrf=abc&xcsrf=",
+                
"/foo/bar?xcsrf=&xcsrf&xcsrf&csrf=foo&xcsrf&xcsrf=abc&csrf=bar&xcsrf=&",
+        };
+
+        String csrfParameterName = "csrf";
+
+        for(String url : urls) {
+            String result = 
CsrfPreventionFilter.CsrfResponseWrapper.removeQueryParameters(url, 
csrfParameterName);
+
+            Assert.assertEquals("Failed to cleanse URL '" + url + "' 
properly", dumbButAccurateCleanse(url, csrfParameterName), result);
+        }
+
+    }
+
+    private static String dumbButAccurateCleanse(String url, String 
csrfParameterName) {

Review Comment:
   This doesn't handle `/foo/bar?bar=fo?&csrf=abc` correctly but the actual 
CSRF code does so not a big deal.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to