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) <[email protected]>
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());
}
}
