From 5f03e16060679c9eb04e546797e9ae5da7dd50af Mon Sep 17 00:00:00 2001 From: prvyk Date: Fri, 31 Jan 2025 21:52:43 +0200 Subject: [PATCH 1/4] Accept lower case script load --- libs/server/Resp/CmdStrings.cs | 3 ++ libs/server/Resp/Parser/RespCommand.cs | 40 ++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/libs/server/Resp/CmdStrings.cs b/libs/server/Resp/CmdStrings.cs index bdafb73705..c389e80c5e 100644 --- a/libs/server/Resp/CmdStrings.cs +++ b/libs/server/Resp/CmdStrings.cs @@ -55,6 +55,7 @@ static partial class CmdStrings public static ReadOnlySpan HELP => "HELP"u8; public static ReadOnlySpan help => "help"u8; public static ReadOnlySpan PING => "PING"u8; + public static ReadOnlySpan SCRIPT => "SCRIPT"u8; public static ReadOnlySpan HELLO => "HELLO"u8; public static ReadOnlySpan TIME => "TIME"u8; public static ReadOnlySpan RESET => "RESET"u8; @@ -316,6 +317,8 @@ static partial class CmdStrings // subcommand parsing strings public static ReadOnlySpan CAT => "CAT"u8; public static ReadOnlySpan DELUSER => "DELUSER"u8; + public static ReadOnlySpan EXISTS => "EXISTS"u8; + public static ReadOnlySpan FLUSH => "FLUSH"u8; public static ReadOnlySpan LOAD => "LOAD"u8; public static ReadOnlySpan LOADCS => "LOADCS"u8; public static ReadOnlySpan SETUSER => "SETUSER"u8; diff --git a/libs/server/Resp/Parser/RespCommand.cs b/libs/server/Resp/Parser/RespCommand.cs index dbb14372ce..1c422c166b 100644 --- a/libs/server/Resp/Parser/RespCommand.cs +++ b/libs/server/Resp/Parser/RespCommand.cs @@ -1686,6 +1686,46 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci { return RespCommand.RUNTXP; } + else if (command.SequenceEqual(CmdStrings.SCRIPT)) + { + if (count == 0) + { + specificErrorMsg = Encoding.ASCII.GetBytes(string.Format(CmdStrings.GenericErrWrongNumArgs, + nameof(RespCommand.SCRIPT))); + return RespCommand.INVALID; + } + + Span subCommand = GetCommand(out bool gotSubCommand); + if (!gotSubCommand) + { + success = false; + return RespCommand.NONE; + } + + AsciiUtils.ToUpperInPlace(subCommand); + + count--; + + if (subCommand.SequenceEqual(CmdStrings.LOAD)) + { + return RespCommand.SCRIPT_LOAD; + } + + if (subCommand.SequenceEqual(CmdStrings.FLUSH)) + { + return RespCommand.SCRIPT_FLUSH; + } + + if (subCommand.SequenceEqual(CmdStrings.EXISTS)) + { + return RespCommand.SCRIPT_EXISTS; + } + + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommand, + Encoding.UTF8.GetString(subCommand), + nameof(RespCommand.SCRIPT)); + specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); + } else if (command.SequenceEqual(CmdStrings.ECHO)) { return RespCommand.ECHO; From f2e60de2e89c68eeb73801c7749116df25d9e744 Mon Sep 17 00:00:00 2001 From: prvyk Date: Fri, 31 Jan 2025 22:16:15 +0200 Subject: [PATCH 2/4] More standard behaviour for slow parse command with missing/wrong arguments. --- libs/server/Resp/CmdStrings.cs | 1 + libs/server/Resp/Parser/RespCommand.cs | 285 ++++++++++++++++--------- 2 files changed, 184 insertions(+), 102 deletions(-) diff --git a/libs/server/Resp/CmdStrings.cs b/libs/server/Resp/CmdStrings.cs index c389e80c5e..4fe471dd77 100644 --- a/libs/server/Resp/CmdStrings.cs +++ b/libs/server/Resp/CmdStrings.cs @@ -257,6 +257,7 @@ static partial class CmdStrings public const string GenericErrUnknownOptionConfigSet = "ERR Unknown option or number of arguments for CONFIG SET - '{0}'"; public const string GenericErrUnknownOption = "ERR Unknown option or number of arguments for '{0}' command"; public const string GenericErrUnknownSubCommand = "ERR unknown subcommand '{0}'. Try {1} HELP"; + public const string GenericErrUnknownSubCommandNoHelp = "ERR unknown subcommand '{0}'."; public const string GenericErrWrongNumArgsTxn = "ERR Invalid number of parameters to stored proc {0}, expected {1}, actual {2}"; public const string GenericSyntaxErrorOption = "ERR Syntax error in {0} option '{1}'"; diff --git a/libs/server/Resp/Parser/RespCommand.cs b/libs/server/Resp/Parser/RespCommand.cs index 1c422c166b..fa49f2ff52 100644 --- a/libs/server/Resp/Parser/RespCommand.cs +++ b/libs/server/Resp/Parser/RespCommand.cs @@ -1721,10 +1721,11 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci return RespCommand.SCRIPT_EXISTS; } - string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommand, + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommandNoHelp, Encoding.UTF8.GetString(subCommand), nameof(RespCommand.SCRIPT)); specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); + return RespCommand.INVALID; } else if (command.SequenceEqual(CmdStrings.ECHO)) { @@ -1744,33 +1745,38 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci { specificErrorMsg = Encoding.ASCII.GetBytes(string.Format(CmdStrings.GenericErrWrongNumArgs, nameof(RespCommand.CONFIG))); + return RespCommand.INVALID; } - else if (count >= 1) + + Span subCommand = GetCommand(out bool gotSubCommand); + if (!gotSubCommand) { - Span subCommand = GetCommand(out bool gotSubCommand); - if (!gotSubCommand) - { - success = false; - return RespCommand.NONE; - } + success = false; + return RespCommand.NONE; + } - AsciiUtils.ToUpperInPlace(subCommand); + AsciiUtils.ToUpperInPlace(subCommand); - count--; + count--; - if (subCommand.SequenceEqual(CmdStrings.GET)) - { - return RespCommand.CONFIG_GET; - } - else if (subCommand.SequenceEqual(CmdStrings.REWRITE)) - { - return RespCommand.CONFIG_REWRITE; - } - else if (subCommand.SequenceEqual(CmdStrings.SET)) - { - return RespCommand.CONFIG_SET; - } + if (subCommand.SequenceEqual(CmdStrings.GET)) + { + return RespCommand.CONFIG_GET; + } + else if (subCommand.SequenceEqual(CmdStrings.REWRITE)) + { + return RespCommand.CONFIG_REWRITE; } + else if (subCommand.SequenceEqual(CmdStrings.SET)) + { + return RespCommand.CONFIG_SET; + } + + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommandNoHelp, + Encoding.UTF8.GetString(subCommand), + nameof(RespCommand.CONFIG)); + specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); + return RespCommand.INVALID; } else if (command.SequenceEqual(CmdStrings.CLIENT)) { @@ -1778,53 +1784,58 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci { specificErrorMsg = Encoding.ASCII.GetBytes(string.Format(CmdStrings.GenericErrWrongNumArgs, nameof(RespCommand.CLIENT))); + return RespCommand.INVALID; } - else if (count >= 1) + + Span subCommand = GetCommand(out bool gotSubCommand); + if (!gotSubCommand) { - Span subCommand = GetCommand(out bool gotSubCommand); - if (!gotSubCommand) - { - success = false; - return RespCommand.NONE; - } + success = false; + return RespCommand.NONE; + } - AsciiUtils.ToUpperInPlace(subCommand); + AsciiUtils.ToUpperInPlace(subCommand); - count--; + count--; - if (subCommand.SequenceEqual(CmdStrings.ID)) - { - return RespCommand.CLIENT_ID; - } - else if (subCommand.SequenceEqual(CmdStrings.INFO)) - { - return RespCommand.CLIENT_INFO; - } - else if (subCommand.SequenceEqual(CmdStrings.LIST)) - { - return RespCommand.CLIENT_LIST; - } - else if (subCommand.SequenceEqual(CmdStrings.KILL)) - { - return RespCommand.CLIENT_KILL; - } - else if (subCommand.SequenceEqual(CmdStrings.GETNAME)) - { - return RespCommand.CLIENT_GETNAME; - } - else if (subCommand.SequenceEqual(CmdStrings.SETNAME)) - { - return RespCommand.CLIENT_SETNAME; - } - else if (subCommand.SequenceEqual(CmdStrings.SETINFO)) - { - return RespCommand.CLIENT_SETINFO; - } - else if (subCommand.SequenceEqual(CmdStrings.UNBLOCK)) - { - return RespCommand.CLIENT_UNBLOCK; - } + if (subCommand.SequenceEqual(CmdStrings.ID)) + { + return RespCommand.CLIENT_ID; + } + else if (subCommand.SequenceEqual(CmdStrings.INFO)) + { + return RespCommand.CLIENT_INFO; + } + else if (subCommand.SequenceEqual(CmdStrings.LIST)) + { + return RespCommand.CLIENT_LIST; + } + else if (subCommand.SequenceEqual(CmdStrings.KILL)) + { + return RespCommand.CLIENT_KILL; + } + else if (subCommand.SequenceEqual(CmdStrings.GETNAME)) + { + return RespCommand.CLIENT_GETNAME; } + else if (subCommand.SequenceEqual(CmdStrings.SETNAME)) + { + return RespCommand.CLIENT_SETNAME; + } + else if (subCommand.SequenceEqual(CmdStrings.SETINFO)) + { + return RespCommand.CLIENT_SETINFO; + } + else if (subCommand.SequenceEqual(CmdStrings.UNBLOCK)) + { + return RespCommand.CLIENT_UNBLOCK; + } + + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommandNoHelp, + Encoding.UTF8.GetString(subCommand), + nameof(RespCommand.CLIENT)); + specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); + return RespCommand.INVALID; } else if (command.SequenceEqual(CmdStrings.AUTH)) { @@ -1880,6 +1891,12 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci { return RespCommand.COMMAND_GETKEYSANDFLAGS; } + + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommand, + Encoding.UTF8.GetString(subCommand), + nameof(RespCommand.COMMAND)); + specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); + return RespCommand.INVALID; } else if (command.SequenceEqual(CmdStrings.PING)) { @@ -1891,6 +1908,13 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci } else if (command.SequenceEqual(CmdStrings.CLUSTER)) { + if (count == 0) + { + specificErrorMsg = Encoding.ASCII.GetBytes(string.Format(CmdStrings.GenericErrWrongNumArgs, + nameof(RespCommand.CLUSTER))); + return RespCommand.INVALID; + } + Span subCommand = GetCommand(out var gotSubCommand); if (!gotSubCommand) { @@ -2071,37 +2095,50 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci return RespCommand.CLUSTER_SEND_CKPT_METADATA; } - string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommand, Encoding.UTF8.GetString(subCommand), "CLUSTER"); + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommand, + Encoding.UTF8.GetString(subCommand), + nameof(RespCommand.CLUSTER)); specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); + return RespCommand.INVALID; } else if (command.SequenceEqual(CmdStrings.LATENCY)) { - if (count >= 1) + if (count == 0) { - Span subCommand = GetCommand(out bool gotSubCommand); - if (!gotSubCommand) - { - success = false; - return RespCommand.NONE; - } + specificErrorMsg = Encoding.ASCII.GetBytes(string.Format(CmdStrings.GenericErrWrongNumArgs, + nameof(RespCommand.LATENCY))); + return RespCommand.INVALID; + } - AsciiUtils.ToUpperInPlace(subCommand); + Span subCommand = GetCommand(out bool gotSubCommand); + if (!gotSubCommand) + { + success = false; + return RespCommand.NONE; + } - count--; + AsciiUtils.ToUpperInPlace(subCommand); - if (subCommand.SequenceEqual(CmdStrings.HELP)) - { - return RespCommand.LATENCY_HELP; - } - else if (subCommand.SequenceEqual(CmdStrings.HISTOGRAM)) - { - return RespCommand.LATENCY_HISTOGRAM; - } - else if (subCommand.SequenceEqual(CmdStrings.RESET)) - { - return RespCommand.LATENCY_RESET; - } + count--; + + if (subCommand.SequenceEqual(CmdStrings.HELP)) + { + return RespCommand.LATENCY_HELP; + } + else if (subCommand.SequenceEqual(CmdStrings.HISTOGRAM)) + { + return RespCommand.LATENCY_HISTOGRAM; + } + else if (subCommand.SequenceEqual(CmdStrings.RESET)) + { + return RespCommand.LATENCY_RESET; } + + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommand, + Encoding.UTF8.GetString(subCommand), + nameof(RespCommand.LATENCY)); + specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); + return RespCommand.INVALID; } else if (command.SequenceEqual(CmdStrings.TIME)) { @@ -2153,22 +2190,32 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci } else if (command.SequenceEqual(CmdStrings.MEMORY)) { - if (count > 0) + if (count == 0) { - ReadOnlySpan subCommand = GetCommand(out bool gotSubCommand); - if (!gotSubCommand) - { - success = false; - return RespCommand.NONE; - } + specificErrorMsg = Encoding.ASCII.GetBytes(string.Format(CmdStrings.GenericErrWrongNumArgs, + nameof(RespCommand.MEMORY))); + return RespCommand.INVALID; + } + + ReadOnlySpan subCommand = GetCommand(out bool gotSubCommand); + if (!gotSubCommand) + { + success = false; + return RespCommand.NONE; + } - count--; + count--; - if (subCommand.EqualsUpperCaseSpanIgnoringCase(CmdStrings.USAGE)) - { - return RespCommand.MEMORY_USAGE; - } + if (subCommand.EqualsUpperCaseSpanIgnoringCase(CmdStrings.USAGE)) + { + return RespCommand.MEMORY_USAGE; } + + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommandNoHelp, + Encoding.UTF8.GetString(subCommand), + nameof(RespCommand.MEMORY)); + specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); + return RespCommand.INVALID; } else if (command.SequenceEqual(CmdStrings.MONITOR)) { @@ -2176,6 +2223,13 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci } else if (command.SequenceEqual(CmdStrings.ACL)) { + if (count == 0) + { + specificErrorMsg = Encoding.ASCII.GetBytes(string.Format(CmdStrings.GenericErrWrongNumArgs, + nameof(RespCommand.ACL))); + return RespCommand.INVALID; + } + Span subCommand = GetCommand(out bool gotSubCommand); if (!gotSubCommand) { @@ -2219,6 +2273,12 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci { return RespCommand.ACL_WHOAMI; } + + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommandNoHelp, + Encoding.UTF8.GetString(subCommand), + nameof(RespCommand.ACL)); + specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); + return RespCommand.INVALID; } else if (command.SequenceEqual(CmdStrings.REGISTERCS)) { @@ -2230,6 +2290,13 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci } else if (command.SequenceEqual(CmdStrings.MODULE)) { + if (count == 0) + { + specificErrorMsg = Encoding.ASCII.GetBytes(string.Format(CmdStrings.GenericErrWrongNumArgs, + nameof(RespCommand.MODULE))); + return RespCommand.INVALID; + } + Span subCommand = GetCommand(out var gotSubCommand); if (!gotSubCommand) { @@ -2243,9 +2310,22 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci { return RespCommand.MODULE_LOADCS; } + + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommandNoHelp, + Encoding.UTF8.GetString(subCommand), + nameof(RespCommand.MODULE)); + specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); + return RespCommand.INVALID; } else if (command.SequenceEqual(CmdStrings.PUBSUB)) { + if (count == 0) + { + specificErrorMsg = Encoding.ASCII.GetBytes(string.Format(CmdStrings.GenericErrWrongNumArgs, + nameof(RespCommand.PUBSUB))); + return RespCommand.INVALID; + } + Span subCommand = GetCommand(out var gotSubCommand); if (!gotSubCommand) { @@ -2267,11 +2347,12 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci { return RespCommand.PUBSUB_NUMPAT; } - else - { - success = false; - return RespCommand.NONE; - } + + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommandNoHelp, + Encoding.UTF8.GetString(subCommand), + nameof(RespCommand.PUBSUB)); + specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); + return RespCommand.INVALID; } else if (command.SequenceEqual(CmdStrings.HCOLLECT)) { From 59beef04e5903c9dae80bfb8127604bd1c8fbcf3 Mon Sep 17 00:00:00 2001 From: prvyk Date: Sat, 1 Feb 2025 12:14:29 +0200 Subject: [PATCH 3/4] dotnet format --- libs/server/Resp/Parser/RespCommand.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/server/Resp/Parser/RespCommand.cs b/libs/server/Resp/Parser/RespCommand.cs index fa49f2ff52..1f2ac44468 100644 --- a/libs/server/Resp/Parser/RespCommand.cs +++ b/libs/server/Resp/Parser/RespCommand.cs @@ -2096,7 +2096,7 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci } string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommand, - Encoding.UTF8.GetString(subCommand), + Encoding.UTF8.GetString(subCommand), nameof(RespCommand.CLUSTER)); specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); return RespCommand.INVALID; @@ -2311,7 +2311,7 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci return RespCommand.MODULE_LOADCS; } - string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommandNoHelp, + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommandNoHelp, Encoding.UTF8.GetString(subCommand), nameof(RespCommand.MODULE)); specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); From e12ce740630cc91fd28cc039e065fc7f959945e6 Mon Sep 17 00:00:00 2001 From: prvyk Date: Sat, 1 Feb 2025 12:42:17 +0200 Subject: [PATCH 4/4] Match COMMAND UNKNOWN error message and expected test result. --- libs/server/Resp/Parser/RespCommand.cs | 2 +- test/Garnet.test/RespCommandTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/server/Resp/Parser/RespCommand.cs b/libs/server/Resp/Parser/RespCommand.cs index 1f2ac44468..109094f42d 100644 --- a/libs/server/Resp/Parser/RespCommand.cs +++ b/libs/server/Resp/Parser/RespCommand.cs @@ -1892,7 +1892,7 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci return RespCommand.COMMAND_GETKEYSANDFLAGS; } - string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommand, + string errMsg = string.Format(CmdStrings.GenericErrUnknownSubCommandNoHelp, Encoding.UTF8.GetString(subCommand), nameof(RespCommand.COMMAND)); specificErrorMsg = Encoding.UTF8.GetBytes(errMsg); diff --git a/test/Garnet.test/RespCommandTests.cs b/test/Garnet.test/RespCommandTests.cs index 76ba19d6c1..fdb95b03c0 100644 --- a/test/Garnet.test/RespCommandTests.cs +++ b/test/Garnet.test/RespCommandTests.cs @@ -297,7 +297,7 @@ public void CommandUnknownSubcommandTest() } catch (RedisServerException e) { - ClassicAssert.AreEqual("ERR unknown command", e.Message); + ClassicAssert.AreEqual("ERR unknown subcommand 'UNKNOWN'.", e.Message); } }