From f7c08df5c15f01e6322a96b0c3d21077015fbdb2 Mon Sep 17 00:00:00 2001 From: Eric Zhao Date: Wed, 20 Feb 2019 14:00:14 +0800 Subject: [PATCH] Fix bug of calculating param size and amount in ParamFlowRequestDataWriter of Sentinel cluster (#495) Signed-off-by: Eric Zhao --- .../data/ParamFlowRequestDataWriter.java | 36 ++++++++- .../data/ParamFlowRequestDataWriterTest.java | 78 +++++++++++++++++++ .../cluster/flow/ClusterParamFlowChecker.java | 4 + 3 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 sentinel-cluster/sentinel-cluster-client-default/src/test/java/com/alibaba/csp/sentinel/cluster/client/codec/data/ParamFlowRequestDataWriterTest.java diff --git a/sentinel-cluster/sentinel-cluster-client-default/src/main/java/com/alibaba/csp/sentinel/cluster/client/codec/data/ParamFlowRequestDataWriter.java b/sentinel-cluster/sentinel-cluster-client-default/src/main/java/com/alibaba/csp/sentinel/cluster/client/codec/data/ParamFlowRequestDataWriter.java index 831d214d..657223e9 100644 --- a/sentinel-cluster/sentinel-cluster-client-default/src/main/java/com/alibaba/csp/sentinel/cluster/client/codec/data/ParamFlowRequestDataWriter.java +++ b/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 { + 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 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 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() {{ + add(1); + }})); + assertEquals(2, writer.calculateParamAmount(new ArrayList() {{ + add(1); add(64); + }})); + assertEquals(2, writer.calculateParamAmount(new ArrayList() {{ + add(1); add(64); add(3); + }})); + } + + @Test + public void testCalculateParamAmount() { + ParamFlowRequestDataWriter writer = new ParamFlowRequestDataWriter(); + assertEquals(6, writer.calculateParamAmount(new ArrayList() {{ + 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() {{ + add(new SomePojo()); + }})); + assertEquals(1, writer.calculateParamAmount(new ArrayList() {{ + 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 + '\'' + + '}'; + } + } +} \ No newline at end of file diff --git a/sentinel-cluster/sentinel-cluster-server-default/src/main/java/com/alibaba/csp/sentinel/cluster/flow/ClusterParamFlowChecker.java b/sentinel-cluster/sentinel-cluster-server-default/src/main/java/com/alibaba/csp/sentinel/cluster/flow/ClusterParamFlowChecker.java index 20d26845..cd8ab8a3 100644 --- a/sentinel-cluster/sentinel-cluster-server-default/src/main/java/com/alibaba/csp/sentinel/cluster/flow/ClusterParamFlowChecker.java +++ b/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;