This is an automated email from the ASF dual-hosted git repository. jacopoc pushed a commit to branch release24.09 in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
commit d8a62e8f5c43b116be471bac90ae0b9f64f6de29 Author: Jacques Le Roux <[email protected]> AuthorDate: Thu Oct 24 16:54:21 2024 +0200 Improved: Rename 2 UtilValidate class methods for clarity (OFBIZ-13160) Backported from trunk efb43da46a with minor modifications. Daniel Watford proposed to rename and better describe the 2 UtilValidate class methods: isUrl and urlInString. I respectively suggested isUrlInString and isUrlInStringAndDoesNotStartByComponentProtocol. Daniel also provided an UtilValidateTests class. Thanks: Daniel --- .../org/apache/ofbiz/base/util/GroovyUtil.java | 2 +- .../org/apache/ofbiz/base/util/ScriptUtil.java | 4 +- .../java/org/apache/ofbiz/base/util/UtilHttp.java | 2 +- .../org/apache/ofbiz/base/util/UtilValidate.java | 12 +++--- .../apache/ofbiz/base/util/UtilValidateTests.java | 46 ++++++++++++++++++++++ .../apache/ofbiz/webapp/control/ControlFilter.java | 2 +- .../apache/ofbiz/webapp/stats/VisitHandler.java | 2 +- .../apache/ofbiz/webtools/WebToolsServices.java | 2 +- .../apache/ofbiz/widget/model/ScreenFactory.java | 2 +- 9 files changed, 60 insertions(+), 14 deletions(-) diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java index c2f8ed32e0..457d36c17e 100644 --- a/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java +++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java @@ -142,7 +142,7 @@ public final class GroovyUtil { Class<?> scriptClass = PARSED_SCRIPTS.get(location); if (scriptClass == null) { URL scriptUrl = FlexibleLocation.resolveLocation(location); - if (scriptUrl == null || UtilValidate.urlInString(scriptUrl.toString())) { + if (scriptUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(scriptUrl.toString())) { throw new GeneralException("Script not found at location [" + location + "]"); } scriptClass = parseClass(scriptUrl.openStream(), location); diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/ScriptUtil.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/ScriptUtil.java index 25401c4da4..d83bf98619 100644 --- a/framework/base/src/main/java/org/apache/ofbiz/base/util/ScriptUtil.java +++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/ScriptUtil.java @@ -130,7 +130,7 @@ public final class ScriptUtil { try { Compilable compilableEngine = (Compilable) engine; URL scriptUrl = FlexibleLocation.resolveLocation(filePath); - if (scriptUrl == null || UtilValidate.urlInString(scriptUrl.toString())) { + if (scriptUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(scriptUrl.toString())) { throw new ScriptException("Script not found at location [" + filePath + "]"); } BufferedReader reader = new BufferedReader(new InputStreamReader(scriptUrl.openStream(), StandardCharsets.UTF_8)); @@ -365,7 +365,7 @@ public final class ScriptUtil { } engine.setContext(scriptContext); URL scriptUrl = FlexibleLocation.resolveLocation(filePath); - if (scriptUrl == null || UtilValidate.urlInString(scriptUrl.toString())) { + if (scriptUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(scriptUrl.toString())) { throw new ScriptException("Script not found at location [" + filePath + "]"); } try ( diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java index 63496286a4..485d533f23 100644 --- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java +++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java @@ -415,7 +415,7 @@ public final class UtilHttp { // if the string contains only an URL beginning by http or ftp => no change to keep special chars if (UtilValidate.isValidUrl(s) && (s.indexOf("://") == 4 || s.indexOf("://") == 3)) { params = params + s + " "; - } else if (UtilValidate.isUrl(s) && !s.isEmpty()) { + } else if (UtilValidate.isUrlInString(s) && !s.isEmpty()) { // if the string contains not only an URL => concatenate possible canonicalized before and after, w/o changing the URL String url = extractUrls(s).get(0); // There should be only 1 URL in a block, makes no sense else int start = s.indexOf(url); diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java index 9dd3735b6b..3be7d81ba2 100644 --- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java +++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java @@ -621,11 +621,11 @@ public final class UtilValidate { } /** - * isUrl returns true if the string contains :// + * isUrlInString returns true if the string is empty or contains :// * @param s String to validate Note: this does not handle "component://" specific to OFBiz - * @return true if s contains :// + * @return true if s is empty or contains :// */ - public static boolean isUrl(String s) { + public static boolean isUrlInString(String s) { if (isEmpty(s)) { return DEFAULT_EMPTY_OK; } @@ -633,11 +633,11 @@ public final class UtilValidate { } /** - * urlInString returns true if the string contains :// and does not start with "component://" + * isUrlInStringAndDoesNotStartByComponentProtocol returns true if the string is non-empty, contains :// but does not start with "component://" * @param s String to validate - * @return true if s contains :// and does not start with "component://" + * @return true if s is non-empty, contains :// and does not start with "component://" */ - public static boolean urlInString(String s) { + public static boolean isUrlInStringAndDoesNotStartByComponentProtocol(String s) { if (isEmpty(s) || s.startsWith("component://")) { return false; } diff --git a/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilValidateTests.java b/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilValidateTests.java new file mode 100644 index 0000000000..dca3a6e38c --- /dev/null +++ b/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilValidateTests.java @@ -0,0 +1,46 @@ +/* + * 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.base.util; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +public class UtilValidateTests { + + @Test + public void testUrlValidations() throws Exception { + assertFalse(UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol("")); + assertFalse(UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol("non-url-string")); + assertFalse(UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol("component://foo/bar")); + assertTrue(UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(" component://foo/bar")); + assertTrue(UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol("http://foo/bar";)); + assertTrue(UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol("https://foo/bar";)); + assertFalse(UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol("component://foo/bar?param=http://moo/far";)); + + assertTrue(UtilValidate.isUrlInString("")); + assertFalse(UtilValidate.isUrlInString("non-url-string")); + assertTrue(UtilValidate.isUrlInString("component://foo/bar")); + assertTrue(UtilValidate.isUrlInString(" component://foo/bar")); + assertTrue(UtilValidate.isUrlInString("http://foo/bar";)); + assertTrue(UtilValidate.isUrlInString("https://foo/bar";)); + assertTrue(UtilValidate.isUrlInString("component://foo/bar?param=http://moo/far";)); + } +} 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 4b044d827e..e2cb44c7e2 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 @@ -242,7 +242,7 @@ public class ControlFilter extends HttpFilter { } if (queryString != null) { queryString = URLDecoder.decode(queryString, "UTF-8"); - if (UtilValidate.isUrl(queryString) + if (UtilValidate.isUrlInString(queryString) || !SecuredUpload.isValidText(queryString.toLowerCase(), ALLOWEDTOKENS, true)) { Debug.logError("For security reason this URL is not accepted", MODULE); throw new RuntimeException("For security reason this URL is not accepted"); diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/stats/VisitHandler.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/stats/VisitHandler.java index aff08ea06b..ead695652f 100644 --- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/stats/VisitHandler.java +++ b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/stats/VisitHandler.java @@ -137,7 +137,7 @@ public class VisitHandler { Locale initialLocaleObj = (Locale) session.getAttribute("_CLIENT_LOCALE_"); String initialRequest = (String) session.getAttribute("_CLIENT_REQUEST_"); String initialReferrer = (String) session.getAttribute("_CLIENT_REFERER_"); - if (!UtilValidate.isUrl(initialReferrer)) { + if (!UtilValidate.isUrlInString(initialReferrer)) { initialReferrer = "Not an URL"; } String initialUserAgent = (String) session.getAttribute("_CLIENT_USER_AGENT_"); diff --git a/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java b/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java index 77c8425059..aa00b03bba 100644 --- a/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java +++ b/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java @@ -147,7 +147,7 @@ public class WebToolsServices { // ############################# // FM Template // ############################# - if (UtilValidate.urlInString(fulltext) + if (UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(fulltext) && !"true".equals(EntityUtilProperties.getPropertyValue("security", "security.datafile.loadurls.enable", "false", delegator))) { Debug.logError("For security reason HTTP URLs are not accepted, see OFBIZ-12304", MODULE); Debug.logInfo("Rather load your data from a file or set SystemProperty security.datafile.loadurls.enable = true", MODULE); 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 faea40b76c..549075c926 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 @@ -121,7 +121,7 @@ public class ScreenFactory { long startTime = System.currentTimeMillis(); URL screenFileUrl = null; screenFileUrl = FlexibleLocation.resolveLocation(resourceName); - if (screenFileUrl == null || UtilValidate.urlInString(screenFileUrl.toString())) { + if (screenFileUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(screenFileUrl.toString())) { throw new IllegalArgumentException("Could not resolve location to URL: " + resourceName); } Document screenFileDoc = UtilXml.readXmlDocument(screenFileUrl, true, true);
