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

framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java

@@ -142,7 +142,7 @@ public static Class<?> getScriptClassFromLocation(String location) throws Genera
             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);

framework/base/src/main/java/org/apache/ofbiz/base/util/ScriptUtil.java

@@ -125,7 +125,7 @@ private static CompiledScript compileScriptFile(String filePath) throws ScriptEx
             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 static Object executeScript(String filePath, String functionName, ScriptC
         }
         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 (

framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java

@@ -414,7 +414,7 @@ public static Map<String, Object> canonicalizeParameterMap(Map<String, Object> p
                         // 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);

framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java

@@ -621,23 +621,23 @@ public static boolean isEmailList(String s) {
     }
 
     /**
-     * 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;
         }
         return s.indexOf("://") != -1;
     }
 
     /**
-     * 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;
         }

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"));
+    }
+}

framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java

@@ -171,7 +171,7 @@ public void doFilter(HttpServletRequest req, HttpServletResponse resp, FilterCha
             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"))) {

framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java

@@ -147,7 +147,7 @@ public static Map<String, Object> entityImport(DispatchContext dctx, Map<String,
         // #############################
         // 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);

framework/widget/src/main/java/org/apache/ofbiz/widget/model/ScreenFactory.java

@@ -121,7 +121,7 @@ public static Map<String, ModelScreen> getScreensFromLocation(String resourceNam
                     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);