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
git-svn-id: https://svn.apache.org/repos/asf/sling/trunk@1767618 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/src/main/java/org/apache/sling/launchpad/base/impl/Sling.java b/src/main/java/org/apache/sling/launchpad/base/impl/Sling.java
index 193344c..15914ab 100644
--- a/src/main/java/org/apache/sling/launchpad/base/impl/Sling.java
+++ b/src/main/java/org/apache/sling/launchpad/base/impl/Sling.java
@@ -44,6 +44,7 @@
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 @@
}
// 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 @@
// 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 @@
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 @@
}
}
- // ---------- 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>${<prop-name>}</tt>, where <tt><prop-name></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);
diff --git a/src/main/java/org/apache/sling/launchpad/base/shared/Util.java b/src/main/java/org/apache/sling/launchpad/base/shared/Util.java
new file mode 100644
index 0000000..43cd7ce
--- /dev/null
+++ b/src/main/java/org/apache/sling/launchpad/base/shared/Util.java
@@ -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>${<prop-name>}</tt>, where <tt><prop-name></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;
+ }
+
+}
diff --git a/src/main/java/org/apache/sling/launchpad/webapp/SlingServlet.java b/src/main/java/org/apache/sling/launchpad/webapp/SlingServlet.java
index d24bb13..60a37d9 100644
--- a/src/main/java/org/apache/sling/launchpad/webapp/SlingServlet.java
+++ b/src/main/java/org/apache/sling/launchpad/webapp/SlingServlet.java
@@ -39,6 +39,7 @@
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 @@
}
// 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 @@
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>${<prop-name>}</tt>, where <tt><prop-name></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;
- }
}
diff --git a/src/test/java/org/apache/sling/launchpad/base/shared/UtilTest.java b/src/test/java/org/apache/sling/launchpad/base/shared/UtilTest.java
new file mode 100644
index 0000000..812fe6f
--- /dev/null
+++ b/src/test/java/org/apache/sling/launchpad/base/shared/UtilTest.java
@@ -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