Adding type validation logic to object operations (microsoft#450)

* Initial support for WRONGTYPE * nit * nit * Added wrong type checks for list & set objects * Added hash commands * Added sorted set commands * Fixing some tests * Some fixes * Added sorted set tests * Adding geo commands + custom object commands * Moving helper methods to RespTestsUtils * Fixing comment --------- Co-authored-by: Badrish Chandramouli <badrishc@microsoft.com>
PayEx · Jun 12, 2024 · 8f46a57 · 8f46a57
1 parent 66ebaa7
commit 8f46a57
Show file tree

Hide file tree

Showing 31 changed files with 1,482 additions and 549 deletions.
diff --git a/libs/server/API/GarnetApiObjectCommands.cs b/libs/server/API/GarnetApiObjectCommands.cs
@@ -213,7 +213,7 @@ public GarnetStatus ListLength(byte[] key, ArgSlice input, out ObjectOutputHeade
             => storageSession.ListLength(key, input, out output, ref objectContext);
 
         /// <inheritdoc />
-        public bool ListMove(ArgSlice source, ArgSlice destination, OperationDirection sourceDirection, OperationDirection destinationDirection, out byte[] element)
+        public GarnetStatus ListMove(ArgSlice source, ArgSlice destination, OperationDirection sourceDirection, OperationDirection destinationDirection, out byte[] element)
             => storageSession.ListMove(source, destination, sourceDirection, destinationDirection, out element);
 
         /// <inheritdoc />

diff --git a/libs/server/API/GarnetStatus.cs b/libs/server/API/GarnetStatus.cs
@@ -19,6 +19,10 @@ public enum GarnetStatus : byte
         /// <summary>
         /// Moved
         /// </summary>
-        MOVED
+        MOVED,
+        /// <summary>
+        /// Wrong type
+        /// </summary>
+        WRONGTYPE
     }
 }
diff --git a/libs/server/API/IGarnetApi.cs b/libs/server/API/IGarnetApi.cs
@@ -723,8 +723,8 @@ public interface IGarnetApi : IGarnetReadApi, IGarnetAdvancedApi
         /// <param name="sourceDirection"></param>
         /// <param name="destinationDirection"></param>
         /// <param name="element">The element being popped and pushed</param>
-        /// <returns>true when success</returns>
-        public bool ListMove(ArgSlice sourceKey, ArgSlice destinationKey, OperationDirection sourceDirection, OperationDirection destinationDirection, out byte[] element);
+        /// <returns>GarnetStatus</returns>
+        public GarnetStatus ListMove(ArgSlice sourceKey, ArgSlice destinationKey, OperationDirection sourceDirection, OperationDirection destinationDirection, out byte[] element);
 
         /// <summary>
         /// Trim an existing list so it only contains the specified range of elements.

diff --git a/libs/server/Custom/CustomObjectBase.cs b/libs/server/Custom/CustomObjectBase.cs
@@ -208,6 +208,12 @@ public sealed override unsafe bool Operate(ref SpanByte input, ref SpanByteAndMe
                         }
                     break;
                 default:
+                    if ((byte)header->type != this.type)
+                    {
+                        // Indicates an incorrect type of key
+                        output.Length = 0;
+                        return true;
+                    }
                     (IMemoryOwner<byte> Memory, int Length) outp = (output.Memory, 0);
                     Operate(header->SubId, input.AsReadOnlySpan().Slice(RespInputHeader.Size), ref outp, out removeKey);
                     output.Memory = outp.Memory;

diff --git a/libs/server/Custom/CustomRespCommands.cs b/libs/server/Custom/CustomRespCommands.cs
@@ -165,31 +165,44 @@ private bool TryCustomObjectCommand<TGarnetApi>(byte* ptr, byte* end, RespComman
                 status = storageApi.RMW_ObjectStore(ref key, ref Unsafe.AsRef<SpanByte>(inputPtr), ref output);
                 Debug.Assert(!output.spanByteAndMemory.IsSpanByte);
 
-                if (output.spanByteAndMemory.Memory != null)
-                    SendAndReset(output.spanByteAndMemory.Memory, output.spanByteAndMemory.Length);
-                else
-                    while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend))
-                        SendAndReset();
+                switch (status)
+                {
+                    case GarnetStatus.WRONGTYPE:
+                        while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend))
+                            SendAndReset();
+                        break;
+                    default:
+                        if (output.spanByteAndMemory.Memory != null)
+                            SendAndReset(output.spanByteAndMemory.Memory, output.spanByteAndMemory.Length);
+                        else
+                            while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend))
+                                SendAndReset();
+                        break;
+                }
             }
             else
             {
                 status = storageApi.Read_ObjectStore(ref key, ref Unsafe.AsRef<SpanByte>(inputPtr), ref output);
                 Debug.Assert(!output.spanByteAndMemory.IsSpanByte);
 
-                if (status == GarnetStatus.OK)
+                switch (status)
                 {
-                    if (output.spanByteAndMemory.Memory != null)
-                        SendAndReset(output.spanByteAndMemory.Memory, output.spanByteAndMemory.Length);
-                    else
-                        while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend))
+                    case GarnetStatus.OK:
+                        if (output.spanByteAndMemory.Memory != null)
+                            SendAndReset(output.spanByteAndMemory.Memory, output.spanByteAndMemory.Length);
+                        else
+                            while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend))
+                                SendAndReset();
+                        break;
+                    case GarnetStatus.NOTFOUND:
+                        Debug.Assert(output.spanByteAndMemory.Memory == null);
+                        while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_ERRNOTFOUND, ref dcurr, dend))
                             SendAndReset();
