This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new ff547c2198 Improved: Check parameters passed in URLs (OFBIZ-13295)
ff547c2198 is described below
commit ff547c21983c525ab722fc2a1b9ba0ef422d55ca
Author: Jacques Le Roux <[email protected]>
AuthorDate: Sat Oct 11 10:39:04 2025 +0200
Improved: Check parameters passed in URLs (OFBIZ-13295)
Tests in ControlFilter.java did not work with previous commit.
This one fixes the issue
---
.../apache/ofbiz/webapp/control/ControlFilter.java | 10 +++++++-
.../ofbiz/webapp/control/ControlFilterTests.java | 28 +++++++++++++++++-----
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java
index da80c8b9c5..a41b928e6c 100644
---
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java
+++
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java
@@ -156,6 +156,11 @@ public class ControlFilter extends HttpFilter {
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
@@ -166,7 +171,10 @@ public class ControlFilter extends HttpFilter {
HttpSession session = req.getSession();
// Prevents stream exploitation
- 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()) {
diff --git
a/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/ControlFilterTests.java
b/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/ControlFilterTests.java
index d1acfe9d26..d4921370bf 100644
---
a/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/ControlFilterTests.java
+++
b/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/ControlFilterTests.java
@@ -18,11 +18,14 @@
*/
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 jakarta.servlet.FilterChain;
import jakarta.servlet.FilterConfig;
@@ -30,9 +33,6 @@ import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpSession;
-import org.junit.Before;
-import org.junit.Test;
-
public class ControlFilterTests {
private FilterConfig config;
@@ -58,6 +58,7 @@ public class ControlFilterTests {
@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 class ControlFilterTests {
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,10 +81,12 @@ public class ControlFilterTests {
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");
@@ -89,10 +94,12 @@ public class ControlFilterTests {
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");
@@ -100,10 +107,12 @@ public class ControlFilterTests {
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 class ControlFilterTests {
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 class ControlFilterTests {
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 class ControlFilterTests {
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 class ControlFilterTests {
filter.doFilter(req, resp, next);
verify(next).doFilter(req, resp);
verify(session).removeAttribute("_FORCE_REDIRECT_");
+ System.clearProperty("ControlFilterTests");
}
}
