You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@storm.apache.org by bo...@apache.org on 2017/10/02 14:50:18 UTC
[1/2] storm git commit: [STORM-2765] Disallow colons in blobstore key
name
Repository: storm
Updated Branches:
refs/heads/master 2151a2d21 -> c607d0482
[STORM-2765] Disallow colons in blobstore key name
Project: http://git-wip-us.apache.org/repos/asf/storm/repo
Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/3970122a
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/3970122a
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/3970122a
Branch: refs/heads/master
Commit: 3970122a07308346701de50e0de616a007999e77
Parents: a4fc110
Author: Ethan Li <et...@gmail.com>
Authored: Thu Sep 28 13:46:14 2017 -0500
Committer: Ethan Li <et...@gmail.com>
Committed: Mon Oct 2 08:42:23 2017 -0500
----------------------------------------------------------------------
.../jvm/org/apache/storm/blobstore/BlobStore.java | 8 ++++----
.../storm/blobstore/ClientBlobStoreTest.java | 4 ++--
.../jvm/org/apache/storm/command/Blobstore.java | 7 ++-----
.../test/clj/org/apache/storm/nimbus_test.clj | 6 +++---
.../java/org/apache/storm/utils/ServerUtils.java | 18 ------------------
5 files changed, 11 insertions(+), 32 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/storm/blob/3970122a/storm-client/src/jvm/org/apache/storm/blobstore/BlobStore.java
----------------------------------------------------------------------
diff --git a/storm-client/src/jvm/org/apache/storm/blobstore/BlobStore.java b/storm-client/src/jvm/org/apache/storm/blobstore/BlobStore.java
index 406ac8b..344a639 100644
--- a/storm-client/src/jvm/org/apache/storm/blobstore/BlobStore.java
+++ b/storm-client/src/jvm/org/apache/storm/blobstore/BlobStore.java
@@ -61,7 +61,7 @@ import org.slf4j.LoggerFactory;
*/
public abstract class BlobStore implements Shutdownable {
private static final Logger LOG = LoggerFactory.getLogger(BlobStore.class);
- private static final Pattern KEY_PATTERN = Pattern.compile("^[\\w \\t\\.:_-]+$", Pattern.UNICODE_CHARACTER_CLASS);
+ private static final Pattern KEY_PATTERN = Pattern.compile("^[\\w \\t\\._-]+$", Pattern.UNICODE_CHARACTER_CLASS);
protected static final String BASE_BLOBS_DIR_NAME = "blobs";
/**
@@ -193,10 +193,10 @@ public abstract class BlobStore implements Shutdownable {
* Validates key checking for potentially harmful patterns
* @param key Key for the blob.
*/
- public static final void validateKey(String key) throws AuthorizationException {
+ public static final void validateKey(String key) throws IllegalArgumentException {
if (StringUtils.isEmpty(key) || "..".equals(key) || ".".equals(key) || !KEY_PATTERN.matcher(key).matches()) {
- LOG.error("'{}' does not appear to be valid {}", key, KEY_PATTERN);
- throw new AuthorizationException(key+" does not appear to be a valid blob key");
+ LOG.error("'{}' does not appear to be valid. It must match {}. And it can't be \".\", \"..\", null or empty string.", key, KEY_PATTERN);
+ throw new IllegalArgumentException(key+" does not appear to be a valid blob key");
}
}
http://git-wip-us.apache.org/repos/asf/storm/blob/3970122a/storm-client/test/jvm/org/apache/storm/blobstore/ClientBlobStoreTest.java
----------------------------------------------------------------------
diff --git a/storm-client/test/jvm/org/apache/storm/blobstore/ClientBlobStoreTest.java b/storm-client/test/jvm/org/apache/storm/blobstore/ClientBlobStoreTest.java
index 9dd34b0..1a5d7b0 100644
--- a/storm-client/test/jvm/org/apache/storm/blobstore/ClientBlobStoreTest.java
+++ b/storm-client/test/jvm/org/apache/storm/blobstore/ClientBlobStoreTest.java
@@ -166,8 +166,8 @@ public class ClientBlobStoreTest {
@Test
public void testBloblStoreKeyWithUnicodesValidation() throws Exception {
- BlobStore.validateKey("msg:kafka-unicodewriterä¶µ-11-1483434711-stormconf.ser");
- BlobStore.validateKey("msg:kafka-ascii-11-148343436363-stormconf.ser");
+ BlobStore.validateKey("msg-kafka-unicodewriterä¶µ-11-1483434711-stormconf.ser");
+ BlobStore.validateKey("msg-kafka-ascii-11-148343436363-stormconf.ser");
}
private void createTestBlob(String testKey, SettableBlobMeta meta) throws AuthorizationException, KeyAlreadyExistsException {
http://git-wip-us.apache.org/repos/asf/storm/blob/3970122a/storm-core/src/jvm/org/apache/storm/command/Blobstore.java
----------------------------------------------------------------------
diff --git a/storm-core/src/jvm/org/apache/storm/command/Blobstore.java b/storm-core/src/jvm/org/apache/storm/command/Blobstore.java
index eaaedff..c6de51c 100644
--- a/storm-core/src/jvm/org/apache/storm/command/Blobstore.java
+++ b/storm-core/src/jvm/org/apache/storm/command/Blobstore.java
@@ -19,10 +19,7 @@ package org.apache.storm.command;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
-import org.apache.storm.blobstore.AtomicOutputStream;
-import org.apache.storm.blobstore.BlobStoreAclHandler;
-import org.apache.storm.blobstore.ClientBlobStore;
-import org.apache.storm.blobstore.InputStreamWithMeta;
+import org.apache.storm.blobstore.*;
import org.apache.storm.generated.AccessControl;
import org.apache.storm.generated.AuthorizationException;
import org.apache.storm.generated.KeyNotFoundException;
@@ -122,7 +119,7 @@ public class Blobstore {
SettableBlobMeta meta = new SettableBlobMeta(acl);
meta.set_replication_factor(replicationFactor);
- ServerUtils.validateKeyName(key);
+ BlobStore.validateKey(key);
LOG.info("Creating {} with ACL {}", key, generateAccessControlsInfo(acl));
http://git-wip-us.apache.org/repos/asf/storm/blob/3970122a/storm-core/test/clj/org/apache/storm/nimbus_test.clj
----------------------------------------------------------------------
diff --git a/storm-core/test/clj/org/apache/storm/nimbus_test.clj b/storm-core/test/clj/org/apache/storm/nimbus_test.clj
index 1f45f9b..42bb788 100644
--- a/storm-core/test/clj/org/apache/storm/nimbus_test.clj
+++ b/storm-core/test/clj/org/apache/storm/nimbus_test.clj
@@ -1646,9 +1646,9 @@
(doto (LocalCluster$Builder. )
(.withDaemonConf {SUPERVISOR-ENABLE false TOPOLOGY-ACKER-EXECUTORS 0 TOPOLOGY-EVENTLOGGER-EXECUTORS 0})))]
(let [nimbus (.getNimbus cluster)]
- (is (thrown-cause? AuthorizationException (.beginFileDownload nimbus nil)))
- (is (thrown-cause? AuthorizationException (.beginFileDownload nimbus "")))
- (is (thrown-cause? AuthorizationException (.beginFileDownload nimbus "/bogus-path/foo")))
+ (is (thrown-cause? IllegalArgumentException (.beginFileDownload nimbus nil)))
+ (is (thrown-cause? IllegalArgumentException (.beginFileDownload nimbus "")))
+ (is (thrown-cause? IllegalArgumentException (.beginFileDownload nimbus "/bogus-path/foo")))
)))
(deftest test-validate-topo-config-on-submit
http://git-wip-us.apache.org/repos/asf/storm/blob/3970122a/storm-server/src/main/java/org/apache/storm/utils/ServerUtils.java
----------------------------------------------------------------------
diff --git a/storm-server/src/main/java/org/apache/storm/utils/ServerUtils.java b/storm-server/src/main/java/org/apache/storm/utils/ServerUtils.java
index 7b127d2..f781d2e 100644
--- a/storm-server/src/main/java/org/apache/storm/utils/ServerUtils.java
+++ b/storm-server/src/main/java/org/apache/storm/utils/ServerUtils.java
@@ -89,12 +89,6 @@ public class ServerUtils {
public static final String DEFAULT_BLOB_VERSION_SUFFIX = ".version";
public static final int SIGKILL = 9;
public static final int SIGTERM = 15;
- /**
- * Make sure a given key name is valid for the storm config.
- * Throw RuntimeException if the key isn't valid.
- * @param name The name of the config key to check.
- */
- private static final Set<String> disallowedKeys = new HashSet<>(Arrays.asList(new String[] {"/", ".", ":", "\\"}));
// A singleton instance allows us to mock delegated static methods in our
// tests by subclassing.
@@ -637,18 +631,6 @@ public class ServerUtils {
}
}
- public static void validateKeyName(String name) {
-
- for(String key : disallowedKeys) {
- if( name.contains(key) ) {
- throw new RuntimeException("Key name cannot contain any of the following: " + disallowedKeys.toString());
- }
- }
- if(name.trim().isEmpty()) {
- throw new RuntimeException("Key name cannot be blank");
- }
- }
-
/**
* Given a zip File input it will return its size
* Only works for zip files whose uncompressed size is less than 4 GB,
[2/2] storm git commit: Merge branch 'STORM-2765' of
https://github.com/Ethanlm/storm into STORM-2765
Posted by bo...@apache.org.
Merge branch 'STORM-2765' of https://github.com/Ethanlm/storm into STORM-2765
STORM-2765: Disallow colons in blobstore key name
This closes #2351
Project: http://git-wip-us.apache.org/repos/asf/storm/repo
Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/c607d048
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/c607d048
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/c607d048
Branch: refs/heads/master
Commit: c607d04828e73f8cc4bf32b4b872318b6b6e189d
Parents: 2151a2d 3970122
Author: Robert Evans <ev...@yahoo-inc.com>
Authored: Mon Oct 2 09:20:52 2017 -0500
Committer: Robert Evans <ev...@yahoo-inc.com>
Committed: Mon Oct 2 09:20:52 2017 -0500
----------------------------------------------------------------------
.../jvm/org/apache/storm/blobstore/BlobStore.java | 8 ++++----
.../storm/blobstore/ClientBlobStoreTest.java | 4 ++--
.../jvm/org/apache/storm/command/Blobstore.java | 7 ++-----
.../test/clj/org/apache/storm/nimbus_test.clj | 6 +++---
.../java/org/apache/storm/utils/ServerUtils.java | 18 ------------------
5 files changed, 11 insertions(+), 32 deletions(-)
----------------------------------------------------------------------