From d433f3bc3f0f74f857b7dafab641f46da3f275eb Mon Sep 17 00:00:00 2001
From: Gary Tully <gary.tully@gmail.com>
Date: Mon, 9 May 2022 19:38:19 +0100
Subject: [PATCH] ARTEMIS-3823 - limit modulo operation to positive hash values

---
 .../policies/ConsistentHashModuloPolicy.java  |  3 +-
 .../ConsistentHashModuloPolicyTest.java       | 39 +++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/routing/policies/ConsistentHashModuloPolicy.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/routing/policies/ConsistentHashModuloPolicy.java
index a9af756a2d..95ea4c3b1f 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/routing/policies/ConsistentHashModuloPolicy.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/routing/policies/ConsistentHashModuloPolicy.java
@@ -35,13 +35,14 @@ public class ConsistentHashModuloPolicy extends ConsistentHashPolicy {
 
    @Override
    public void init(Map<String, String> properties) {
+      super.init(properties);
       modulo = Integer.parseInt(properties.get(MODULO));
    }
 
    @Override
    public String transformKey(String key) {
       if (modulo > 0) {
-         return String.valueOf(getHash(key) % modulo);
+         return String.valueOf((getHash(key) & Integer.MAX_VALUE) % modulo);
       }
 
       return key;
diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/routing/policies/ConsistentHashModuloPolicyTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/routing/policies/ConsistentHashModuloPolicyTest.java
index 492259df75..f53b534703 100644
--- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/routing/policies/ConsistentHashModuloPolicyTest.java
+++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/routing/policies/ConsistentHashModuloPolicyTest.java
@@ -24,6 +24,7 @@ import org.junit.Assert;
 import org.junit.Test;
 
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 public class ConsistentHashModuloPolicyTest {
@@ -52,4 +53,42 @@ public class ConsistentHashModuloPolicyTest {
       assertNotEquals(v1, v2);
       assertTrue(v1 < modulo && v2 < modulo);
    }
+
+   @Test
+   public void transformKeyNotNegative() {
+      ConsistentHashModuloPolicy underTest = new ConsistentHashModuloPolicy();
+      HashMap<String, String> properties = new HashMap<>();
+      final int modulo = 2;
+      properties.put(ConsistentHashModuloPolicy.MODULO, String.valueOf(modulo));
+      underTest.init(properties);
+
+      assertNotNull(underTest.getProperties());
+
+      String[] values = new String[]{"ONE", "TWO", "THREE", "FOUR"};
+      for (String v : values) {
+         assertTrue("non negative for: " + v, Integer.valueOf(underTest.transformKey(v)) >= 0);
+      }
+   }
+
+   @Test
+   public void transformKeyNotNegativeWithExplicitNegativeHash() {
+      final int[] negs = {-1, Integer.MAX_VALUE, Integer.MIN_VALUE, 100, 500, 22, 2, 1};
+      ConsistentHashModuloPolicy underTest = new ConsistentHashModuloPolicy() {
+         int v = 0;
+         @Override
+         protected int getHash(String str) {
+            return negs[v++ % negs.length];
+         }
+      };
+      HashMap<String, String> properties = new HashMap<>();
+      final int modulo = 2;
+      properties.put(ConsistentHashModuloPolicy.MODULO, String.valueOf(modulo));
+      underTest.init(properties);
+
+      assertNotNull(underTest.getProperties());
+
+      for (int i = 0; i < negs.length; i++) {
+         assertTrue("non negative for: " + i, Integer.valueOf(underTest.transformKey("BLA")) >= 0);
+      }
+   }
 }
\ No newline at end of file