You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@storm.apache.org by et...@apache.org on 2020/04/09 20:32:55 UTC
[storm] branch master updated: [STORM-3619] Add null check for
topology name
This is an automated email from the ASF dual-hosted git repository.
ethanli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/storm.git
The following commit(s) were added to refs/heads/master by this push:
new be63269 [STORM-3619] Add null check for topology name
new ae5b70f Merge pull request #3247 from Ethanlm/STORM-3619
be63269 is described below
commit be63269cc338078c4bfb3d88f2732dcae672aab8
Author: Meng Li (Ethan) <et...@gmail.com>
AuthorDate: Wed Apr 8 15:54:50 2020 -0500
[STORM-3619] Add null check for topology name
---
.../src/jvm/org/apache/storm/StormSubmitter.java | 13 ++++++------
.../src/jvm/org/apache/storm/utils/Utils.java | 24 +++++++++++++++++-----
.../org/apache/storm/daemon/nimbus/Nimbus.java | 8 ++++----
3 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/storm-client/src/jvm/org/apache/storm/StormSubmitter.java b/storm-client/src/jvm/org/apache/storm/StormSubmitter.java
index a31ef00..d391784 100644
--- a/storm-client/src/jvm/org/apache/storm/StormSubmitter.java
+++ b/storm-client/src/jvm/org/apache/storm/StormSubmitter.java
@@ -220,6 +220,10 @@ public class StormSubmitter {
public static void submitTopologyAs(String name, Map<String, Object> topoConf, StormTopology topology, SubmitOptions opts,
ProgressListener progressListener, String asUser)
throws AlreadyAliveException, InvalidTopologyException, AuthorizationException, IllegalArgumentException {
+
+ //validate topology name first; nothing else should be done if it's invalid.
+ Utils.validateTopologyName(name);
+
if (!Utils.isValidConf(topoConf)) {
throw new IllegalArgumentException("Storm conf is not valid. Must be json-serializable");
}
@@ -248,11 +252,8 @@ public class StormSubmitter {
try {
String serConf = JSONValue.toJSONString(topoConf);
try (NimbusClient client = NimbusClient.getConfiguredClientAs(conf, asUser)) {
- if (topologyNameExists(name, client)) {
- throw new RuntimeException("Topology with name `" + name + "` already exists on cluster");
- }
- if (!Utils.isValidKey(name)) {
- throw new IllegalArgumentException(name + " does not appear to be a valid topology name.");
+ if (isTopologyNameAllowed(name, client)) {
+ throw new RuntimeException("Topology with name `" + name + "` is either not allowed or it already exists on cluster");
}
// Dependency uploading only makes sense for distributed mode
@@ -435,7 +436,7 @@ public class StormSubmitter {
});
}
- private static boolean topologyNameExists(String name, NimbusClient client) {
+ private static boolean isTopologyNameAllowed(String name, NimbusClient client) {
try {
return !client.getClient().isTopologyNameAllowed(name);
} catch (Exception e) {
diff --git a/storm-client/src/jvm/org/apache/storm/utils/Utils.java b/storm-client/src/jvm/org/apache/storm/utils/Utils.java
index f90a606..ff656fa 100644
--- a/storm-client/src/jvm/org/apache/storm/utils/Utils.java
+++ b/storm-client/src/jvm/org/apache/storm/utils/Utils.java
@@ -122,7 +122,9 @@ public class Utils {
// tests by subclassing.
private static Utils _instance = new Utils();
private static String memoizedLocalHostnameString = null;
- public static final Pattern TOPOLOGY_KEY_PATTERN = Pattern.compile("^[\\w \\t\\._-]+$", Pattern.UNICODE_CHARACTER_CLASS);
+ public static final Pattern BLOB_KEY_PATTERN =
+ Pattern.compile("^[\\w \\t\\._-]+$", Pattern.UNICODE_CHARACTER_CLASS);
+ private static final Pattern TOPOLOGY_NAME_REGEX = Pattern.compile("^[^/.:\\\\]+$");
static {
localConf = readStormConfig();
@@ -1755,20 +1757,32 @@ public class Utils {
}
/**
- * Validates topology name / blob key.
+ * Validates blob key.
*
- * @param key topology name / Key for the blob.
+ * @param key Key for the blob.
*/
public static boolean isValidKey(String key) {
- if (StringUtils.isEmpty(key) || "..".equals(key) || ".".equals(key) || !TOPOLOGY_KEY_PATTERN.matcher(key).matches()) {
+ if (StringUtils.isEmpty(key) || "..".equals(key) || ".".equals(key) || !BLOB_KEY_PATTERN.matcher(key).matches()) {
LOG.error("'{}' does not appear to be valid. It must match {}. And it can't be \".\", \"..\", null or empty string.", key,
- TOPOLOGY_KEY_PATTERN);
+ BLOB_KEY_PATTERN);
return false;
}
return true;
}
/**
+ * Validates topology name.
+ * @param name the topology name
+ * @throws IllegalArgumentException if the topology name is not valid
+ */
+ public static void validateTopologyName(String name) throws IllegalArgumentException {
+ if (name == null || !TOPOLOGY_NAME_REGEX.matcher(name).matches()) {
+ String message = "Topology name '" + name + "' is not valid. It can't be null and it must match " + TOPOLOGY_NAME_REGEX;
+ throw new IllegalArgumentException(message);
+ }
+ }
+
+ /**
* A thread that can answer if it is sleeping in the case of simulated time. This class is not useful when simulated time is not being
* used.
*/
diff --git a/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java b/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
index 8153bf6..d2fea7c 100644
--- a/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
+++ b/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
@@ -398,7 +398,6 @@ public class Nimbus implements Iface, Shutdownable, DaemonCommon {
.build();
private static final List<String> EMPTY_STRING_LIST = Collections.unmodifiableList(Collections.emptyList());
private static final Set<String> EMPTY_STRING_SET = Collections.unmodifiableSet(Collections.emptySet());
- private static final Pattern TOPOLOGY_NAME_REGEX = Pattern.compile("^[^/.:\\\\]+$");
private static final RotatingMap<String, Long> topologyCleanupDetected = new RotatingMap<>(2);
private static long topologyCleanupRotationTime = 0L;
@@ -1185,9 +1184,10 @@ public class Nimbus implements Iface, Shutdownable, DaemonCommon {
}
private static void validateTopologyName(String name) throws InvalidTopologyException {
- Matcher m = TOPOLOGY_NAME_REGEX.matcher(name);
- if (!m.matches()) {
- throw new WrappedInvalidTopologyException("Topology name must match " + TOPOLOGY_NAME_REGEX);
+ try {
+ Utils.validateTopologyName(name);
+ } catch (IllegalArgumentException e) {
+ throw new WrappedInvalidTopologyException(e.getMessage());
}
}