modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptArgumentParser.java
| 41 ++++++----
modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptDiscoveryComponent.java
| 27 ------
modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptServerComponent.java
| 40 +++++++--
modules/plugins/script/src/main/resources/META-INF/rhq-plugin.xml
| 13 ++-
modules/plugins/script/src/test/java/org/rhq/plugins/script/ScriptArgumentParserTest.java
| 7 +
5 files changed, 76 insertions(+), 52 deletions(-)
New commits:
commit 8058e6ba255af1cce6e269b4452fe9be2c548833
Author: Lukas Krejci <lkrejci(a)redhat.com>
Date: Sat Jan 25 00:08:01 2014 +0100
[BZ 1049608] - Changes to arg parsing in script server plugin
* we by default keep the broken old behavior (blind split by space)
* a boolean plugin prop to switch quoting and escapes on/off
* configurable escape char set to \ for all platforms
* escaping (when switched on) is POSIX-like:
1) In unquoted text: the escape character preserves the value of any
following character
2) In double-quoted text: the escape character escapes " and itself
otherwise is both the escape character and the following character
are left intact
3) In single-quoted text: no escaping can occur
(cherry picked from commit d8e1e71efc8eb3ffcf2729fc4518b37e5cd304e5)
Signed-off-by: Jirka Kremser <jkremser(a)redhat.com>
diff --git
a/modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptArgumentParser.java
b/modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptArgumentParser.java
index cd12f0d..a152b1e 100644
---
a/modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptArgumentParser.java
+++
b/modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptArgumentParser.java
@@ -10,7 +10,7 @@ import java.util.List;
public final class ScriptArgumentParser {
private enum State {
- SPACE, ESCAPE, ARG, QUOTE
+ SPACE, ESCAPE, ARG, DOUBLE_QUOTE, DOUBLE_QUOTE_ESCAPE, SINGLE_QUOTE
}
private ScriptArgumentParser() {
@@ -18,7 +18,6 @@ public final class ScriptArgumentParser {
public static String[] parse(String args, char escape) {
State state = State.SPACE;
- char activeQuote = '\u0000';
List<String> parsedArgs = new ArrayList<String>();
int i = 0;
@@ -37,9 +36,10 @@ public final class ScriptArgumentParser {
if (c == escape) {
state = State.ESCAPE;
- } else if (isQuote(c)) {
- activeQuote = c;
- state = State.QUOTE;
+ } else if (c == '"') {
+ state = State.DOUBLE_QUOTE;
+ } else if (c == '\'') {
+ state = State.SINGLE_QUOTE;
} else if (isNotWhitespace) {
arg.append(c);
state = State.ARG;
@@ -52,20 +52,35 @@ public final class ScriptArgumentParser {
case ARG:
if (c == escape) {
state = State.ESCAPE;
- } else if (isQuote(c)) {
- activeQuote = c;
- state = State.QUOTE;
+ } else if (c == '"') {
+ state = State.DOUBLE_QUOTE;
+ } else if (c == '\'') {
+ state = State.SINGLE_QUOTE;
} else if (!Character.isWhitespace(c)) {
arg.append(c);
} else {
state = State.SPACE;
}
break;
- case QUOTE:
- if (c == activeQuote) {
+ case DOUBLE_QUOTE:
+ if (c == '"') {
state = State.ARG;
} else if (c == escape) {
- state = State.ESCAPE;
+ state = State.DOUBLE_QUOTE_ESCAPE;
+ } else {
+ arg.append(c);
+ }
+ break;
+ case DOUBLE_QUOTE_ESCAPE:
+ if (c != '"' && c != escape) {
+ arg.append(escape);
+ }
+ arg.append(c);
+ state = State.DOUBLE_QUOTE;
+ break;
+ case SINGLE_QUOTE:
+ if (c == '\'') {
+ state = State.ARG;
} else {
arg.append(c);
}
@@ -88,8 +103,4 @@ public final class ScriptArgumentParser {
bld.delete(0, bld.length());
}
}
-
- private static boolean isQuote(char c) {
- return c == '\'' || c == '"';
- }
}
diff --git
a/modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptDiscoveryComponent.java
b/modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptDiscoveryComponent.java
index 4ee055a..82cf34e 100644
---
a/modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptDiscoveryComponent.java
+++
b/modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptDiscoveryComponent.java
@@ -52,11 +52,7 @@ import org.rhq.core.util.exception.ThrowableUtil;
*
* @author John Mazzitelli
*/
-public class ScriptDiscoveryComponent implements
ResourceDiscoveryComponent<ResourceComponent<?>>,
ManualAddFacet<ResourceComponent<?>>,
- ResourceUpgradeFacet<ResourceComponent<?>> {
-
- public static final String ESCAPE_CHARACTER_PROP_NAME = "escapeCharacter";
- private static final String ESCAPE_CHARACTER_DEFAULT =
"__TO_BE_SET_TO_\\_OR_^__";
+public class ScriptDiscoveryComponent implements
ResourceDiscoveryComponent<ResourceComponent<?>>,
ManualAddFacet<ResourceComponent<?>> {
private final Log log = LogFactory.getLog(ScriptDiscoveryComponent.class);
@@ -128,7 +124,7 @@ public class ScriptDiscoveryComponent implements
ResourceDiscoveryComponent<Reso
} else {
String args =
pluginConfig.getSimpleValue(ScriptServerComponent.PLUGINCONFIG_DESC_ARGS, null);
ProcessExecutionResults results =
ScriptServerComponent.executeExecutable(context
- .getSystemInformation(), pluginConfig, args, 5000L, true);
+ .getSystemInformation(), pluginConfig, args, 5000L, true,
ScriptServerComponent.getConfiguredEscapeCharacter(pluginConfig));
if (results != null) {
if (results.getError() != null) {
log.warn("Failed to execute cli executable to get
description. Cause: "
@@ -164,23 +160,6 @@ public class ScriptDiscoveryComponent implements
ResourceDiscoveryComponent<Reso
return description;
}
- @Override
- public ResourceUpgradeReport
upgrade(ResourceUpgradeContext<ResourceComponent<?>> inventoriedResource) {
- PropertySimple escapeCharacter =
inventoriedResource.getPluginConfiguration().getSimple(ESCAPE_CHARACTER_PROP_NAME);
-
- if (escapeCharacter != null &&
!ESCAPE_CHARACTER_DEFAULT.equals(escapeCharacter.getStringValue())) {
- return null;
- }
-
- char escapeChar = File.separatorChar == '/' ? '\\' :
'^';
-
- ResourceUpgradeReport report = new ResourceUpgradeReport();
-
report.setNewPluginConfiguration(inventoriedResource.getPluginConfiguration().clone());
- report.getNewPluginConfiguration().put(new
PropertySimple(ESCAPE_CHARACTER_PROP_NAME, Character.toString(escapeChar)));
-
- return report;
- }
-
/**
* Attempts to determine the version of the resource managed by the CLI.
*
@@ -198,7 +177,7 @@ public class ScriptDiscoveryComponent implements
ResourceDiscoveryComponent<Reso
} else {
String args =
pluginConfig.getSimpleValue(ScriptServerComponent.PLUGINCONFIG_VERSION_ARGS, null);
ProcessExecutionResults results =
ScriptServerComponent.executeExecutable(context
- .getSystemInformation(), pluginConfig, args, 5000L, true);
+ .getSystemInformation(), pluginConfig, args, 5000L, true,
ScriptServerComponent.getConfiguredEscapeCharacter(pluginConfig));
if (results != null) {
if (results.getError() != null) {
log.warn("Failed to execute cli executable to get version.
Cause: "
diff --git
a/modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptServerComponent.java
b/modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptServerComponent.java
index 7a81a6f..d62f2a1 100644
---
a/modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptServerComponent.java
+++
b/modules/plugins/script/src/main/java/org/rhq/plugins/script/ScriptServerComponent.java
@@ -78,6 +78,8 @@ public class ScriptServerComponent implements ResourceComponent,
MeasurementFace
protected static final String PLUGINCONFIG_DESC_ARGS =
"descriptionArguments";
protected static final String PLUGINCONFIG_DESC_REGEX =
"descriptionRegex";
protected static final String PLUGINCONFIG_FIXED_DESC =
"fixedDescription";
+ protected static final String PLUGINCONFIG_QUOTING_ENABLED =
"quotingEnabled";
+ protected static final String PLUGINCONFIG_ESCAPE_CHARACTER =
"escapeCharacter";
protected static final String OPERATION_PARAM_ARGUMENTS = "arguments";
protected static final String OPERATION_PARAM_WAIT_TIME = "waitTime";
@@ -90,7 +92,9 @@ public class ScriptServerComponent implements ResourceComponent,
MeasurementFace
protected static final String METRIC_PROPERTY_REGEX = "regex";
protected static final String METRIC_PROPERTY_EXITCODE = "exitcode";
- private Configuration resourceConfiguration;
+ protected static final char DISABLING_ESCAPE_CHARACTER = '\u0000';
+
+ private char escapeChar = DISABLING_ESCAPE_CHARACTER;
private ResourceContext resourceContext;
public void start(ResourceContext context) {
@@ -99,6 +103,8 @@ public class ScriptServerComponent implements ResourceComponent,
MeasurementFace
}
this.resourceContext = context;
+
+ escapeChar =
getConfiguredEscapeCharacter(resourceContext.getPluginConfiguration());
}
public void stop() {
@@ -380,7 +386,7 @@ public class ScriptServerComponent implements ResourceComponent,
MeasurementFace
SystemInfo sysInfo = this.resourceContext.getSystemInformation();
Configuration pluginConfig = this.resourceContext.getPluginConfiguration();
ProcessExecutionResults results = executeExecutable(sysInfo, pluginConfig, args,
wait, captureOutput,
- killOnTimeout);
+ killOnTimeout, escapeChar);
if (log.isDebugEnabled()) {
logDebug("CLI results: exitcode=[" + results.getExitCode() +
"]; error=[" + results.getError()
@@ -390,22 +396,36 @@ public class ScriptServerComponent implements ResourceComponent,
MeasurementFace
return results;
}
+
+ protected static char getConfiguredEscapeCharacter(Configuration pluginConfiguration)
{
+ char escapeChar = DISABLING_ESCAPE_CHARACTER;
+ boolean quotingEnabled =
Boolean.parseBoolean(pluginConfiguration.getSimpleValue(PLUGINCONFIG_QUOTING_ENABLED,
"false"));
+ if (quotingEnabled) {
+ String ec = pluginConfiguration.getSimpleValue(PLUGINCONFIG_ESCAPE_CHARACTER,
"\\");
+ escapeChar = ec.charAt(0);
+ }
+
+ return escapeChar;
+ }
+
// This is protected static so the discovery component can use it.
protected static ProcessExecutionResults executeExecutable(SystemInfo sysInfo,
Configuration pluginConfig,
- String args, long wait, boolean captureOutput) throws
InvalidPluginConfigurationException {
+ String args, long wait, boolean captureOutput, char escapeChar) throws
InvalidPluginConfigurationException {
- return executeExecutable(sysInfo, pluginConfig, args, wait, captureOutput,
true);
+ return executeExecutable(sysInfo, pluginConfig, args, wait, captureOutput, true,
escapeChar);
}
private static ProcessExecutionResults executeExecutable(SystemInfo sysInfo,
Configuration pluginConfig,
- String args, long wait, boolean captureOutput, boolean killOnTimeout)
+ String args, long wait, boolean captureOutput, boolean killOnTimeout, char
escapeChar)
throws InvalidPluginConfigurationException {
ProcessExecution processExecution = getProcessExecutionInfo(pluginConfig);
if (args != null) {
- char escapeChar =
pluginConfig.getSimpleValue(ScriptDiscoveryComponent.ESCAPE_CHARACTER_PROP_NAME,
"\\").charAt(
- 0);
- processExecution.setArguments(ScriptArgumentParser.parse(args, escapeChar));
+ if (isQuotingEnabled(escapeChar)) {
+ processExecution.setArguments(ScriptArgumentParser.parse(args,
escapeChar));
+ } else {
+ processExecution.setArguments(args.split(" "));
+ }
}
processExecution.setCaptureOutput(captureOutput);
processExecution.setWaitForCompletion(wait);
@@ -601,4 +621,8 @@ public class ScriptServerComponent implements ResourceComponent,
MeasurementFace
private void logDebug(String msg) {
log.debug("[" + this.resourceContext.getResourceKey() + "]: "
+ msg);
}
+
+ private static boolean isQuotingEnabled(char escapeChar) {
+ return escapeChar != DISABLING_ESCAPE_CHARACTER;
+ }
}
diff --git a/modules/plugins/script/src/main/resources/META-INF/rhq-plugin.xml
b/modules/plugins/script/src/main/resources/META-INF/rhq-plugin.xml
index 3f63d36..e1a96f2 100644
--- a/modules/plugins/script/src/main/resources/META-INF/rhq-plugin.xml
+++ b/modules/plugins/script/src/main/resources/META-INF/rhq-plugin.xml
@@ -42,9 +42,16 @@
<c:simple-property name="descriptionRegex"
required="false" description="The regex that can pick out the description
from the executable output. If the regex has a captured group, its matched content will be
used as the description. If there is no captured group, the entire output will be used as
the description."/>
<c:simple-property name="fixedDescription"
required="false" description="If specified, this will be the description of
the managed resource - the executable will not be invoked to determine it." />
</c:group>
- <c:group name="advanced" displayName="Advanced">
- <c:simple-property name="escapeCharacter"
required="true" description="The escape character to be used when parsing
arguments. By default this is (without quotes) '^' on Windows and '\'
everywhere else."
- default="__TO_BE_SET_TO_\_OR_^__">
+ <c:group name="argumentsParsing" displayName="Parsing of
Arguments">
+ <c:simple-property name="quotingEnabled"
displayName="Enable Quoting of Arguments" required="true"
type="boolean" default="false">
+ <c:description>
+ 1) In unquoted text: the escape character preserves the value of any
following character
+ 2) In double-quoted text: the escape character escapes " and
itself otherwise is both the escape character and the following character are left intact
+ 3) In single-quoted text: no escaping can occur
+ </c:description>
+ </c:simple-property>
+ <c:simple-property name="escapeCharacter"
required="true" description="The escape character to be used when parsing
arguments. By default the escape character is backslash."
+ default="\">
<c:constraint>
<c:regex-constraint expression="." />
</c:constraint>
diff --git
a/modules/plugins/script/src/test/java/org/rhq/plugins/script/ScriptArgumentParserTest.java
b/modules/plugins/script/src/test/java/org/rhq/plugins/script/ScriptArgumentParserTest.java
index 67a88c0..5c30200 100644
---
a/modules/plugins/script/src/test/java/org/rhq/plugins/script/ScriptArgumentParserTest.java
+++
b/modules/plugins/script/src/test/java/org/rhq/plugins/script/ScriptArgumentParserTest.java
@@ -2,6 +2,7 @@ package org.rhq.plugins.script;
import static org.testng.Assert.assertEquals;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
@@ -19,12 +20,14 @@ public class ScriptArgumentParserTest {
testCases.put("1 2\t3", new String[]{"1", "2",
"3"});
testCases.put("1 '2 ' \"3'\" '4'abs
'5\"'", new String[]{"1", "2 ", "3'",
"4abs", "5\""});
- testCases.put("\\ \\2 '3\\'a'", new String[]{"
", "2", "3'a"});
+ testCases.put("\\ \\2 '3\\'\\'a'", new String[]{"
", "2", "3\\'a"});
+ testCases.put("\"C:\\Program Files\\Lukas'
\\\"Tests\\\"\\1\"", new String[]{"C:\\Program Files\\Lukas'
\"Tests\"\\1"});
for(Map.Entry<String, String[]> testCase : testCases.entrySet()) {
String[] result = ScriptArgumentParser.parse(testCase.getKey(),
'\\');
- assertEquals(result, testCase.getValue(), "Failed to parse [" +
testCase.getKey() + "]");
+ assertEquals(result, testCase.getValue(), "Failed to parse [" +
testCase.getKey() + "]. Expected: " + Arrays
+ .asList(testCase.getValue()) + ", but got: " +
Arrays.asList(result));
}
}
}