This is an automated email from the ASF dual-hosted git repository.
ashishvijaywargiya 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 9ca27f4410 Improved: Enhance RequestHandler view state persistence and
screen location resolution (#1624)
9ca27f4410 is described below
commit 9ca27f4410c5b003e208514481a63ba6883e2e51
Author: Krishna Uprit <[email protected]>
AuthorDate: Fri Aug 14 12:03:38 2026 +0530
Improved: Enhance RequestHandler view state persistence and screen location
resolution (#1624)
This PR builds on [PR
#1586](Krishnauprit18:secure-widget-resource-loading) and keeps the
WidgetSecureLocation approach, adding two related changes in
RequestHandler:
1. View state persistence check (_LAST_VIEW_NAME_): Validates candidate
last view names against ControllerConfig before they are stored in the
session, so view navigation falls back cleanly.
2. Parameter-level attribute filtering (_LAST_VIEW_PARAMS_): Filters
dynamic resource and location parameters (*Location, *Screen, *Template,
*Uri) during session persistence and view-last attribute restoration.
Thank you Krishna for the contribution.
---
.../product/webapp/facility/WEB-INF/controller.xml | 2 +-
.../ofbiz/webapp/control/RequestHandler.java | 49 ++++++++++++++++++----
.../apache/ofbiz/widget/model/ScreenFactory.java | 9 ++--
.../ofbiz/widget/model/WidgetSecureLocation.java | 47 +++++++++++++++++++++
4 files changed, 93 insertions(+), 14 deletions(-)
diff --git a/applications/product/webapp/facility/WEB-INF/controller.xml
b/applications/product/webapp/facility/WEB-INF/controller.xml
index 2a92c4f8cb..eb28e0de81 100644
--- a/applications/product/webapp/facility/WEB-INF/controller.xml
+++ b/applications/product/webapp/facility/WEB-INF/controller.xml
@@ -1316,7 +1316,7 @@ under the License.
<view-map name="EditShipmentPlan" type="screen"
page="component://product/widget/facility/ShipmentScreens.xml#EditShipmentPlan"/>
<view-map name="ViewShipmentReceipts" type="screen"
page="component://product/widget/facility/ShipmentScreens.xml#ViewShipmentReceipts"/>
<view-map name="EditShipmentPackages" type="screen"
page="component://product/widget/facility/ShipmentScreens.xml#EditShipmentPackages"/>
- <view-map name="EditShipmentRouteSegments" type="screen"
page="component://product/widget/facility/ShipmentScreens.xml#EditShipmentRouteSegments"
auth="false"/>
+ <view-map name="EditShipmentRouteSegments" type="screen"
page="component://product/widget/facility/ShipmentScreens.xml#EditShipmentRouteSegments"
auth="true"/>
<view-map name="AddItemsFromOrder" type="screen"
page="component://product/widget/facility/ShipmentScreens.xml#AddItemsFromOrder"/>
<view-map name="AddItemsFromInventory" type="screen"
page="component://product/widget/facility/ShipmentScreens.xml#AddItemsFromInventory"/>
<view-map name="ReceiveInventoryAgainstPurchaseOrder" type="screen"
page="component://product/widget/facility/ShipmentScreens.xml#ReceiveInventoryAgainstPurchaseOrder"/>
diff --git
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
index a43e6535a5..1aa558bcf8 100644
---
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
+++
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
@@ -1004,8 +1004,13 @@ public final class RequestHandler {
if (urlParams != null) {
for (Map.Entry<String, Object> urlParamEntry :
urlParams.entrySet()) {
String key = urlParamEntry.getKey();
- // Don't overwrite messages coming from the current
event
- if (!("_EVENT_MESSAGE_".equals(key) ||
"_ERROR_MESSAGE_".equals(key)
+ // Filter out event messages and dynamic
resource/location override parameters
+ if (key != null && !key.startsWith("_")
+ && !key.endsWith("Location")
+ && !key.endsWith("Screen")
+ && !key.endsWith("Template")
+ && !key.endsWith("Uri")
+ && !("_EVENT_MESSAGE_".equals(key) ||
"_ERROR_MESSAGE_".equals(key)
|| "_EVENT_MESSAGE_LIST_".equals(key) ||
"_ERROR_MESSAGE_LIST_".equals(key))) {
request.setAttribute(key,
urlParamEntry.getValue());
}
@@ -1213,22 +1218,48 @@ public final class RequestHandler {
// add in the attributes as well so everything needed for the
rendering context will be in place if/when we get back to this view
paramMap.putAll(UtilHttp.getAttributeMap(req));
UtilMisc.makeMapSerializable(paramMap);
- // Used by lookups to keep the real view (request); accept the request
parameter only if it is a safe view name (alphanumeric/dash/underscore)
- String lastViewNameParam = (String) paramMap.get("_LAST_VIEW_NAME_");
- String lastViewName = (lastViewNameParam != null &&
lastViewNameParam.matches("[\\w\\-]+")) ? lastViewNameParam : view;
- req.getSession().setAttribute("_LAST_VIEW_NAME_", lastViewName);
- req.getSession().setAttribute("_LAST_VIEW_PARAMS_", paramMap);
+
+ // Used by lookups to keep the real view (request); validate candidate
last view against controller authorization policy
+ String candidateLastView = (String) paramMap.get("_LAST_VIEW_NAME_");
+ String safeLastView = view;
+ if (UtilValidate.isNotEmpty(candidateLastView) &&
candidateLastView.matches("[\\w\\-]+")) {
+ ConfigXMLReader.ControllerConfig cConfig = getControllerConfig();
+ if (cConfig != null) {
+ ConfigXMLReader.ViewMap candidateViewMap =
cConfig.getViewMapMap().get(candidateLastView);
+ ConfigXMLReader.RequestMap candidateReqMap =
cConfig.getRequestMapMap().get(candidateLastView);
+ if (candidateViewMap != null) {
+ boolean requiresAuth = candidateViewMap.isSecurityAuth()
+ || (candidateReqMap != null &&
candidateReqMap.isSecurityAuth());
+ if (!requiresAuth || UtilValidate.isNotEmpty(userLogin)) {
+ safeLastView = candidateLastView;
+ } else {
+ Debug.logWarning("Blocked unauthorized
_LAST_VIEW_NAME_ attempt: [" + candidateLastView
+ + "] for unauthenticated session " +
showSessionId(req), MODULE);
+ }
+ }
+ }
+ }
+ req.getSession().setAttribute("_LAST_VIEW_NAME_", safeLastView);
+
+ // Filter out sensitive dynamic location/template override parameters
from persisted session params
+ Map<String, Object> sanitizedParamMap = new HashMap<>(paramMap);
+ sanitizedParamMap.keySet().removeIf(key -> key != null && (
+ key.endsWith("Location")
+ || key.endsWith("Screen")
+ || key.endsWith("Template")
+ || key.endsWith("Uri")));
+ req.getSession().setAttribute("_LAST_VIEW_PARAMS_", sanitizedParamMap);
if ("SAVED".equals(saveName)) {
//Debug.logInfo("======save current view: " + view);
req.getSession().setAttribute("_SAVED_VIEW_NAME_", view);
- req.getSession().setAttribute("_SAVED_VIEW_PARAMS_", paramMap);
+ req.getSession().setAttribute("_SAVED_VIEW_PARAMS_",
sanitizedParamMap);
}
if ("HOME".equals(saveName)) {
//Debug.logInfo("======save home view: " + view);
req.getSession().setAttribute("_HOME_VIEW_NAME_", view);
- req.getSession().setAttribute("_HOME_VIEW_PARAMS_", paramMap);
+ req.getSession().setAttribute("_HOME_VIEW_PARAMS_",
sanitizedParamMap);
// clear other saved views
req.getSession().removeAttribute("_SAVED_VIEW_NAME_");
req.getSession().removeAttribute("_SAVED_VIEW_PARAMS_");
diff --git
a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ScreenFactory.java
b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ScreenFactory.java
index 9c540abafe..739816fb44 100644
---
a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ScreenFactory.java
+++
b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ScreenFactory.java
@@ -198,12 +198,13 @@ public class ScreenFactory {
ModelScreen modelScreen = null;
if (UtilValidate.isNotEmpty(location)) {
- if (UtilValidate.isBlockedPath(location)) {
- Debug.logWarning("The location of screen [%s] is on a blocked
Path. Abbort rendering. Raw location [%s]", MODULE, name, location);
- throw new IllegalArgumentException("Abort screenrendering due
to screenlocation pointing to a blocked path");
+ String sanitizedLocation = WidgetSecureLocation.sanitize(location);
+ if (sanitizedLocation == null) {
+ Debug.logWarning("The location of screen [%s] isn't an allowed
Path. Abort rendering. Raw location [%s]", MODULE, name, location);
+ throw new IllegalArgumentException("Abort screen rendering due
to unallowed screen location");
}
try {
- modelScreen = ScreenFactory.getScreenFromLocation(location,
name);
+ modelScreen =
ScreenFactory.getScreenFromLocation(sanitizedLocation, name);
} catch (IOException | SAXException | ParserConfigurationException
e) {
String errMsg = "Error rendering included screen named [" +
name + "] at location [" + location + "]: " + e.toString();
Debug.logError(e, errMsg, MODULE);
diff --git
a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/WidgetSecureLocation.java
b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/WidgetSecureLocation.java
new file mode 100644
index 0000000000..8c87b7b640
--- /dev/null
+++
b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/WidgetSecureLocation.java
@@ -0,0 +1,47 @@
+/*******************************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+
*******************************************************************************/
+package org.apache.ofbiz.widget.model;
+
+import java.nio.file.Paths;
+import org.apache.ofbiz.base.util.Debug;
+import org.apache.ofbiz.base.util.UtilValidate;
+
+public final class WidgetSecureLocation {
+
+ private static final String MODULE = WidgetSecureLocation.class.getName();
+ private static final String COMPO_TYPE = "component://";
+
+ public static String sanitize(String location) {
+ if (UtilValidate.isEmpty(location)
+ ||
UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(location)
+ || location.startsWith("file:/")) {
+ Debug.logWarning(String.format("Unable to sanitize location:
[%s]", location), MODULE);
+ return null;
+ }
+ if (location.startsWith(COMPO_TYPE) && location.length() > 12) {
+ if (location.indexOf("..") > 0) {
+ Debug.logWarning(String.format("For security reason traversal
sequence '..' is not allowed: [%s]", location), MODULE);
+ return null;
+ }
+ return COMPO_TYPE + Paths.get(location.substring(12)).normalize();
+ }
+
+ return location.startsWith(COMPO_TYPE) ? location : null;
+ }
+}
