Improved: Check parameters passed in URLs (OFBIZ-13295)

Tests in ControlFilter.java did not work with previous commit.
This one fixes the issue

Conflicts handled by hand in ControlFilter.java

Author: JacquesLeRoux

framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java

@@ -160,6 +160,11 @@ private static List<String> getAllowedTokens() {
         return UtilValidate.isNotEmpty(allowedTokens) ? StringUtil.split(allowedTokens, ",") : new ArrayList<>();
     }
 
+    private static boolean isControlFilterTests() {
+        return null != System.getProperty("ControlFilterTests");
+    }
+
+
     /**
      * Makes allowed paths pass through while redirecting the others to a fix location.
      * Reject wrong URLs
@@ -171,7 +176,9 @@ public void doFilter(HttpServletRequest req, HttpServletResponse resp, FilterCha
 
         // Prevents stream exploitation
         if (!isSolrTest()) {
-            UrlServletHelper.setRequestAttributes(req, null, req.getServletContext());
+            if (!isControlFilterTests()) {
+                UrlServletHelper.setRequestAttributes(req, null, req.getServletContext());
+            }
             Map<String, Object> parameters = UtilHttp.getParameterMap(req);
             boolean reject = false;
             if (!parameters.isEmpty()) {

framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/ControlFilterTests.java

@@ -18,21 +18,21 @@
  */
 package org.apache.ofbiz.webapp.control;
 
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.anyString;
 import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.junit.Before;
+import org.junit.Test;
 
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
-import org.junit.Before;
-import org.junit.Test;
-
 public class ControlFilterTests {
 
     private FilterConfig config;
@@ -58,6 +58,7 @@ public void setUp() {
 
     @Test
     public void filterWithExactAllowedPath() throws Exception {
+        System.setProperty("ControlFilterTests", "runsAfterControlFilter");
         when(config.getInitParameter("redirectPath")).thenReturn("/foo");
         when(config.getInitParameter("allowedPaths")).thenReturn("/foo:/bar");
         when(req.getRequestURI()).thenReturn("/servlet/bar");
@@ -66,10 +67,12 @@ public void filterWithExactAllowedPath() throws Exception {
         filter.init(config);
         filter.doFilter(req, resp, next);
         verify(next).doFilter(req, resp);
+        System.clearProperty("ControlFilterTests");
     }
 
     @Test
     public void filterWithAllowedSubPath() throws Exception {
+        System.setProperty("ControlFilterTests", "runsAfterControlFilter");
         when(config.getInitParameter("redirectPath")).thenReturn("/foo");
         when(config.getInitParameter("allowedPaths")).thenReturn("/foo:/bar");
         when(req.getRequestURI()).thenReturn("/servlet/bar/baz");
@@ -78,32 +81,38 @@ public void filterWithAllowedSubPath() throws Exception {
         filter.init(config);
         filter.doFilter(req, resp, next);
         verify(next).doFilter(req, resp);
+        System.clearProperty("ControlFilterTests");
     }
 
     @Test
     public void filterWithRedirection() throws Exception {
+        System.setProperty("ControlFilterTests", "runsAfterControlFilter");
         when(config.getInitParameter("redirectPath")).thenReturn("/foo");
         when(config.getInitParameter("allowedPaths")).thenReturn("/bar:/baz");
         when(req.getRequestURI()).thenReturn("/missing/path");
 
         filter.init(config);
         filter.doFilter(req, resp, next);
         verify(resp).sendRedirect("/foo");
+        System.clearProperty("ControlFilterTests");
     }
 
     @Test
     public void filterWithURIredirection() throws Exception {
+        System.setProperty("ControlFilterTests", "runsAfterControlFilter");
         when(config.getInitParameter("redirectPath")).thenReturn("http://example.org/foo");
         when(config.getInitParameter("allowedPaths")).thenReturn("/foo:/bar");
         when(req.getRequestURI()).thenReturn("/baz");
 
         filter.init(config);
         filter.doFilter(req, resp, next);
         verify(resp).sendRedirect("http://example.org/foo");
+        System.clearProperty("ControlFilterTests");
     }
 
     @Test
     public void bailsOutWithVariousErrorCodes() throws Exception {
+        System.setProperty("ControlFilterTests", "runsAfterControlFilter");
         when(config.getInitParameter("allowedPaths")).thenReturn("/foo");
         when(req.getRequestURI()).thenReturn("/baz");
 
@@ -129,10 +138,12 @@ public void bailsOutWithVariousErrorCodes() throws Exception {
         filter.init(config);
         filter.doFilter(req, resp, next);
         verify(resp).sendError(404, "/baz");
+        System.clearProperty("ControlFilterTests");
     }
 
     @Test
     public void redirectAllAllowed() throws Exception {
+        System.setProperty("ControlFilterTests", "runsAfterControlFilter");
         when(config.getInitParameter("redirectPath")).thenReturn("/bar");
         when(config.getInitParameter("forceRedirectAll")).thenReturn("Y");
         when(config.getInitParameter("allowedPaths")).thenReturn("/foo");
@@ -141,10 +152,12 @@ public void redirectAllAllowed() throws Exception {
         filter.init(config);
         filter.doFilter(req, resp, next);
         verify(resp).sendRedirect("/bar");
+        System.clearProperty("ControlFilterTests");
     }
 
     @Test
     public void redirectAllNotAllowed() throws Exception {
+        System.setProperty("ControlFilterTests", "runsAfterControlFilter");
         when(config.getInitParameter("redirectPath")).thenReturn("/bar");
         when(config.getInitParameter("forceRedirectAll")).thenReturn("Y");
         when(config.getInitParameter("allowedPaths")).thenReturn("/foo");
@@ -153,10 +166,12 @@ public void redirectAllNotAllowed() throws Exception {
         filter.init(config);
         filter.doFilter(req, resp, next);
         verify(resp).sendRedirect("/bar");
+        System.clearProperty("ControlFilterTests");
     }
 
     @Test
     public void redirectAllRecursive() throws Exception {
+        System.setProperty("ControlFilterTests", "runsAfterControlFilter");
         when(config.getInitParameter("redirectPath")).thenReturn("/foo");
         when(config.getInitParameter("forceRedirectAll")).thenReturn("Y");
         when(config.getInitParameter("allowedPaths")).thenReturn("/foo");
@@ -174,5 +189,6 @@ public void redirectAllRecursive() throws Exception {
         filter.doFilter(req, resp, next);
         verify(next).doFilter(req, resp);
         verify(session).removeAttribute("_FORCE_REDIRECT_");
+        System.clearProperty("ControlFilterTests");
     }
 }