From 7997cf65fc4f9a292ab4d9b8f494ae1611bd47dc Mon Sep 17 00:00:00 2001 From: Eric Zhao Date: Tue, 18 Jun 2019 15:11:05 +0800 Subject: [PATCH] Fix the numeric overflow bug of ping response data in the cluster module (#844) - Change type of cluster ping data response from byte to int. This change is compatible with old versions Signed-off-by: Eric Zhao --- .../sentinel-cluster-client-default/pom.xml | 5 ++ .../codec/data/PingResponseDataDecoder.java | 7 ++- .../data/PingResponseDataDecoderTest.java | 44 +++++++++++++++++ .../sentinel-cluster-server-default/pom.xml | 5 ++ .../codec/data/PingResponseDataWriter.java | 2 +- .../data/PingResponseDataWriterTest.java | 48 +++++++++++++++++++ 6 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 sentinel-cluster/sentinel-cluster-client-default/src/test/java/com/alibaba/csp/sentinel/cluster/client/codec/data/PingResponseDataDecoderTest.java create mode 100644 sentinel-cluster/sentinel-cluster-server-default/src/test/java/com/alibaba/csp/sentinel/cluster/server/codec/data/PingResponseDataWriterTest.java diff --git a/sentinel-cluster/sentinel-cluster-client-default/pom.xml b/sentinel-cluster/sentinel-cluster-client-default/pom.xml index 80bf32df..ae85e33a 100644 --- a/sentinel-cluster/sentinel-cluster-client-default/pom.xml +++ b/sentinel-cluster/sentinel-cluster-client-default/pom.xml @@ -53,5 +53,10 @@ mockito-core test + + org.assertj + assertj-core + test + \ No newline at end of file diff --git a/sentinel-cluster/sentinel-cluster-client-default/src/main/java/com/alibaba/csp/sentinel/cluster/client/codec/data/PingResponseDataDecoder.java b/sentinel-cluster/sentinel-cluster-client-default/src/main/java/com/alibaba/csp/sentinel/cluster/client/codec/data/PingResponseDataDecoder.java index e12395f0..73c4a62b 100644 --- a/sentinel-cluster/sentinel-cluster-client-default/src/main/java/com/alibaba/csp/sentinel/cluster/client/codec/data/PingResponseDataDecoder.java +++ b/sentinel-cluster/sentinel-cluster-client-default/src/main/java/com/alibaba/csp/sentinel/cluster/client/codec/data/PingResponseDataDecoder.java @@ -27,9 +27,14 @@ public class PingResponseDataDecoder implements EntityDecoder @Override public Integer decode(ByteBuf source) { - if (source.readableBytes() >= 1) { + int size = source.readableBytes(); + if (size == 1) { + // Compatible with old version (< 1.7.0). return (int) source.readByte(); } + if (size >= 4) { + return source.readInt(); + } return -1; } } diff --git a/sentinel-cluster/sentinel-cluster-client-default/src/test/java/com/alibaba/csp/sentinel/cluster/client/codec/data/PingResponseDataDecoderTest.java b/sentinel-cluster/sentinel-cluster-client-default/src/test/java/com/alibaba/csp/sentinel/cluster/client/codec/data/PingResponseDataDecoderTest.java new file mode 100644 index 00000000..f2b04ec6 --- /dev/null +++ b/sentinel-cluster/sentinel-cluster-client-default/src/test/java/com/alibaba/csp/sentinel/cluster/client/codec/data/PingResponseDataDecoderTest.java @@ -0,0 +1,44 @@ +/* + * Copyright 1999-2018 Alibaba Group Holding Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.alibaba.csp.sentinel.cluster.client.codec.data; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Eric Zhao + */ +public class PingResponseDataDecoderTest { + + @Test + public void testDecodePingResponseData() { + ByteBuf buf = Unpooled.buffer(); + PingResponseDataDecoder decoder = new PingResponseDataDecoder(); + + int big = Integer.MAX_VALUE; + buf.writeInt(big); + assertThat(decoder.decode(buf)).isEqualTo(big); + + byte small = 12; + buf.writeByte(small); + assertThat(decoder.decode(buf)).isEqualTo(small); + + buf.release(); + } +} diff --git a/sentinel-cluster/sentinel-cluster-server-default/pom.xml b/sentinel-cluster/sentinel-cluster-server-default/pom.xml index 6522b710..cd7cdc65 100644 --- a/sentinel-cluster/sentinel-cluster-server-default/pom.xml +++ b/sentinel-cluster/sentinel-cluster-server-default/pom.xml @@ -52,5 +52,10 @@ mockito-core test + + org.assertj + assertj-core + test + \ No newline at end of file diff --git a/sentinel-cluster/sentinel-cluster-server-default/src/main/java/com/alibaba/csp/sentinel/cluster/server/codec/data/PingResponseDataWriter.java b/sentinel-cluster/sentinel-cluster-server-default/src/main/java/com/alibaba/csp/sentinel/cluster/server/codec/data/PingResponseDataWriter.java index 26e31ecd..f380b61d 100644 --- a/sentinel-cluster/sentinel-cluster-server-default/src/main/java/com/alibaba/csp/sentinel/cluster/server/codec/data/PingResponseDataWriter.java +++ b/sentinel-cluster/sentinel-cluster-server-default/src/main/java/com/alibaba/csp/sentinel/cluster/server/codec/data/PingResponseDataWriter.java @@ -31,6 +31,6 @@ public class PingResponseDataWriter implements EntityWriter { if (entity == null || target == null) { return; } - target.writeByte(entity); + target.writeInt(entity); } } diff --git a/sentinel-cluster/sentinel-cluster-server-default/src/test/java/com/alibaba/csp/sentinel/cluster/server/codec/data/PingResponseDataWriterTest.java b/sentinel-cluster/sentinel-cluster-server-default/src/test/java/com/alibaba/csp/sentinel/cluster/server/codec/data/PingResponseDataWriterTest.java new file mode 100644 index 00000000..96191cfa --- /dev/null +++ b/sentinel-cluster/sentinel-cluster-server-default/src/test/java/com/alibaba/csp/sentinel/cluster/server/codec/data/PingResponseDataWriterTest.java @@ -0,0 +1,48 @@ +/* + * Copyright 1999-2019 Alibaba Group Holding Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.alibaba.csp.sentinel.cluster.server.codec.data; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Test cases for {@link PingResponseDataWriter}. + * + * @author Eric Zhao + */ +public class PingResponseDataWriterTest { + + @Test + public void testWritePingResponseAndParse() { + ByteBuf buf = Unpooled.buffer(); + PingResponseDataWriter writer = new PingResponseDataWriter(); + + int small = 120; + writer.writeTo(small, buf); + assertThat(buf.readableBytes()).isGreaterThanOrEqualTo(4); + assertThat(buf.readInt()).isEqualTo(small); + + int big = Integer.MAX_VALUE; + writer.writeTo(big, buf); + assertThat(buf.readableBytes()).isGreaterThanOrEqualTo(4); + assertThat(buf.readInt()).isEqualTo(big); + + buf.release(); + } +}