Use concurrent skiplist map for NettyRequest arguments

linkedin · Oct 24, 2024 · a5dba9c · a5dba9c
1 parent 639fa06
commit a5dba9c
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 4 deletions.
diff --git a/ambry-rest/src/main/java/com/github/ambry/rest/NettyRequest.java b/ambry-rest/src/main/java/com/github/ambry/rest/NettyRequest.java
@@ -41,7 +41,7 @@
 import java.util.Map;
 import java.util.Queue;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentSkipListMap;
 import java.util.concurrent.Future;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -69,7 +69,10 @@ public class NettyRequest implements RestRequest {
   protected final HttpRequest request;
   protected final Channel channel;
   protected final NettyMetrics nettyMetrics;
-  protected final Map<String, Object> allArgs = new ConcurrentHashMap<>();
+  // This allArgs map needs to be
+  // 1. case-insensitive, as headers and query parameters in http request should be case-insensitive
+  // 2. thread safe, as we might access/update the map in different threads.
+  protected final Map<String, Object> allArgs = new ConcurrentSkipListMap<>(String.CASE_INSENSITIVE_ORDER);
   protected final Queue<HttpContent> requestContents = new LinkedBlockingQueue<>();
   protected final ReentrantLock contentLock = new ReentrantLock();
 

diff --git a/ambry-rest/src/test/java/com/github/ambry/rest/NettyRequestTest.java b/ambry-rest/src/test/java/com/github/ambry/rest/NettyRequestTest.java
@@ -754,7 +754,15 @@ private void validateRequest(NettyRequest nettyRequest, RestMethod restMethod, S
 
     for (Map.Entry<String, String> e : headers) {
       if (!e.getKey().equalsIgnoreCase(HttpHeaderNames.COOKIE.toString())) {
+        // Make sure we have this key in receivedArgs
         assertTrue("Did not find key: " + e.getKey(), receivedArgs.containsKey(e.getKey()));
+        // Now make sure we can find a case-insensitive key in the request
+        String lowerKey = e.getKey().toLowerCase();
+        assertTrue("Case insensitive key should exist for lower key" + e.getKey(),
+            nettyRequest.getArgs().containsKey(lowerKey));
+        String upperKey = e.getKey().toUpperCase();
+        assertTrue("Case insensitive key should exist for upper key" + e.getKey(),
+            nettyRequest.getArgs().containsKey(upperKey));
         if (!keyValueCount.containsKey(e.getKey())) {
           keyValueCount.put(e.getKey(), 0);
         }
@@ -769,8 +777,7 @@ private void validateRequest(NettyRequest nettyRequest, RestMethod restMethod, S
     assertEquals("Number of args does not match", keyValueCount.size(), receivedArgs.size());
     for (Map.Entry<String, Integer> e : keyValueCount.entrySet()) {
       assertEquals("Value count for key " + e.getKey() + " does not match",
-          e.getValue() == 0 ? 1 : e.getValue().intValue(),
-          receivedArgs.get(e.getKey()).size());
+          e.getValue() == 0 ? 1 : e.getValue().intValue(), receivedArgs.get(e.getKey()).size());
     }
 
     assertEquals("Auto-read is in an invalid state",