You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by nc...@apache.org on 2016/02/11 14:47:05 UTC
[05/17] ambari git commit: AMBARI-14984 - Script Alert Parameters
With Shell Characters Are Not Properly Escaped (jonathanhurley)
AMBARI-14984 - Script Alert Parameters With Shell Characters Are Not Properly Escaped (jonathanhurley)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/5e6a5556
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/5e6a5556
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/5e6a5556
Branch: refs/heads/branch-dev-patch-upgrade
Commit: 5e6a5556b2a40435c6d35d7851e88d896350f53d
Parents: 946ac82
Author: Jonathan Hurley <jh...@hortonworks.com>
Authored: Tue Feb 9 18:38:55 2016 -0500
Committer: Jonathan Hurley <jh...@hortonworks.com>
Committed: Wed Feb 10 15:12:42 2016 -0500
----------------------------------------------------------------------
ambari-funtest/pom.xml | 5 --
ambari-project/pom.xml | 2 +-
ambari-server/pom.xml | 5 --
.../dispatchers/AlertScriptDispatcher.java | 18 +++++-
.../services/AlertNoticeDispatchService.java | 2 +-
.../state/services/CachedAlertFlushService.java | 2 +-
.../dispatchers/AlertScriptDispatcherTest.java | 61 ++++++++++++++++++++
7 files changed, 80 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/ambari/blob/5e6a5556/ambari-funtest/pom.xml
----------------------------------------------------------------------
diff --git a/ambari-funtest/pom.xml b/ambari-funtest/pom.xml
index b9c591f..4fa342f 100644
--- a/ambari-funtest/pom.xml
+++ b/ambari-funtest/pom.xml
@@ -496,11 +496,6 @@
<version>4.2.5</version>
</dependency>
<dependency>
- <groupId>com.google.guava</groupId>
- <artifactId>guava</artifactId>
- <version>14.0.1</version>
- </dependency>
- <dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
<version>1.3.9</version>
http://git-wip-us.apache.org/repos/asf/ambari/blob/5e6a5556/ambari-project/pom.xml
----------------------------------------------------------------------
diff --git a/ambari-project/pom.xml b/ambari-project/pom.xml
index f19ca14..ed94004 100644
--- a/ambari-project/pom.xml
+++ b/ambari-project/pom.xml
@@ -220,7 +220,7 @@
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
- <version>14.0.1</version>
+ <version>16.0</version>
</dependency>
<dependency>
<groupId>com.google.code.findbugs</groupId>
http://git-wip-us.apache.org/repos/asf/ambari/blob/5e6a5556/ambari-server/pom.xml
----------------------------------------------------------------------
diff --git a/ambari-server/pom.xml b/ambari-server/pom.xml
index b500fb2..51fd88b 100644
--- a/ambari-server/pom.xml
+++ b/ambari-server/pom.xml
@@ -1179,11 +1179,6 @@
<version>4.2.5</version>
</dependency>
<dependency>
- <groupId>com.google.guava</groupId>
- <artifactId>guava</artifactId>
- <version>14.0.1</version>
- </dependency>
- <dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
<version>1.3.9</version>
http://git-wip-us.apache.org/repos/asf/ambari/blob/5e6a5556/ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java b/ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
index 092aaf4..907588d 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
@@ -40,6 +40,8 @@ import org.apache.commons.lang.SystemUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.escape.Escaper;
+import com.google.common.escape.Escapers;
import com.google.inject.Inject;
/**
@@ -82,6 +84,18 @@ public class AlertScriptDispatcher implements NotificationDispatcher {
private static final long DEFAULT_SCRIPT_TIMEOUT = 5000L;
/**
+ * Used to escape text being passed into the shell command.
+ */
+ public static final Escaper SHELL_ESCAPE;
+
+ static {
+ final Escapers.Builder builder = Escapers.builder();
+ builder.addEscape('\"', "\\\"");
+ builder.addEscape('!', "\\!");
+ SHELL_ESCAPE = builder.build();
+ }
+
+ /**
* Configuration data from the ambari.properties file.
*/
@Inject
@@ -242,8 +256,8 @@ public class AlertScriptDispatcher implements NotificationDispatcher {
// these could have spaces in them, so quote them so they don't mess up the
// command line
- String alertLabel = "\"" + definition.getLabel() + "\"";
- String alertText = "\"" + alertInfo.getAlertText() + "\"";
+ String alertLabel = "\"" + SHELL_ESCAPE.escape(definition.getLabel()) + "\"";
+ String alertText = "\"" + SHELL_ESCAPE.escape(alertInfo.getAlertText()) + "\"";
Object[] params = new Object[] { script, definitionName, alertLabel, serviceName,
alertState.name(), alertText };
http://git-wip-us.apache.org/repos/asf/ambari/blob/5e6a5556/ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java b/ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java
index 3a88f37..a27bc1d 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java
@@ -712,7 +712,7 @@ public class AlertNoticeDispatchService extends AbstractScheduledService {
*
* @param history
*/
- protected AlertInfo(AlertHistoryEntity history) {
+ public AlertInfo(AlertHistoryEntity history) {
m_history = history;
}
http://git-wip-us.apache.org/repos/asf/ambari/blob/5e6a5556/ambari-server/src/main/java/org/apache/ambari/server/state/services/CachedAlertFlushService.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/services/CachedAlertFlushService.java b/ambari-server/src/main/java/org/apache/ambari/server/state/services/CachedAlertFlushService.java
index 72bf68a..2e38c9b 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/services/CachedAlertFlushService.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/services/CachedAlertFlushService.java
@@ -76,7 +76,7 @@ public class CachedAlertFlushService extends AbstractScheduledService {
protected void startUp() throws Exception {
boolean enabled = m_configuration.isAlertCacheEnabled();
if (!enabled) {
- stop();
+ stopAsync();
}
}
http://git-wip-us.apache.org/repos/asf/ambari/blob/5e6a5556/ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java b/ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
index 12141c9..9e0e406 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
@@ -20,6 +20,7 @@ package org.apache.ambari.server.notifications.dispatchers;
import java.lang.reflect.Field;
import java.util.Collections;
import java.util.HashMap;
+import java.util.List;
import java.util.UUID;
import java.util.concurrent.Executor;
@@ -28,8 +29,12 @@ import org.apache.ambari.server.notifications.DispatchCallback;
import org.apache.ambari.server.notifications.DispatchFactory;
import org.apache.ambari.server.notifications.Notification;
import org.apache.ambari.server.notifications.NotificationDispatcher;
+import org.apache.ambari.server.orm.entities.AlertDefinitionEntity;
+import org.apache.ambari.server.orm.entities.AlertHistoryEntity;
+import org.apache.ambari.server.state.AlertState;
import org.apache.ambari.server.state.alert.AlertNotification;
import org.apache.ambari.server.state.alert.TargetType;
+import org.apache.ambari.server.state.services.AlertNoticeDispatchService.AlertInfo;
import org.apache.ambari.server.state.stack.OsFamily;
import org.easymock.EasyMock;
import org.junit.Before;
@@ -44,6 +49,8 @@ import com.google.inject.Guice;
import com.google.inject.Inject;
import com.google.inject.Injector;
+import junit.framework.Assert;
+
/**
* Tests {@link AlertScriptDispatcher}.
*/
@@ -217,6 +224,60 @@ public class AlertScriptDispatcherTest {
}
/**
+ * Tests that arguments given to the {@link ProcessBuilder} are properly
+ * escaped.
+ *
+ * @throws Exception
+ */
+ @Test
+ public void testArgumentEscaping() throws Exception {
+ final String ALERT_DEFINITION_NAME = "mock_alert_with_quotes";
+ final String ALERT_DEFINITION_LABEL = "Mock alert with Quotes";
+ final String ALERT_LABEL = "Alert Label";
+ final String ALERT_SERVICE_NAME = "FOO_SERVICE";
+ final String ALERT_TEXT = "Did you know, \"Quotes are hard!!!\"";
+ final String ALERT_TEXT_ESCAPED = "Did you know, \\\"Quotes are hard\\!\\!\\!\\\"";
+
+ DispatchCallback callback = EasyMock.createNiceMock(DispatchCallback.class);
+ AlertNotification notification = new AlertNotification();
+ notification.Callback = callback;
+ notification.CallbackIds = Collections.singletonList(UUID.randomUUID().toString());
+
+ AlertDefinitionEntity definition = new AlertDefinitionEntity();
+ definition.setDefinitionName(ALERT_DEFINITION_NAME);
+ definition.setLabel(ALERT_DEFINITION_LABEL);
+
+ AlertHistoryEntity history = new AlertHistoryEntity();
+ history.setAlertDefinition(definition);
+ history.setAlertLabel(ALERT_LABEL);
+ history.setAlertText(ALERT_TEXT);
+ history.setAlertState(AlertState.OK);
+ history.setServiceName(ALERT_SERVICE_NAME);
+
+ AlertInfo alertInfo = new AlertInfo(history);
+ notification.setAlertInfo(alertInfo);
+
+ AlertScriptDispatcher dispatcher = new AlertScriptDispatcher();
+ m_injector.injectMembers(dispatcher);
+
+ ProcessBuilder processBuilder = dispatcher.getProcessBuilder(SCRIPT_CONFIG_VALUE, notification);
+ List<String> commands = processBuilder.command();
+ Assert.assertEquals(3, commands.size());
+ Assert.assertEquals("sh", commands.get(0));
+ Assert.assertEquals("-c", commands.get(1));
+
+ StringBuilder buffer = new StringBuilder();
+ buffer.append(SCRIPT_CONFIG_VALUE).append(" ");
+ buffer.append(ALERT_DEFINITION_NAME).append(" ");
+ buffer.append("\"").append(ALERT_DEFINITION_LABEL).append("\"").append(" ");
+ buffer.append(ALERT_SERVICE_NAME).append(" ");
+ buffer.append(AlertState.OK).append(" ");
+ buffer.append("\"").append(ALERT_TEXT_ESCAPED).append("\"");
+
+ Assert.assertEquals(buffer.toString(), commands.get(2));
+ }
+
+ /**
*
*/
private class MockModule extends AbstractModule {