Fix bug of calculating param size and amount in ParamFlowRequestDataWriter of Sentinel cluster (#495)

Signed-off-by: Eric Zhao <sczyh16@gmail.com>

sentinel-cluster/sentinel-cluster-client-default/src/main/java/com/alibaba/csp/sentinel/cluster/client/codec/data/ParamFlowRequestDataWriter.java

@@ -20,6 +20,8 @@ import java.util.Collection;
 import com.alibaba.csp.sentinel.cluster.ClusterConstants;
 import com.alibaba.csp.sentinel.cluster.codec.EntityWriter;
 import com.alibaba.csp.sentinel.cluster.request.data.ParamFlowRequestData;
+import com.alibaba.csp.sentinel.log.RecordLog;
+import com.alibaba.csp.sentinel.util.AssertUtil;
 
 import io.netty.buffer.ByteBuf;
 
@@ -30,6 +32,17 @@ import io.netty.buffer.ByteBuf;
  */
 public class ParamFlowRequestDataWriter implements EntityWriter<ParamFlowRequestData, ByteBuf> {
 
+    private final int maxParamByteSize;
+
+    public ParamFlowRequestDataWriter() {
+        this(DEFAULT_PARAM_MAX_SIZE);
+    }
+
+    public ParamFlowRequestDataWriter(int maxParamByteSize) {
+        AssertUtil.isTrue(maxParamByteSize > 0, "maxParamByteSize should be positive");
+        this.maxParamByteSize = maxParamByteSize;
+    }
+
     @Override
     public void writeTo(ParamFlowRequestData entity, ByteBuf target) {
         target.writeLong(entity.getFlowId());
@@ -84,20 +97,35 @@ public class ParamFlowRequestDataWriter implements EntityWriter<ParamFlowRequest
         target.writeBytes(tmpChars);
     }
 
-    private int calculateParamAmount(/*@NonEmpty*/ Collection<Object> params) {
+    /**
+     * Calculate amount of valid parameters in provided parameter list.
+     *
+     * @param params non-empty parameter list
+     * @return amount of valid parameters
+     */
+    int calculateParamAmount(/*@NonEmpty*/ Collection<Object> params) {
         int size = 0;
         int length = 0;
         for (Object param : params) {
             int s = calculateParamTransportSize(param);
-            if (size + s > PARAM_MAX_SIZE) {
+            if (s <= 0) {
+                RecordLog.warn("[ParamFlowRequestDataWriter] WARN: Non-primitive type detected in params of "
+                    + "cluster parameter flow control, which is not supported: " + param);
+                continue;
+            }
+            if (size + s > maxParamByteSize) {
                 break;
             }
+            size += s;
             length++;
         }
         return length;
     }
 
-    private int calculateParamTransportSize(Object value) {
+    int calculateParamTransportSize(Object value) {
+        if (value == null) {
+            return 0;
+        }
         // Layout for primitives: |type flag(1)|value|
         // size = original size + type flag (1)
         if (value instanceof Integer || int.class.isInstance(value)) {
@@ -125,5 +153,5 @@ public class ParamFlowRequestDataWriter implements EntityWriter<ParamFlowRequest
         }
     }
 
-    private static final int PARAM_MAX_SIZE = 1000;
+    private static final int DEFAULT_PARAM_MAX_SIZE = 1024;
 }

sentinel-cluster/sentinel-cluster-client-default/src/test/java/com/alibaba/csp/sentinel/cluster/client/codec/data/ParamFlowRequestDataWriterTest.java

@@ -0,0 +1,78 @@
+package com.alibaba.csp.sentinel.cluster.client.codec.data;
+
+import java.util.ArrayList;
+
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+/**
+ * @author Eric Zhao
+ */
+public class ParamFlowRequestDataWriterTest {
+
+    @Test
+    public void testCalculateParamTransportSize() {
+        ParamFlowRequestDataWriter writer = new ParamFlowRequestDataWriter();
+        // POJO (non-primitive type) should not be regarded as a valid parameter.
+        assertEquals(0, writer.calculateParamTransportSize(new SomePojo().setParam1("abc")));
+
+        assertEquals(4 + 1, writer.calculateParamTransportSize(1));
+        assertEquals(1 + 1, writer.calculateParamTransportSize((byte) 1));
+        assertEquals(1 + 1, writer.calculateParamTransportSize(false));
+        assertEquals(8 + 1, writer.calculateParamTransportSize(2L));
+        assertEquals(8 + 1, writer.calculateParamTransportSize(4.0d));
+        final String paramStr = "Sentinel";
+        assertEquals(1 + 4 + paramStr.getBytes().length, writer.calculateParamTransportSize(paramStr));
+    }
+
+    @Test
+    public void testCalculateParamAmountExceedsMaxSize() {
+        final int maxSize = 10;
+        ParamFlowRequestDataWriter writer = new ParamFlowRequestDataWriter(maxSize);
+        assertEquals(1, writer.calculateParamAmount(new ArrayList<Object>() {{
+            add(1);
+        }}));
+        assertEquals(2, writer.calculateParamAmount(new ArrayList<Object>() {{
+            add(1); add(64);
+        }}));
+        assertEquals(2, writer.calculateParamAmount(new ArrayList<Object>() {{
+            add(1); add(64); add(3);
+        }}));
+    }
+
+    @Test
+    public void testCalculateParamAmount() {
+        ParamFlowRequestDataWriter writer = new ParamFlowRequestDataWriter();
+        assertEquals(6, writer.calculateParamAmount(new ArrayList<Object>() {{
+            add(1); add(1d); add(1f); add((byte) 1); add("123"); add(true);
+        }}));
+        // POJO (non-primitive type) should not be regarded as a valid parameter.
+        assertEquals(0, writer.calculateParamAmount(new ArrayList<Object>() {{
+            add(new SomePojo());
+        }}));
+        assertEquals(1, writer.calculateParamAmount(new ArrayList<Object>() {{
+            add(new Object()); add(1);
+        }}));
+    }
+
+    private static class SomePojo {
+        private String param1;
+
+        public String getParam1() {
+            return param1;
+        }
+
+        public SomePojo setParam1(String param1) {
+            this.param1 = param1;
+            return this;
+        }
+
+        @Override
+        public String toString() {
+            return "SomePojo{" +
+                "param1='" + param1 + '\'' +
+                '}';
+        }
+    }
+}

sentinel-cluster/sentinel-cluster-server-default/src/main/java/com/alibaba/csp/sentinel/cluster/flow/ClusterParamFlowChecker.java

@@ -51,6 +51,10 @@ public final class ClusterParamFlowChecker {
             // Unexpected state, return FAIL.
             return new TokenResult(TokenResultStatus.FAIL);
         }
+        if (values == null || values.isEmpty()) {
+            // Empty parameter list will always pass.
+            return new TokenResult(TokenResultStatus.OK);
+        }
         double remaining = -1;
         boolean hasPassed = true;
         Object blockObject = null;