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/09/28 20:52:21 UTC

[storm] branch 2.1.x-branch updated (cbf07d9 -> 58320c6)

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

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


    from cbf07d9  [STORM-3629] Logviewer should always allow admins to access logs
     new 3b6ae55  [STORM-3767] fix NPE in getComponentPendingProfileActions
     new 58320c6  [STORM-3767] Follow up: fix NullPointerException in getUnixGroups

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 conf/defaults.yaml                                     |  2 +-
 docs/STORM-UI-REST-API.md                              |  4 ++--
 docs/Serialization.md                                  |  4 ++--
 storm-client/src/jvm/org/apache/storm/Config.java      |  2 +-
 .../apache/storm/messaging/netty/MessageDecoder.java   | 11 ++++++++++-
 .../storm/security/auth/ShellBasedGroupsMapping.java   |  5 +++++
 .../src/jvm/org/apache/storm/utils/ShellUtils.java     | 18 ++----------------
 .../java/org/apache/storm/daemon/nimbus/Nimbus.java    | 12 +++++++-----
 8 files changed, 30 insertions(+), 28 deletions(-)

[storm] 01/02: [STORM-3767] fix NPE in getComponentPendingProfileActions

Posted by et...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 3b6ae55c34aae443aee0a500df501518ae8ffbb1
Author: Meng (Ethan) Li <et...@gmail.com>
AuthorDate: Wed Sep 15 23:52:32 2021 -0500

    [STORM-3767] fix NPE in getComponentPendingProfileActions
---
 conf/defaults.yaml                                     |  2 +-
 docs/STORM-UI-REST-API.md                              |  4 ++--
 docs/Serialization.md                                  |  4 ++--
 storm-client/src/jvm/org/apache/storm/Config.java      |  2 +-
 .../apache/storm/messaging/netty/MessageDecoder.java   | 11 ++++++++++-
 .../src/jvm/org/apache/storm/utils/ShellUtils.java     | 18 ++----------------
 .../java/org/apache/storm/daemon/nimbus/Nimbus.java    | 12 +++++++-----
 7 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/conf/defaults.yaml b/conf/defaults.yaml
index ee5fe56..a55bc72 100644
--- a/conf/defaults.yaml
+++ b/conf/defaults.yaml
@@ -264,7 +264,7 @@ topology.max.spout.pending: null    # ideally should be larger than topology.pro
 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.tick.tuple.freq.secs: null
diff --git a/docs/STORM-UI-REST-API.md b/docs/STORM-UI-REST-API.md
index 469855b..1a1b63a 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,
@@ -728,7 +728,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 e35a0f9..47f0606 100644
--- a/docs/Serialization.md
+++ b/docs/Serialization.md
@@ -53,11 +53,11 @@ You may use this like any other service loader and storm will register the bindi
 
 ### 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-client/src/jvm/org/apache/storm/Config.java b/storm-client/src/jvm/org/apache/storm/Config.java
