fix: operation direction parse error (#278)

* fix: operation direction parse error

* fix format

* optimize logic

* fix format

* optimize string parse logic

* optimize string parse logic

* optimize string parse logic

* optimize string parse logic

* optimize string parse logic

* add test case

* add test case

---------

Co-authored-by: Badrish Chandramouli <badrishc@microsoft.com>
Co-authored-by: Tal Zaccai <talzacc@microsoft.com>

libs/server/Objects/List/ListObject.cs

@@ -47,6 +47,7 @@ public enum OperationDirection
         /// Right or tail
         /// </summary>
         Right,
+        Unknown,
     }
 
 

libs/server/Resp/Objects/ListCommands.cs

@@ -678,8 +678,12 @@ private unsafe bool ListMove<TGarnetApi>(int count, byte* ptr, ref TGarnetApi st
                 if (!RespReadUtils.ReadPtrWithLengthHeader(ref param2.ptr, ref param2.length, ref ptr, recvBufferPtr + bytesRead))
                     return false;
 
-                var sourceDirection = param1.ReadOnlySpan.SequenceEqual("RIGHT"u8) ? OperationDirection.Right : OperationDirection.Left;
-                var destinationDirection = param2.ReadOnlySpan.SequenceEqual("RIGHT"u8) ? OperationDirection.Right : OperationDirection.Left;
+                OperationDirection sourceDirection = GetOperationDirection(param1);
+                OperationDirection destinationDirection = GetOperationDirection(param2);
+                if (sourceDirection == OperationDirection.Unknown || destinationDirection == OperationDirection.Unknown)
+                {
+                    return AbortWithErrorMessage(count, CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR);
+                }
 
                 result = ListMove(count, sourceKey, destinationKey, sourceDirection, destinationDirection, out var node, ref storageApi);
                 if (node != null)

libs/server/Resp/Objects/ObjectStoreUtils.cs

@@ -65,5 +65,27 @@ private bool AbortWithErrorMessage(int count, ReadOnlySpan<byte> errorMessage)
 
             return true;
         }
+
+        /// <summary>
+        /// Tries to parse the input as "LEFT" or "RIGHT" and returns the corresponding OperationDirection.
+        /// If parsing fails, returns OperationDirection.Unknown.
+        /// </summary>
+        /// <param name="input">The input to parse.</param>
+        /// <returns>The parsed OperationDirection, or OperationDirection.Unknown if parsing fails.</returns>
+        public OperationDirection GetOperationDirection(ArgSlice input)
+        {
+            // Optimize for the common case
+            if (input.ReadOnlySpan.SequenceEqual("LEFT"u8))
+                return OperationDirection.Left;
+            if (input.ReadOnlySpan.SequenceEqual("RIGHT"u8))
+                return OperationDirection.Right;
+            // Rare case: try making upper case and retry
+            MakeUpperCase(input.ptr);
+            if (input.ReadOnlySpan.SequenceEqual("LEFT"u8))
+                return OperationDirection.Left;
+            if (input.ReadOnlySpan.SequenceEqual("RIGHT"u8))
+                return OperationDirection.Right;
+            return OperationDirection.Unknown;
+        }
     }
 }

test/Garnet.test/RespListTests.cs

@@ -638,6 +638,13 @@ public async Task CanUseLMoveGC()
             using var db = TestUtils.GetGarnetClient();
             db.Connect();
 
+            // Test for Operation direction error.
+            var exception = Assert.ThrowsAsync<Exception>(async () =>
+            {
+                await db.ExecuteForStringResultAsync("LMOVE", new string[] { "mylist", "myotherlist", "right", "lef" });
+            });
+            Assert.AreEqual("ERR syntax error", exception.Message);
+
             //If source does not exist, the value nil is returned and no operation is performed.
             var response = await db.ExecuteForStringResultAsync("LMOVE", new string[] { "mylist", "myotherlist", "RIGHT", "LEFT" });
             Assert.AreEqual(null, response);
@@ -681,6 +688,39 @@ public async Task CanUseLMoveGC()
             Assert.AreEqual(expectedResponseArray, responseArray);
         }
 
+        [Test]
+        public async Task CanUseLMoveWithCaseInsensitiveDirectionGC()
+        {
+            using var db = TestUtils.GetGarnetClient();
+            db.Connect();
+
+            await db.ExecuteForStringResultAsync("RPUSH", new string[] { "mylist", "one" });
+            await db.ExecuteForStringResultAsync("RPUSH", new string[] { "mylist", "two" });
+            await db.ExecuteForStringResultAsync("RPUSH", new string[] { "mylist", "three" });
+
+            var response = await db.ExecuteForStringResultAsync("LMOVE", new string[] { "mylist", "myotherlist", "right", "left" });
+            Assert.AreEqual("three", response);
+
+            var responseArray = await db.ExecuteForStringArrayResultAsync("LRANGE", new string[] { "mylist", "0", "-1" });
+            var expectedResponseArray = new string[] { "one", "two" };
+            Assert.AreEqual(expectedResponseArray, responseArray);
+
+            responseArray = await db.ExecuteForStringArrayResultAsync("LRANGE", new string[] { "myotherlist", "0", "-1" });
+            expectedResponseArray = new string[] { "three" };
+            Assert.AreEqual(expectedResponseArray, responseArray);
+
+            response = await db.ExecuteForStringResultAsync("LMOVE", new string[] { "mylist", "myotherlist", "LeFT", "RIghT" });
+            Assert.AreEqual("one", response);
+
+            responseArray = await db.ExecuteForStringArrayResultAsync("LRANGE", new string[] { "mylist", "0", "-1" });
+            expectedResponseArray = new string[] { "two" };
+            Assert.AreEqual(expectedResponseArray, responseArray);
+
+            responseArray = await db.ExecuteForStringArrayResultAsync("LRANGE", new string[] { "myotherlist", "0", "-1" });
+            expectedResponseArray = new string[] { "three", "one" };
+            Assert.AreEqual(expectedResponseArray, responseArray);
+        }
+
         [Test]
         public async Task CanUseLMoveWithCancellationTokenGC()
         {