You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by fm...@apache.org on 2016/11/02 08:51:17 UTC

svn commit: r1767618 - in /sling/trunk/launchpad/base/src: main/java/org/apache/sling/launchpad/base/impl/ main/java/org/apache/sling/launchpad/base/shared/ main/java/org/apache/sling/launchpad/webapp/ test/java/org/apache/sling/launchpad/base/shared/

Author: fmeschbe
Date: Wed Nov  2 08:51:17 2016
New Revision: 1767618

URL: http://svn.apache.org/viewvc?rev=1767618&view=rev
Log:
SLING-6226 substVars not properly handling unknown properties

- Apply patch
- Add new Util class with substVars method
- Remove duplicates in SlingServlet and Sling
- Add unit test for Util class

Added:
    sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/base/shared/Util.java
    sling/trunk/launchpad/base/src/test/java/org/apache/sling/launchpad/base/shared/
    sling/trunk/launchpad/base/src/test/java/org/apache/sling/launchpad/base/shared/UtilTest.java   (with props)
Modified:
    sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/base/impl/Sling.java
    sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/webapp/SlingServlet.java

Modified: sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/base/impl/Sling.java
URL: http://svn.apache.org/viewvc/sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/base/impl/Sling.java?rev=1767618&r1=1767617&r2=1767618&view=diff
==============================================================================
--- sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/base/impl/Sling.java (original)
+++ sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/base/impl/Sling.java Wed Nov  2 08:51:17 2016
@@ -44,6 +44,7 @@ import org.apache.felix.framework.util.F
 import org.apache.sling.launchpad.api.LaunchpadContentProvider;
 import org.apache.sling.launchpad.base.shared.Notifiable;
 import org.apache.sling.launchpad.base.shared.SharedConstants;
+import org.apache.sling.launchpad.base.shared.Util;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleException;
@@ -408,7 +409,7 @@ public class Sling {
         }
 
         // resolve variables and ensure sling.home is an absolute path
-        slingHome = substVars(slingHome, SharedConstants.SLING_HOME, null, staticProps);
+        slingHome = Util.substVars(slingHome, SharedConstants.SLING_HOME, null, staticProps);
         File slingHomeFile = new File(slingHome).getAbsoluteFile();
         slingHome = slingHomeFile.getAbsolutePath();
 
