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 efb43da46a Improved: Rename 2  UtilValidate class methods for clarity 
(OFBIZ-13160)
efb43da46a is described below

commit efb43da46a6db4688b610387a596eef49e2f73e5
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Thu Oct 24 16:54:21 2024 +0200

    Improved: Rename 2  UtilValidate class methods for clarity (OFBIZ-13160)
    
    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/webtools/WebToolsServices.java    |  2 +-
 .../apache/ofbiz/widget/model/ScreenFactory.java   |  2 +-
 8 files changed, 59 insertions(+), 13 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 cb673e0c30..b5cf491c5c 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
@@ -125,7 +125,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));
@@ -357,7 +357,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 eb8515e2d1..f9032237fd 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
@@ -414,7 +414,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 b696cd6367..7a7511271f 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
@@ -171,7 +171,7 @@ public class ControlFilter extends HttpFilter {
             if (queryString != null) {
                 queryString = URLDecoder.decode(queryString, "UTF-8");
                 // wt=javabin allows Solr tests, see 
https://cwiki.apache.org/confluence/display/solr/javabin
-                if (UtilValidate.isUrl(queryString)
+                if (UtilValidate.isUrlInString(queryString)
                         || !SecuredUpload.isValidText(queryString, 
Collections.emptyList())
                         && !(queryString.contains("JavaScriptEnabled=Y")
                                 || queryString.contains("wt=javabin"))) {
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);

Reply via email to