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 <[email protected]> Authored: Thu Sep 28 13:46:14 2017 -0500 Committer: Ethan Li <[email protected]> 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,
