Fix cache lookup for negative long values #2019

We now correctly check the cache bounds for negative integer arguments.
redis · Feb 25, 2022 · b034541 · b034541
1 parent 2324f46
commit b034541
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 9 deletions.
diff --git a/src/main/java/io/lettuce/core/protocol/CommandArgs.java b/src/main/java/io/lettuce/core/protocol/CommandArgs.java
@@ -22,11 +22,11 @@
 import java.util.List;
 import java.util.Map;
 
-import io.lettuce.core.internal.LettuceStrings;
 import io.lettuce.core.codec.RedisCodec;
 import io.lettuce.core.codec.StringCodec;
 import io.lettuce.core.codec.ToByteBufEncoder;
 import io.lettuce.core.internal.LettuceAssert;
+import io.lettuce.core.internal.LettuceStrings;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.UnpooledByteBufAllocator;
 
@@ -497,7 +497,7 @@ static IntegerArgument of(long val) {
                 return IntegerCache.cache[(int) val];
             }
 
-            if (val < 0 && -val < IntegerCache.cache.length) {
+            if (val < 0 && val > Integer.MIN_VALUE && -val < IntegerCache.cache.length) {
                 return IntegerCache.negativeCache[(int) -val];
             }
 

diff --git a/src/test/java/io/lettuce/core/cluster/topology/TopologyRefreshIntegrationTests.java b/src/test/java/io/lettuce/core/cluster/topology/TopologyRefreshIntegrationTests.java
@@ -15,15 +15,13 @@
  */
 package io.lettuce.core.cluster.topology;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.*;
 
 import java.time.Duration;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BiFunction;
 
 import javax.inject.Inject;
@@ -32,7 +30,6 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
-import io.lettuce.test.ReflectionTestUtils;
 
 import io.lettuce.category.SlowTests;
 import io.lettuce.core.RedisClient;
@@ -55,7 +52,6 @@
 import io.lettuce.test.Wait;
 import io.lettuce.test.resource.FastShutdown;
 import io.lettuce.test.settings.TestSettings;
-import io.netty.util.concurrent.ScheduledFuture;
 
 /**
  * Test for topology refreshing.
@@ -276,10 +272,11 @@ void adaptiveTriggerOnMoveRedirection() {
         assertThat(clusterClient.getPartitions().getPartitionByNodeId(node1.getNodeId()).getSlots()).hasSize(0);
         assertThat(clusterClient.getPartitions().getPartitionByNodeId(node2.getNodeId()).getSlots()).hasSize(16384);
 
-        clusterConnection.set("b", value); // slot 3300
+        Future<String> b = connection.reactive().set("b", value).toFuture();// slot 3300
 
         Wait.untilEquals(12000, () -> clusterClient.getPartitions().getPartitionByNodeId(node1.getNodeId()).getSlots().size())
                 .waitOrTimeout();
+        System.out.println(b);
 
         assertThat(clusterClient.getPartitions().getPartitionByNodeId(node1.getNodeId()).getSlots()).hasSize(12000);
         assertThat(clusterClient.getPartitions().getPartitionByNodeId(node2.getNodeId()).getSlots()).hasSize(4384);

diff --git a/src/test/java/io/lettuce/core/protocol/CommandArgsUnitTests.java b/src/test/java/io/lettuce/core/protocol/CommandArgsUnitTests.java
@@ -15,7 +15,7 @@
  */
 package io.lettuce.core.protocol;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.*;
 
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
@@ -29,6 +29,8 @@
 import io.netty.buffer.Unpooled;
 
 /**
+ * Unit tests for {@link CommandArgs}.
+ *
  * @author Mark Paluch
  */
 class CommandArgsUnitTests {
@@ -58,6 +60,25 @@ void getFirstIntegerShouldReturnFirstNegativeInteger() {
         assertThat(CommandArgsAccessor.getFirstInteger(args)).isEqualTo(-1L);
     }
 
+    @Test
+    void getFirstIntegerShouldReturnFirstPositiveLong() {
+
+        CommandArgs<String, String> args = new CommandArgs<>(StringCodec.UTF8).add(Long.MAX_VALUE);
+
+        assertThat(CommandArgsAccessor.getFirstInteger(args)).isEqualTo(Long.MAX_VALUE);
+    }
+
+    @Test
+    void getFirstIntegerShouldReturnFirstNegativeLong() {
+
+        assertThat(CommandArgsAccessor.getFirstInteger(new CommandArgs<>(StringCodec.UTF8).add(Long.MIN_VALUE)))
+                .isEqualTo(Long.MIN_VALUE);
+        assertThat(CommandArgsAccessor.getFirstInteger(new CommandArgs<>(StringCodec.UTF8).add(Long.MIN_VALUE + 2)))
+                .isEqualTo(Long.MIN_VALUE + 2);
+        assertThat(CommandArgsAccessor.getFirstInteger(new CommandArgs<>(StringCodec.UTF8).add(Integer.MIN_VALUE)))
+                .isEqualTo(Integer.MIN_VALUE);
+    }
+
     @Test
     void getFirstStringShouldReturnNull() {