-
-                }
-                else
-                {
-                    Debug.Assert(output.spanByteAndMemory.Memory == null);
-                    while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_ERRNOTFOUND, ref dcurr, dend))
-                        SendAndReset();
+                        break;
+                    case GarnetStatus.WRONGTYPE:
+                        while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_WRONG_TYPE, ref dcurr, dend))
+                            SendAndReset();
+                        break;
                 }
             }
 

diff --git a/libs/server/Objects/List/ListObject.cs b/libs/server/Objects/List/ListObject.cs
@@ -136,7 +136,7 @@ public override unsafe bool Operate(ref SpanByte input, ref SpanByteAndMemory ou
                 var header = (RespInputHeader*)_input;
                 if (header->type != GarnetObjectType.List)
                 {
-                    //Indicates an incorrect type of key
+                    // Indicates an incorrect type of key
                     output.Length = 0;
                     sizeChange = 0;
                     return true;

diff --git a/libs/server/Objects/Set/SetObject.cs b/libs/server/Objects/Set/SetObject.cs
@@ -110,7 +110,15 @@ public override unsafe bool Operate(ref SpanByte input, ref SpanByteAndMemory ou
             fixed (byte* _output = output.SpanByte.AsSpan())
             {
                 var header = (RespInputHeader*)_input;
-                Debug.Assert(header->type == GarnetObjectType.Set);
+                if (header->type != GarnetObjectType.Set)
+                {
+                    // Indicates an incorrect type of key
+                    output.Length = 0;
+                    sizeChange = 0;
+                    removeKey = false;
+                    return true;
+                }
+
                 long prevSize = this.Size;
                 switch (header->SetOp)
                 {

diff --git a/libs/server/Objects/SortedSet/SortedSetObject.cs b/libs/server/Objects/SortedSet/SortedSetObject.cs
@@ -183,8 +183,16 @@ public override unsafe bool Operate(ref SpanByte input, ref SpanByteAndMemory ou
             fixed (byte* _output = output.SpanByte.AsSpan())
             {
                 var header = (RespInputHeader*)_input;
-                Debug.Assert(header->type == GarnetObjectType.SortedSet);
-                long previouseSize = this.Size;
+                if (header->type != GarnetObjectType.SortedSet)
+                {
+                    // Indicates an incorrect type of key
+                    output.Length = 0;
+                    sizeChange = 0;
+                    removeKey = false;
+                    return true;
+                }
+
+                long prevSize = this.Size;
                 switch (header->SortedSetOp)
                 {
                     case SortedSetOperation.ZADD:
@@ -269,7 +277,7 @@ public override unsafe bool Operate(ref SpanByte input, ref SpanByteAndMemory ou
                     default:
                         throw new GarnetException($"Unsupported operation {(SortedSetOperation)_input[0]} in SortedSetObject.Operate");
                 }
-                sizeChange = this.Size - previouseSize;
+                sizeChange = this.Size - prevSize;
             }
 
             removeKey = sortedSetDict.Count == 0;

diff --git a/libs/server/Resp/CmdStrings.cs b/libs/server/Resp/CmdStrings.cs
@@ -142,6 +142,8 @@ static partial class CmdStrings
         public static ReadOnlySpan<byte> RESP_ERR_UNSUPPORTED_PROTOCOL_VERSION => "ERR Unsupported protocol version"u8;
         public static ReadOnlySpan<byte> RESP_ERR_ASYNC_PROTOCOL_CHANGE => "ERR protocol change is not allowed with pending async operations"u8;
         public static ReadOnlySpan<byte> RESP_ERR_NOT_VALID_FLOAT => "ERR value is not a valid float"u8;
+        public static ReadOnlySpan<byte> RESP_ERR_MIN_MAX_NOT_VALID_FLOAT => "ERR min or max is not a float"u8;
+        public static ReadOnlySpan<byte> RESP_ERR_MIN_MAX_NOT_VALID_STRING => "ERR min or max not valid string range item"u8;
         public static ReadOnlySpan<byte> RESP_WRONGPASS_INVALID_PASSWORD => "WRONGPASS Invalid password"u8;
         public static ReadOnlySpan<byte> RESP_WRONGPASS_INVALID_USERNAME_PASSWORD => "WRONGPASS Invalid username/password combination"u8;
         public static ReadOnlySpan<byte> RESP_SYNTAX_ERROR => "ERR syntax error"u8;