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 2021/10/01 17:54:41 UTC

[storm] branch 1.x-branch updated: [MINOR] Remove unused getGroupsCommand method

This is an automated email from the ASF dual-hosted git repository.

ethanli pushed a commit to branch 1.x-branch
in repository https://gitbox.apache.org/repos/asf/storm.git


The following commit(s) were added to refs/heads/1.x-branch by this push:
     new 53411db  [MINOR] Remove unused getGroupsCommand method
53411db is described below

commit 53411db6d7311f3a2afb67396c5a5e441b957794
Author: Meng Li (Ethan) <et...@gmail.com>
AuthorDate: Wed Sep 29 13:56:51 2021 -0500

    [MINOR] Remove unused getGroupsCommand method
---
 conf/defaults.yaml                                     |  2 +-
 docs/STORM-UI-REST-API.md                              |  4 ++--
 docs/Serialization.md                                  |  4 ++--
 storm-core/src/jvm/org/apache/storm/Config.java        |  2 +-
 .../storm/security/auth/ShellBasedGroupsMapping.java   |  5 +++++
 .../src/jvm/org/apache/storm/utils/ShellUtils.java     | 18 +++++-------------
 6 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/conf/defaults.yaml b/conf/defaults.yaml
index 3a8db94..ddeee81 100644
--- a/conf/defaults.yaml
+++ b/conf/defaults.yaml
@@ -244,7 +244,7 @@ topology.max.spout.pending: null
 topology.state.synchronization.timeout.secs: 60
 topology.stats.sample.rate: 0.05
 topology.builtin.metrics.bucket.size.secs: 60
-topology.fall.back.on.java.serialization: true
+topology.fall.back.on.java.serialization: false
 topology.worker.childopts: null
 topology.worker.logwriter.childopts: "-Xmx64m"
 topology.executor.receive.buffer.size: 1024 #batched
diff --git a/docs/STORM-UI-REST-API.md b/docs/STORM-UI-REST-API.md
index 334402c..73a6ff3 100644
--- a/docs/STORM-UI-REST-API.md
+++ b/docs/STORM-UI-REST-API.md
@@ -63,7 +63,7 @@ Sample response (does not include all the data fields):
     "dev.zookeeper.path": "/tmp/dev-storm-zookeeper",
     "topology.tick.tuple.freq.secs": null,
     "topology.builtin.metrics.bucket.size.secs": 60,
-    "topology.fall.back.on.java.serialization": true,
+    "topology.fall.back.on.java.serialization": false,
     "topology.max.error.report.per.interval": 5,
     "zmq.linger.millis": 5000,
     "topology.skip.missing.kryo.registrations": false,
@@ -651,7 +651,7 @@ Sample response:
         "dev.zookeeper.path": "/tmp/dev-storm-zookeeper",
         "topology.tick.tuple.freq.secs": null,
         "topology.builtin.metrics.bucket.size.secs": 60,
-        "topology.fall.back.on.java.serialization": true,
+        "topology.fall.back.on.java.serialization": false,
         "topology.max.error.report.per.interval": 5,
         "zmq.linger.millis": 5000,
         "topology.skip.missing.kryo.registrations": false,
diff --git a/docs/Serialization.md b/docs/Serialization.md
index c2e129b..1e75741 100644
--- a/docs/Serialization.md
+++ b/docs/Serialization.md
@@ -47,11 +47,11 @@ There's an advanced config called `Config.TOPOLOGY_SKIP_MISSING_KRYO_REGISTRATIO
 
 ### Java serialization
 
-If Storm encounters a type for which it doesn't have a serialization registered, it will use Java serialization if possible. If the object can't be serialized with Java serialization, then Storm will throw an error.
+When `Config.TOPOLOGY_FALL_BACK_ON_JAVA_SERIALIZATION` is set true, if Storm encounters a type for which it doesn't have a serialization registered, it will use Java serialization if possible. If the object can't be serialized with Java serialization, then Storm will throw an error.
 
 Beware that Java serialization is extremely expensive, both in terms of CPU cost as well as the size of the serialized object. It is highly recommended that you register custom serializers when you put the topology in production. The Java serialization behavior is there so that it's easy to prototype new topologies.
 
