fix: crash when passing empty arguments (#3627)

* fix: crash when passing empty arguments

Fix the case where we pass an empty argument, which then is parsed as an
empty string view with null pointer. The null pointer is not handled correctly
by our low level c code, hence switch to using ""sv for that.


* chore: add more list asserts + improve test_hypothesis
---------

Signed-off-by: Roman Gershman <roman@dragonflydb.io>

.github/workflows/ci.yml

@@ -106,7 +106,7 @@ jobs:
             -DCMAKE_C_COMPILER="${{matrix.compiler.c}}" \
             -DCMAKE_CXX_COMPILER="${{matrix.compiler.cxx}}" \
             -DCMAKE_CXX_COMPILER_LAUNCHER=sccache -DCMAKE_C_COMPILER_LAUNCHER=sccache \
-            -DCMAKE_CXX_FLAGS="${{matrix.cxx_flags}}" \
+            -DCMAKE_CXX_FLAGS="${{matrix.cxx_flags}}" -DWITH_AWS:BOOL=OFF \
             -L
           cd ${GITHUB_WORKSPACE}/build && pwd
           du -hcs _deps/

src/facade/cmd_arg_parser.h

@@ -165,9 +165,10 @@ struct CmdArgParser {
   }
 
   std::string_view SafeSV(size_t i) const {
+    using namespace std::literals::string_view_literals;
     if (i >= args_.size())
-      return "";
+      return ""sv;
-    return ToSV(args_[i]);
+    return args_[i].empty() ? ""sv : ToSV(args_[i]);
   }
 
   void Report(ErrorType type, size_t idx) {

src/server/list_family.cc

@@ -446,8 +446,10 @@ OpResult<string> OpIndex(const OpArgs& op_args, std::string_view key, long index
   return str;
 }
 
-OpResult<vector<uint32_t>> OpPos(const OpArgs& op_args, std::string_view key,
+OpResult<vector<uint32_t>> OpPos(const OpArgs& op_args, string_view key, string_view element,
-                                 std::string_view element, int rank, int count, int max_len) {
+                                 int rank, int count, int max_len) {
+  DCHECK(key.data() && element.data());
+
   auto it_res = op_args.GetDbSlice().FindReadOnly(op_args.db_cntx, key, OBJ_LIST);
   if (!it_res.ok())
     return it_res.status();
@@ -491,6 +493,8 @@ OpResult<vector<uint32_t>> OpPos(const OpArgs& op_args, std::string_view key,
 
 OpResult<int> OpInsert(const OpArgs& op_args, string_view key, string_view pivot, string_view elem,
                        InsertParam insert_param) {
+  DCHECK(key.data() && pivot.data() && elem.data());
+
   auto& db_slice = op_args.GetDbSlice();
   auto it_res = db_slice.FindMutable(op_args.db_cntx, key, OBJ_LIST);
   if (!it_res)
@@ -987,6 +991,7 @@ void ListFamily::LInsert(CmdArgList args, ConnectionContext* cntx) {
   string_view key = parser.Next();
   InsertParam where = parser.Switch("AFTER", INSERT_AFTER, "BEFORE", INSERT_BEFORE);
   auto [pivot, elem] = parser.Next<string_view, string_view>();
+  DCHECK(pivot.data() && elem.data());
 
   if (auto err = parser.Error(); err)
     return cntx->SendError(err->MakeReply());

src/server/list_family_test.cc

@@ -967,6 +967,14 @@ TEST_F(ListFamilyTest, LInsert) {
 
   // Insert after, pivot not found.
   EXPECT_THAT(Run({"linsert", "mylist", "after", "notfound", "x"}), IntArg(-1));
+
+  // insert empty
+  Run({"rpush", "k", "a"});
+  Run({"linsert", "k", "before", "a", ""});
+  resp = Run({"lpop", "k"});
+  EXPECT_EQ(resp, "");
+  resp = Run({"linsert", "k", "before", "", ""});
+  EXPECT_THAT(resp, IntArg(-1));
 }
 
 TEST_F(ListFamilyTest, BLPopUnwakesInScript) {

tests/fakeredis/test/test_hypothesis.py

@@ -49,7 +49,9 @@ int_as_bytes = st.builds(lambda x: str(default_normalize(x)).encode(), st.intege
 float_as_bytes = st.builds(
     lambda x: repr(default_normalize(x)).encode(), st.floats(width=32)
 )
-counts = st.integers(min_value=-3, max_value=3) | st.integers()
+counts = st.integers(min_value=-3, max_value=3) | st.integers(
+    min_value=-2147483648, max_value=2147483647
+)
 limits = st.just(()) | st.tuples(st.just("limit"), counts, counts)
 # Redis has an integer overflow bug in swapdb, so we confine the numbers to
 # a limited range (https://github.com/antirez/redis/issues/5737).
@@ -497,7 +499,7 @@ class TestList(BaseTest):
         | commands(
             st.sampled_from(["lpop", "rpop"]),
             keys,
-            st.just(None) | st.just([]) | st.integers(),
+            st.just(None) | st.just([]) | counts,
         )
         | commands(
             st.sampled_from(["lpush", "lpushx", "rpush", "rpushx"]),