This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch release24.09
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/release24.09 by this push:
new 63dc7833e3 Improved: Check parameters passed in URLs (OFBIZ-13295)
63dc7833e3 is described below
commit 63dc7833e35f058ace6376f847b8fce0fd53ea5a
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
Conflicts handled by hand in ControlFilter.java
---
.../apache/ofbiz/webapp/control/ControlFilter.java | 9 ++++++-
.../ofbiz/webapp/control/ControlFilterTests.java | 28 +++++++++++++++++-----
2 files changed, 30 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 5aba5dd967..810a4c76b3 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
@@ -160,6 +160,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
@@ -171,7 +176,9 @@ public class ControlFilter extends HttpFilter {
// 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()) {
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 1b2a01e40b..c437b6175d 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 javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
@@ -30,9 +33,6 @@ 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 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");
}
}
