Fix several bugs.

Fix #17, also fix #14, fix #10 and fix #16.

src/facade/error.h

@@ -10,6 +10,7 @@
 namespace facade {
 
 std::string WrongNumArgsError(std::string_view cmd);
+std::string InvalidExpireTime(std::string_view cmd);
 
 extern const char kSyntaxErr[];
 extern const char kWrongTypeErr[];
@@ -23,8 +24,9 @@ extern const char kInvalidDbIndErr[];
 extern const char kScriptNotFound[];
 extern const char kAuthRejected[];
 extern const char kExpiryOutOfRange[];
-extern const char kInvalidExpireTime[];
 extern const char kSyntaxErrType[];
 extern const char kScriptErrType[];
+extern const char kIndexOutOfRange[];
+
 
 }  // namespace dfly

src/facade/facade.cc

@@ -49,10 +49,14 @@ ConnectionStats& ConnectionStats::operator+=(const ConnectionStats& o) {
 
 #undef ADD
 
-string WrongNumArgsError(std::string_view cmd) {
+string WrongNumArgsError(string_view cmd) {
   return absl::StrCat("wrong number of arguments for '", cmd, "' command");
 }
 
+string InvalidExpireTime(string_view cmd) {
+  return absl::StrCat("invalid expire time in '", cmd, "' command");
+}
+
 const char kSyntaxErr[] = "syntax error";
 const char kWrongTypeErr[] = "-WRONGTYPE Operation against a key holding the wrong kind of value";
 const char kKeyNotFoundErr[] = "no such key";
@@ -65,9 +69,10 @@ const char kInvalidDbIndErr[] = "invalid DB index";
 const char kScriptNotFound[] = "-NOSCRIPT No matching script. Please use EVAL.";
 const char kAuthRejected[] = "-WRONGPASS invalid username-password pair or user is disabled.";
 const char kExpiryOutOfRange[] = "expiry is out of range";
-const char kInvalidExpireTime[] = "invalid expire time";
 const char kSyntaxErrType[] = "syntax_error";
 const char kScriptErrType[] = "script_error";
+const char kIndexOutOfRange[] = "index out of range";
+
 
 const char* RespExpr::TypeName(Type t) {
   switch (t) {

src/facade/reply_builder.cc

@@ -211,6 +211,9 @@ void RedisReplyBuilder::SendError(OpStatus status) {
     case OpStatus::WRONG_TYPE:
       SendError(kWrongTypeErr);
       break;
+    case OpStatus::OUT_OF_RANGE:
+      SendError(kIndexOutOfRange);
+      break;
     default:
       LOG(ERROR) << "Unsupported status " << status;
       SendError("Internal error");

src/server/generic_family.cc

@@ -21,8 +21,7 @@ DEFINE_uint32(dbnum, 16, "Number of databases");
 
 namespace dfly {
 using namespace std;
-using facade::kExpiryOutOfRange;
-using facade::Protocol;
+using namespace facade;
 
 namespace {
 
@@ -271,18 +270,20 @@ void GenericFamily::Expire(CmdArgList args, ConnectionContext* cntx) {
     return (*cntx)->SendError(kInvalidIntErr);
   }
 
+  if (int_arg > kMaxExpireDeadlineSec || int_arg < -kMaxExpireDeadlineSec) {
+    ToLower(&args[0]);
+    return (*cntx)->SendError(InvalidExpireTime(ArgS(args, 0)));
+  }
+
   int_arg = std::max(int_arg, -1L);
   ExpireParams params{.ts = int_arg};
 
   auto cb = [&](Transaction* t, EngineShard* shard) {
     return OpExpire(OpArgs{shard, t->db_index()}, key, params);
   };
+
   OpStatus status = cntx->transaction->ScheduleSingleHop(move(cb));
-  if (status == OpStatus::OUT_OF_RANGE) {
-    return (*cntx)->SendError(kExpiryOutOfRange);
-  } else {
-    (*cntx)->SendLong(status == OpStatus::OK);
-  }
+  (*cntx)->SendLong(status == OpStatus::OK);
 }
 
 void GenericFamily::ExpireAt(CmdArgList args, ConnectionContext* cntx) {
@@ -527,7 +528,6 @@ OpStatus GenericFamily::OpExpire(const OpArgs& op_args, string_view key,
   int64_t msec = (params.unit == TimeUnit::SEC) ? params.ts * 1000 : params.ts;
   int64_t now_msec = db_slice.Now();
   int64_t rel_msec = params.absolute ? msec - now_msec : msec;
-
   if (rel_msec > kMaxExpireDeadlineSec * 1000) {
     return OpStatus::OUT_OF_RANGE;
   }

src/server/list_family_test.cc

@@ -184,6 +184,8 @@ TEST_F(ListFamilyTest, Lset) {
   ASSERT_THAT(Run({"lpop", kKey1}), RespEq("bar"));
   ASSERT_THAT(Run({"lset", kKey1, "-1", "foo"}), RespEq("OK"));
   ASSERT_THAT(Run({"rpop", kKey1}), RespEq("foo"));
+  Run({"rpush", kKey2, "a"});
+  ASSERT_THAT(Run({"lset", kKey2, "1", "foo"}), ElementsAre(ErrArg("index out of range")));
 }
 
 TEST_F(ListFamilyTest, BLPopSerialize) {

src/server/string_family.cc

@@ -139,14 +139,14 @@ void StringFamily::Set(CmdArgList args, ConnectionContext* cntx) {
       }
 
       if (int_arg <= 0 || (!is_ms && int_arg >= kMaxExpireDeadlineSec)) {
-        return builder->SendError(kExpiryOutOfRange);
+        return builder->SendError(InvalidExpireTime("set"));
       }
 
       if (!is_ms) {
         int_arg *= 1000;
       }
       if (int_arg >= kMaxExpireDeadlineSec * 1000) {
-        return builder->SendError(kExpiryOutOfRange);
+        return builder->SendError(InvalidExpireTime("set"));
       }
       sparams.expire_after_ms = int_arg;
     } else if (cur_arg == "NX") {
@@ -363,7 +363,7 @@ void StringFamily::SetExGeneric(bool seconds, CmdArgList args, ConnectionContext
 
   if (unit_vals < 1) {
     ToLower(&args[0]);
-    return (*cntx)->SendError(absl::StrCat(kInvalidExpireTime, " in ", ArgS(args, 0)));
+    return (*cntx)->SendError(InvalidExpireTime(ArgS(args, 0)));
   }
 
   SetCmd::SetParams sparams{cntx->db_index()};

src/server/string_family_test.cc

@@ -93,7 +93,7 @@ TEST_F(StringFamilyTest, Set) {
   ASSERT_THAT(resp, ElementsAre(ErrArg(kInvalidIntErr)));
 
   resp = Run({"set", "foo", "bar", "ex", "-1"});
-  ASSERT_THAT(resp, ElementsAre(ErrArg("out of range")));
+  ASSERT_THAT(resp, ElementsAre(ErrArg("invalid expire time")));
 
   resp = Run({"set", "foo", "bar", "ex", "1"});
   ASSERT_THAT(resp, RespEq("OK"));

src/server/zset_family_test.cc

@@ -98,6 +98,8 @@ TEST_F(ZSetFamilyTest, ZRemRangeScore) {
   EXPECT_THAT(Run({"zrange", "x", "0", "5"}), ElementsAre("b"));
   EXPECT_THAT(Run({"ZREMRANGEBYSCORE", "x", "(2.0", "+inf"}), ElementsAre(IntArg(1)));
   EXPECT_THAT(Run({"type", "x"}), ElementsAre("none"));
+  EXPECT_THAT(Run({"zremrangebyscore", "x", "1", "NaN"}),
+              ElementsAre(ErrArg("min or max is not a float")));
 }
 
 TEST_F(ZSetFamilyTest, IncrBy) {