-You can turn off the behavior to fall back on Java serialization by setting the `Config.TOPOLOGY_FALL_BACK_ON_JAVA_SERIALIZATION` config to false.
+You can turn on/off the behavior to fall back on Java serialization by setting the `Config.TOPOLOGY_FALL_BACK_ON_JAVA_SERIALIZATION` config to true/false. The default value is false for security reasons.
 
 ### Component-specific serialization registrations
 
diff --git a/storm-core/src/jvm/org/apache/storm/Config.java b/storm-core/src/jvm/org/apache/storm/Config.java
index 4849b97..ed88132 100644
--- a/storm-core/src/jvm/org/apache/storm/Config.java
+++ b/storm-core/src/jvm/org/apache/storm/Config.java
@@ -1917,7 +1917,7 @@ public class Config extends HashMap<String, Object> {
     public static final String TOPOLOGY_BUILTIN_METRICS_BUCKET_SIZE_SECS="topology.builtin.metrics.bucket.size.secs";
 
     /**
-     * Whether or not to use Java serialization in a topology.
+     * Whether or not to use Java serialization in a topology. Default is set false for security reasons.
      */
     @isBoolean
     public static final String TOPOLOGY_FALL_BACK_ON_JAVA_SERIALIZATION="topology.fall.back.on.java.serialization";
diff --git a/storm-core/src/jvm/org/apache/storm/security/auth/ShellBasedGroupsMapping.java b/storm-core/src/jvm/org/apache/storm/security/auth/ShellBasedGroupsMapping.java
index 5d7f702..b5f0f52 100644
--- a/storm-core/src/jvm/org/apache/storm/security/auth/ShellBasedGroupsMapping.java
+++ b/storm-core/src/jvm/org/apache/storm/security/auth/ShellBasedGroupsMapping.java
@@ -73,6 +73,11 @@ public class ShellBasedGroupsMapping implements
      * @throws IOException if encounter any error when running the command
      */
     private static Set<String> getUnixGroups(final String user) throws IOException {
+        if (user == null) {
+            LOG.debug("User is null. Returning an empty set as the result");
+            return new HashSet<>();
+        }
+
         String result;
         try {
             result = ShellUtils.execCommand(ShellUtils.getGroupsForUserCommand(user));
diff --git a/storm-core/src/jvm/org/apache/storm/utils/ShellUtils.java b/storm-core/src/jvm/org/apache/storm/utils/ShellUtils.java
index 3b0f934..8509457 100644
--- a/storm-core/src/jvm/org/apache/storm/utils/ShellUtils.java
+++ b/storm-core/src/jvm/org/apache/storm/utils/ShellUtils.java
@@ -134,25 +134,17 @@ abstract public class ShellUtils {
         this.dir = dir;
     }
 
-    /** a Unix command to get the current user's groups list */
-    public static String[] getGroupsCommand() {
-        return (WINDOWS)? new String[]{"cmd", "/c", "groups"}
-        : new String[]{"bash", "-c", "groups"};
-    }
-
     /**
-     * a Unix command to get a given user's groups list.
-     * If the OS is not WINDOWS, the command will get the user's primary group
-     * first and finally get the groups list which includes the primary group.
-     * i.e. the user's primary group will be included twice.
+     * a Unix command to get a given user's groups list. Windows is not supported.
      */
     public static String[] getGroupsForUserCommand(final String user) {
+        if (WINDOWS) {
+            throw new UnsupportedOperationException("Getting user groups is not supported on Windows");
+        }
         //'groups username' command return is non-consistent across different unixes
-        return new String [] {"bash", "-c", "id -gn " + user
-                         + "&& id -Gn " + user};
+        return new String[]{"id", "-Gn", user};
     }
 
-
     /** check to see if a command needs to be executed and execute if needed */
     protected void run() throws IOException {
         if (lastTime + interval > System.currentTimeMillis())