index 979b287..926b612 100644
--- a/storm-client/src/jvm/org/apache/storm/Config.java
+++ b/storm-client/src/jvm/org/apache/storm/Config.java
@@ -475,7 +475,7 @@ public class Config extends HashMap<String, Object> {
     @IsInteger
     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-client/src/jvm/org/apache/storm/messaging/netty/MessageDecoder.java b/storm-client/src/jvm/org/apache/storm/messaging/netty/MessageDecoder.java
index bced87c..a3b282a 100644
--- a/storm-client/src/jvm/org/apache/storm/messaging/netty/MessageDecoder.java
+++ b/storm-client/src/jvm/org/apache/storm/messaging/netty/MessageDecoder.java
@@ -19,9 +19,12 @@ import org.apache.storm.serialization.KryoValuesDeserializer;
 import org.apache.storm.shade.io.netty.buffer.ByteBuf;
 import org.apache.storm.shade.io.netty.channel.ChannelHandlerContext;
 import org.apache.storm.shade.io.netty.handler.codec.ByteToMessageDecoder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class MessageDecoder extends ByteToMessageDecoder {
 
+    private static final Logger LOG = LoggerFactory.getLogger(MessageDecoder.class);
     private final KryoValuesDeserializer deser;
 
     public MessageDecoder(KryoValuesDeserializer deser) {
@@ -164,4 +167,10 @@ public class MessageDecoder extends ByteToMessageDecoder {
             out.add(ret);
         }
     }
-}
+
+    @Override
+    public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
+        LOG.error("Exception thrown while decoding messages in channel {}; exception: ", ctx.channel(), cause);
+        ctx.close();
+    }
+}
\ No newline at end of file
diff --git a/storm-client/src/jvm/org/apache/storm/utils/ShellUtils.java b/storm-client/src/jvm/org/apache/storm/utils/ShellUtils.java
index 9e141d7..5d9da25 100644
--- a/storm-client/src/jvm/org/apache/storm/utils/ShellUtils.java
+++ b/storm-client/src/jvm/org/apache/storm/utils/ShellUtils.java
@@ -103,28 +103,14 @@ public abstract class ShellUtils {
     }
 
     /**
-     * a Unix command to get the current user's groups list.
-     */
-    public static String[] getGroupsCommand() {
-        if (WINDOWS) {
-            throw new UnsupportedOperationException("Getting user groups is not supported on Windows");
-        }
-        return 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};
     }
 
     private static void joinThread(Thread t) {
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 37bd01a..52f841b 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
@@ -3450,12 +3450,14 @@ public class Nimbus implements Iface, Shutdownable, DaemonCommon {
         try {
             getComponentPendingProfileActionsCalls.mark();
             CommonTopoInfo info = getCommonTopoInfo(id, "getComponentPendingProfileActions");
-            Map<String, String> nodeToHost = info.assignment.get_node_host();
             Map<List<? extends Number>, List<Object>> exec2hostPort = new HashMap<>();
-            for (Entry<List<Long>, NodeInfo> entry : info.assignment.get_executor_node_port().entrySet()) {
-                NodeInfo ni = entry.getValue();
-                List<Object> hostPort = Arrays.asList(nodeToHost.get(ni.get_node()), ni.get_port_iterator().next().intValue());
-                exec2hostPort.put(entry.getKey(), hostPort);
+            if (info.assignment != null) {
+                Map<String, String> nodeToHost = info.assignment.get_node_host();
+                for (Entry<List<Long>, NodeInfo> entry : info.assignment.get_executor_node_port().entrySet()) {
+                    NodeInfo ni = entry.getValue();
+                    List<Object> hostPort = Arrays.asList(nodeToHost.get(ni.get_node()), ni.get_port_iterator().next().intValue());
+                    exec2hostPort.put(entry.getKey(), hostPort);
+                }
             }
             List<Map<String, Object>> nodeInfos =
                 StatsUtil.extractNodeInfosFromHbForComp(exec2hostPort, info.taskToComponent, false, componentId);

[storm] 02/02: [STORM-3767] Follow up: fix NullPointerException in getUnixGroups

Posted by et...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 58320c6888055b0f45c8069e304d35b563b5dfc4
Author: Meng (Ethan) Li <et...@gmail.com>
AuthorDate: Mon Sep 20 20:23:10 2021 -0500

    [STORM-3767] Follow up: fix NullPointerException in getUnixGroups
---
 .../jvm/org/apache/storm/security/auth/ShellBasedGroupsMapping.java  | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/storm-client/src/jvm/org/apache/storm/security/auth/ShellBasedGroupsMapping.java b/storm-client/src/jvm/org/apache/storm/security/auth/ShellBasedGroupsMapping.java
index 31d00dc..c80e293 100644
--- a/storm-client/src/jvm/org/apache/storm/security/auth/ShellBasedGroupsMapping.java
+++ b/storm-client/src/jvm/org/apache/storm/security/auth/ShellBasedGroupsMapping.java
@@ -105,6 +105,11 @@ public class ShellBasedGroupsMapping implements
      * @throws IOException if encounter any error when running the command
      */
     private 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 = shellCommandRunner.execCommand(ShellUtils.getGroupsForUserCommand(user));