@@ -471,7 +472,7 @@ public class Sling {
 
         // Perform variable substitution for system properties.
         for (Entry<String, String> entry : runtimeProps.entrySet()) {
-            entry.setValue(substVars(entry.getValue(), entry.getKey(), null,
+            entry.setValue(Util.substVars(entry.getValue(), entry.getKey(), null,
                 runtimeProps));
         }
 
@@ -755,7 +756,7 @@ public class Sling {
             String include = entry.getValue();
 
             // ensure variable resolution on this property
-            include = substVars(include, key, null, props);
+            include = Util.substVars(include, key, null, props);
 
             StringTokenizer tokener = new StringTokenizer(include, ",");
             while (tokener.hasMoreTokens()) {
@@ -856,131 +857,6 @@ public class Sling {
         }
     }
 
-    // ---------- Property file variable substition support --------------------
-
-    /**
-     * The starting delimiter of variable names (value is "${").
-     */
-    private static final String DELIM_START = "${";
-
-    /**
-     * The ending delimiter of variable names (value is "}").
-     */
-    private static final String DELIM_STOP = "}";
-
-    /**
-     * This method performs property variable substitution on the specified
-     * value. If the specified value contains the syntax
-     * <tt>${&lt;prop-name&gt;}</tt>, where <tt>&lt;prop-name&gt;</tt>
-     * refers to either a configuration property or a system property, then the
-     * corresponding property value is substituted for the variable placeholder.
-     * Multiple variable placeholders may exist in the specified value as well
-     * as nested variable placeholders, which are substituted from inner most to
-     * outer most. Configuration properties override system properties.
-     *
-     * NOTE - this is a verbatim copy of the same-named method
-     * in o.a.s.launchpad.webapp.SlingServlet. Please keep them in sync.
-     *
-     * @param val The string on which to perform property substitution.
-     * @param currentKey The key of the property being evaluated used to detect
-     *            cycles.
-     * @param cycleMap Map of variable references used to detect nested cycles.
-     * @param configProps Set of configuration properties.
-     * @return The value of the specified string after system property
-     *         substitution.
-     * @throws IllegalArgumentException If there was a syntax error in the
-     *             property placeholder syntax or a recursive variable
-     *             reference.
-     */
-    private static String substVars(String val, String currentKey,
-            Map<String, String> cycleMap, Map<String, String> configProps)
-            throws IllegalArgumentException {
-        // If there is currently no cycle map, then create
-        // one for detecting cycles for this invocation.
-        if (cycleMap == null) {
-            cycleMap = new HashMap<String, String>();
-        }
-
-        // Put the current key in the cycle map.
-        cycleMap.put(currentKey, currentKey);
-
-        // Assume we have a value that is something like:
-        // "leading ${foo.${bar}} middle ${baz} trailing"
-
-        // Find the first ending '}' variable delimiter, which
-        // will correspond to the first deepest nested variable
-        // placeholder.
-        int stopDelim = val.indexOf(DELIM_STOP);
-
-        // Find the matching starting "${" variable delimiter
-        // by looping until we find a start delimiter that is
-        // greater than the stop delimiter we have found.
-        int startDelim = val.indexOf(DELIM_START);
-        while (stopDelim >= 0) {
-            int idx = val.indexOf(DELIM_START, startDelim
-                + DELIM_START.length());
-            if ((idx < 0) || (idx > stopDelim)) {
-                break;
-            } else if (idx < stopDelim) {
-                startDelim = idx;
-            }
-        }
-
-        // If we do not have a start or stop delimiter, then just
-        // return the existing value.
-        if ((startDelim < 0) && (stopDelim < 0)) {
-            return val;
-        }
-        // At this point, we found a stop delimiter without a start,
-        // so throw an exception.
-        else if (((startDelim < 0) || (startDelim > stopDelim))
-            && (stopDelim >= 0)) {
-            throw new IllegalArgumentException(
-                "stop delimiter with no start delimiter: " + val);
-        }
-
-        // At this point, we have found a variable placeholder so
-        // we must perform a variable substitution on it.
-        // Using the start and stop delimiter indices, extract
-        // the first, deepest nested variable placeholder.
-        String variable = val.substring(startDelim + DELIM_START.length(),
-            stopDelim);
-
-        // Verify that this is not a recursive variable reference.
-        if (cycleMap.get(variable) != null) {
-            throw new IllegalArgumentException("recursive variable reference: "
-                + variable);
-        }
-
-        // Get the value of the deepest nested variable placeholder.
-        // Try to configuration properties first.
-        String substValue = (configProps != null)
-                ? configProps.get(variable)
-                : null;
-        if (substValue == null) {
-            // Ignore unknown property values.
-            substValue = System.getProperty(variable, "");
-        }
-
-        // Remove the found variable from the cycle map, since
-        // it may appear more than once in the value and we don't
-        // want such situations to appear as a recursive reference.
-        cycleMap.remove(variable);
-
-        // Append the leading characters, the substituted value of
-        // the variable, and the trailing characters to get the new
-        // value.
-        val = val.substring(0, startDelim) + substValue
-            + val.substring(stopDelim + DELIM_STOP.length(), val.length());
-
-        // Now perform substitution again, since there could still
-        // be substitutions to make.
-        val = substVars(val, currentKey, cycleMap, configProps);
-
-        // Return the value.
-        return val;
-    }
-
     private void copyBootstrapCommandFile(final Map<String, String> props) {
         // check last modification date
         final URL url = this.resourceProvider.getResource(BootstrapInstaller.BOOTSTRAP_CMD_FILENAME);

Added: sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/base/shared/Util.java
URL: http://svn.apache.org/viewvc/sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/base/shared/Util.java?rev=1767618&view=auto
==============================================================================
--- sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/base/shared/Util.java (added)
+++ sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/base/shared/Util.java Wed Nov  2 08:51:17 2016
@@ -0,0 +1,160 @@
+/*
+ * 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.sling.launchpad.base.shared;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * The <code>Util</code> class provides general shared utilities.
+ */
+public final class Util {
+
+    // no instantiate
+    private Util() {}
+
+    // ---------- Property file variable substition support --------------------
+
+    /**
+     * The starting delimiter of variable names (value is "${").
+     */
+    private static final String DELIM_START = "${";
+
+    /**
+     * The ending delimiter of variable names (value is "}").
+     */
+    private static final String DELIM_STOP = "}";
+
+    /**
+     * This method performs property variable substitution on the specified
+     * value. If the specified value contains the syntax
+     * <tt>${&lt;prop-name&gt;}</tt>, where <tt>&lt;prop-name&gt;</tt>
+     * refers to either a configuration property or a system property, then the
+     * corresponding property value is substituted for the variable placeholder.
+     * Multiple variable placeholders may exist in the specified value as well
+     * as nested variable placeholders, which are substituted from inner most to
+     * outer most. Configuration properties override system properties.
+     *
+     * @param val The string on which to perform property substitution.
+     * @param currentKey The key of the property being evaluated used to detect
+     *            cycles.
+     * @param cycleMap Map of variable references used to detect nested cycles.
+     * @param configProps Set of configuration properties.
+     * @return The value of the specified string after system property
+     *         substitution.
+     * @throws IllegalArgumentException If there was a syntax error in the
+     *             property placeholder syntax or a recursive variable
+     *             reference.
+     */
+    public static String substVars(String val, String currentKey,
+            Map<String, String> cycleMap, Map<String, String> configProps)
+            throws IllegalArgumentException {
+
+        /////////////////////////////////////////////////////////////////
+        // This version copied from org.apache.felix.framework.util.Util, Rev. 1762242
+        /////////////////////////////////////////////////////////////////
+
+        // If there is currently no cycle map, then create
+        // one for detecting cycles for this invocation.
+        if (cycleMap == null) {
+            cycleMap = new HashMap<>();
+        }
+
+        // Put the current key in the cycle map.
+        cycleMap.put(currentKey, currentKey);
+
+        // Assume we have a value that is something like:
+        // "leading ${foo.${bar}} middle ${baz} trailing"
+
+        // Find the first ending '}' variable delimiter, which
+        // will correspond to the first deepest nested variable
+        // placeholder.
+        int stopDelim = -1;
+        int startDelim = -1;
+
+        do {
+            stopDelim = val.indexOf(DELIM_STOP, stopDelim + 1);
+            // If there is no stopping delimiter, then just return
+            // the value since there is no variable declared.
+            if (stopDelim < 0) {
+                return val;
+            }
+            // Try to find the matching start delimiter by
+            // looping until we find a start delimiter that is
+            // greater than the stop delimiter we have found.
+            startDelim = val.indexOf(DELIM_START);
+            // If there is no starting delimiter, then just return
+            // the value since there is no variable declared.
+            if (startDelim < 0) {
+                return val;
+            }
+            while (stopDelim >= 0) {
+                int idx = val.indexOf(DELIM_START, startDelim + DELIM_START.length());
+                if ((idx < 0) || (idx > stopDelim)) {
+                    break;
+                } else if (idx < stopDelim) {
+                    startDelim = idx;
+                }
+            }
+        } while ((startDelim > stopDelim) && (stopDelim >= 0));
+
+        // At this point, we have found a variable placeholder so
+        // we must perform a variable substitution on it.
+        // Using the start and stop delimiter indices, extract
+        // the first, deepest nested variable placeholder.
+        String variable =
+                val.substring(startDelim + DELIM_START.length(), stopDelim);
+
+        // Verify that this is not a recursive variable reference.
+        if (cycleMap.get(variable) != null) {
+            throw new IllegalArgumentException(
+                "recursive variable reference: " + variable);
+        }
+
+        // Get the value of the deepest nested variable placeholder.
+        // Try to configuration properties first.
+        String substValue = (configProps != null)
+                ? configProps.get(variable)
+                : null;
+        if (substValue == null) {
+            // Ignore unknown property values.
+            substValue = System.getProperty(variable, "");
+        }
+
+        // Remove the found variable from the cycle map, since
+        // it may appear more than once in the value and we don't
+        // want such situations to appear as a recursive reference.
+        cycleMap.remove(variable);
+
+        // Append the leading characters, the substituted value of
+        // the variable, and the trailing characters to get the new
+        // value.
+        val = val.substring(0, startDelim)
+            + substValue
+            + val.substring(stopDelim + DELIM_STOP.length(), val.length());
+
+        // Now perform substitution again, since there could still
+        // be substitutions to make.
+        val = substVars(val, currentKey, cycleMap, configProps);
+
+        // Return the value.
+        return val;
+    }
+
+}

Modified: sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/webapp/SlingServlet.java
URL: http://svn.apache.org/viewvc/sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/webapp/SlingServlet.java?rev=1767618&r1=1767617&r2=1767618&view=diff
==============================================================================
--- sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/webapp/SlingServlet.java (original)
+++ sling/trunk/launchpad/base/src/main/java/org/apache/sling/launchpad/webapp/SlingServlet.java Wed Nov  2 08:51:17 2016
@@ -39,6 +39,7 @@ import org.apache.sling.launchpad.base.s
 import org.apache.sling.launchpad.base.shared.Loader;
 import org.apache.sling.launchpad.base.shared.Notifiable;
 import org.apache.sling.launchpad.base.shared.SharedConstants;
+import org.apache.sling.launchpad.base.shared.Util;
 
 /**
  * The <code>SlingServlet</code> is the externally visible Web Application
@@ -467,7 +468,7 @@ public class SlingServlet extends Generi
         }
 
         // substitute any ${...} references and make absolute
-        slingHome = substVars(slingHome);
+        slingHome = Util.substVars(slingHome, SharedConstants.SLING_HOME, null, properties);
         slingHome = new File(slingHome).getAbsolutePath();
 
         log("Setting sling.home=" + slingHome + " (" + source + ")");
@@ -573,134 +574,4 @@ public class SlingServlet extends Generi
         return props;
     }
 
-    /**
-     * The starting delimiter of variable names (value is "${").
-     */
-    private static final String DELIM_START = "${";
-
-    /**
-     * The ending delimiter of variable names (value is "}").
-     */
-    private static final String DELIM_STOP = "}";
-
-    private String substVars(final String val) {
-        if (val.contains(DELIM_START)) {
-            return substVars(val, null, null, properties);
-        }
-
-        return val;
-    }
-
-    /**
-     * This method performs property variable substitution on the specified
-     * value. If the specified value contains the syntax
-     * <tt>${&lt;prop-name&gt;}</tt>, where <tt>&lt;prop-name&gt;</tt>
-     * refers to either a configuration property or a system property, then the
-     * corresponding property value is substituted for the variable placeholder.
-     * Multiple variable placeholders may exist in the specified value as well
-     * as nested variable placeholders, which are substituted from inner most to
-     * outer most. Configuration properties override system properties.
-     *
-     * NOTE - this is a verbatim copy of the same-named method
-     * in o.a.s.launchpad.base.impl.Sling. Please keep them in sync.
-     *
-     * @param val The string on which to perform property substitution.
-     * @param currentKey The key of the property being evaluated used to detect
-     *            cycles.
-     * @param cycleMap Map of variable references used to detect nested cycles.
-     * @param configProps Set of configuration properties.
-     * @return The value of the specified string after system property
-     *         substitution.
-     * @throws IllegalArgumentException If there was a syntax error in the
-     *             property placeholder syntax or a recursive variable
-     *             reference.
-     */
-    private static String substVars(String val, String currentKey,
-            Map<String, String> cycleMap, Map<String, String> configProps)
-            throws IllegalArgumentException {
-        // If there is currently no cycle map, then create
-        // one for detecting cycles for this invocation.
-        if (cycleMap == null) {
-            cycleMap = new HashMap<String, String>();
-        }
-
-        // Put the current key in the cycle map.
-        cycleMap.put(currentKey, currentKey);
-
-        // Assume we have a value that is something like:
-        // "leading ${foo.${bar}} middle ${baz} trailing"
-
-        // Find the first ending '}' variable delimiter, which
-        // will correspond to the first deepest nested variable
-        // placeholder.
-        int stopDelim = val.indexOf(DELIM_STOP);
-
-        // Find the matching starting "${" variable delimiter
-        // by looping until we find a start delimiter that is
-        // greater than the stop delimiter we have found.
-        int startDelim = val.indexOf(DELIM_START);
-        while (stopDelim >= 0) {
-            int idx = val.indexOf(DELIM_START, startDelim
-                + DELIM_START.length());
-            if ((idx < 0) || (idx > stopDelim)) {
-                break;
-            } else if (idx < stopDelim) {
-                startDelim = idx;
-            }
-        }
-
-        // If we do not have a start or stop delimiter, then just
-        // return the existing value.
-        if ((startDelim < 0) && (stopDelim < 0)) {
-            return val;
-        }
-        // At this point, we found a stop delimiter without a start,
-        // so throw an exception.
-        else if (((startDelim < 0) || (startDelim > stopDelim))
-            && (stopDelim >= 0)) {
-            throw new IllegalArgumentException(
-                "stop delimiter with no start delimiter: " + val);
-        }
-
-        // At this point, we have found a variable placeholder so
-        // we must perform a variable substitution on it.
-        // Using the start and stop delimiter indices, extract
-        // the first, deepest nested variable placeholder.
-        String variable = val.substring(startDelim + DELIM_START.length(),
-            stopDelim);
-
-        // Verify that this is not a recursive variable reference.
-        if (cycleMap.get(variable) != null) {
-            throw new IllegalArgumentException("recursive variable reference: "
-                + variable);
-        }
-
-        // Get the value of the deepest nested variable placeholder.
-        // Try to configuration properties first.
-        String substValue = (configProps != null)
-                ? configProps.get(variable)
-                : null;
-        if (substValue == null) {
-            // Ignore unknown property values.
-            substValue = System.getProperty(variable, "");
-        }
-
-        // Remove the found variable from the cycle map, since
-        // it may appear more than once in the value and we don't
-        // want such situations to appear as a recursive reference.
-        cycleMap.remove(variable);
-
-        // Append the leading characters, the substituted value of
-        // the variable, and the trailing characters to get the new
-        // value.
-        val = val.substring(0, startDelim) + substValue
-            + val.substring(stopDelim + DELIM_STOP.length(), val.length());
-
-        // Now perform substitution again, since there could still
-        // be substitutions to make.
-        val = substVars(val, currentKey, cycleMap, configProps);
-
-        // Return the value.
-        return val;
-    }
 }

Added: sling/trunk/launchpad/base/src/test/java/org/apache/sling/launchpad/base/shared/UtilTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/launchpad/base/src/test/java/org/apache/sling/launchpad/base/shared/UtilTest.java?rev=1767618&view=auto
==============================================================================
--- sling/trunk/launchpad/base/src/test/java/org/apache/sling/launchpad/base/shared/UtilTest.java (added)
+++ sling/trunk/launchpad/base/src/test/java/org/apache/sling/launchpad/base/shared/UtilTest.java Wed Nov  2 08:51:17 2016
@@ -0,0 +1,73 @@
+/*
+ * 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.sling.launchpad.base.shared;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Test;
+
+import junit.framework.TestCase;
+
+public class UtilTest {
+
+    @SuppressWarnings("serial")
+    static Map<String, String> properties = Collections.unmodifiableMap(new HashMap<String, String>() {
+        {
+            put("foo", "_v_foo_");
+            put("bar", "_v_bar_");
+            put("baz", "_v_baz_");
+            put("foo._v_bar_", "_v_foo.bar_");
+        }
+    });
+
+    @Test
+    public void test_substVars_no_replacement() {
+        TestCase.assertEquals("foo", Util.substVars("foo", "the_foo", null, properties));
+        TestCase.assertEquals("%d{yyyy-MM-dd} %t{short} %m", Util.substVars("%d{yyyy-MM-dd} %t{short} %m", "the_foo", null, properties));
+    }
+
+    @Test
+    public void test_substVars_single_replacement() {
+        TestCase.assertEquals("_v_foo_", Util.substVars("${foo}", "the_foo", null, properties));
+        TestCase.assertEquals("leading _v_foo_ trailing", Util.substVars("leading ${foo} trailing", "the_foo", null, properties));
+        TestCase.assertEquals("leading _v_foo_ middle _v_baz_ trailing",
+            Util.substVars("leading ${foo} middle ${baz} trailing", "the_foo", null, properties));
+    }
+
+    @Test
+    public void test_substVars_nested_replacement() {
+        TestCase.assertEquals("leading _v_foo.bar_ middle _v_baz_ trailing",
+            Util.substVars("leading ${foo.${bar}} middle ${baz} trailing", "the_foo", null, properties));
+    }
+
+    @Test
+    public void test_substVars_missing_replacement() {
+        System.setProperty("foobar", "_v_foobar_");
+        System.clearProperty("foobaz");
+        TestCase.assertEquals("leading _v_foobar_ middle  trailing",
+            Util.substVars("leading ${foobar} middle ${foobaz} trailing", "the_foo", null, properties));
+    }
+
+    @Test(expected=IllegalArgumentException.class)
+    public void test_substVars_recursive_failure() {
+        Util.substVars("leading ${foo} middle ${baz} trailing", "foo", null, properties);
+    }
+}
\ No newline at end of file

Propchange: sling/trunk/launchpad/base/src/test/java/org/apache/sling/launchpad/base/shared/UtilTest.java
------------------------------------------------------------------------------
    svn:eol-style = native