From 04a364b4b006e498838c5c7631ff53a10288c96c Mon Sep 17 00:00:00 2001
From: Borys <borys@dragonflydb.io>
Date: Wed, 1 Jul 2026 20:37:53 +0300
Subject: [PATCH 1/2] feat: add more rules for parsing

---
 src/facade/cmd_arg_parser.h        |  95 ++++++++++++++++-----
 src/facade/cmd_arg_parser_test.cc  | 128 ++++++++++++++++++++++++++++-
 src/facade/error.h                 |   1 +
 src/server/bitops_family.cc        |  38 ++-------
 src/server/bloom_family.cc         |  13 +--
 src/server/cuckoo_filter_family.cc |  35 +++-----
 src/server/generic_family.cc       |   9 +-
 src/server/hset_family.cc          |   6 +-
 src/server/list_family.cc          |  23 ++----
 src/server/search/search_family.cc |  25 +++---
 src/server/set_family.cc           |   2 +-
 src/server/string_family.cc        |   6 +-
 src/server/topk_family.cc          |  15 ++--
 src/server/zset_family.cc          |  10 +--
 14 files changed, 253 insertions(+), 153 deletions(-)

diff --git a/src/facade/cmd_arg_parser.h b/src/facade/cmd_arg_parser.h
index 17fe1962f1e3..a23bccdbbf57 100644
--- a/src/facade/cmd_arg_parser.h
+++ b/src/facade/cmd_arg_parser.h
@@ -8,6 +8,7 @@
 #include <absl/strings/numbers.h>
 
 #include <cassert>
+#include <cmath>
 #include <concepts>
 #include <optional>
 #include <string_view>
@@ -21,6 +22,11 @@ namespace facade {
 
 // CmdArgParser — utility for parsing command option lists.
 //
+// Ideology: parse everything, then check the error ONCE at the end. The first error is latched and
+// every later read becomes a no-op, so intermediate `if (HasError())` guards are unnecessary. Fold
+// value checks into the read via Next<FInt<...>>() / Next<Validated<...>>() instead of post-parse
+// `if` blocks.
+//
 // Reading individual args:
 //   CmdArgParser parser(args);
 //   auto key = parser.Next<string_view>();                      // read one arg by type
@@ -29,6 +35,7 @@ namespace facade {
 //                                                               // (INVALID_INT if out of range)
 //   auto f  = parser.Next<FInt<1, 99>>("bad f");                // FInt with a custom out-of-range
 //                                                               // / non-integer error message
+//   auto s  = parser.Next<Validated<double, NotNan<kMsg>>>();   // parse + custom-error rule check
 //   auto count = parser.NextOrDefault<size_t>(10);              // read optional with default
 //   Range fields = parser.NextRange();                         // [N, e1..eN] counted list
 //   Range pairs  = parser.NextRange(2);                        // [N, f1,v1,..] N field/value pairs
@@ -77,34 +84,75 @@ namespace facade {
 //   parser.ReportCustom("bad option");                          // inject a custom error (no-op if
 //                                                               // one is already set)
 
-// A validated number for Next<T>(): a VNum-derived type that adds a static validate()
-// predicate. Convert<T>() parses the underlying value, runs validate(), and reports
-// INVALID_INT/INVALID_FLOAT on failure. FInt (below) is the built-in range-restricted integer;
-// domain-specific double validators are defined next to their callers.
+// Result of a validation rule: `failed` marks rejection. A non-empty `msg` is reported verbatim;
+// an empty one leaves the generic type error (INVALID_INT / INVALID_FLOAT), chosen by the caller.
+struct RuleError {
+  bool failed = false;
+  std::string_view msg = {};
+};
+
 template <class T>
-concept as_vnum = requires(T t, typename T::underlying_t v) {
-  static_cast<typename T::underlying_t>(t);
-  { T::validate(v) } -> std::same_as<bool>;
+concept as_vnum = requires(T t) {
+  static_cast<decltype(t.value)>(t);
+  { T::validate(t.value) } -> std::same_as<RuleError>;
 };
 
-// Base for as_vnum types: stores the parsed value and converts back to it. Derive and add a static
-// validate() predicate to define a new validated number.
+// Base for validated numbers: holds the parsed value and converts back to it.
 template <class T> struct VNum {
-  using underlying_t = T;
-  underlying_t value = {};
-  operator underlying_t() const {
+  T value = {};
+  operator T() const {
     return value;
   }
 };
 
-// Range-restricted integer used with Next<FInt<lo, hi>>() (INVALID_INT if out of range).
-template <auto min, auto max> struct FInt : VNum<decltype(min)> {
+// Validation rules for Next<Validated<T, Rules...>>(): free functions `RuleError rule(T)`. Reusable
+// ones take the message as a reference-to-constexpr NTTP. They also validate values from elsewhere:
+//   if (auto e = SomeRule<...>(v); e.failed) parser.ReportCustom(std::string{e.msg});
+
+// Out of [min, max] -> generic type error (FInt is the idiomatic spelling).
+template <auto min, auto max> RuleError InRange(decltype(min) v) {
   static_assert(std::is_same_v<decltype(min), decltype(max)>, "inconsistent types");
-  static constexpr bool validate(decltype(min) v) {
-    return v >= min && v <= max;
+  return {v < min || v > max, {}};
+}
+
+// Out of [min, max] -> custom Msg; also rejects NaN (safe for floating-point ranges).
+template <auto min, auto max, const auto& Msg> RuleError Bounded(decltype(min) v) {
+  static_assert(std::is_same_v<decltype(min), decltype(max)>, "inconsistent types");
+  return {!(v >= min && v <= max), Msg};
+}
+
+// v < min -> custom Msg; NaN is accepted (matches a plain `v < min` guard).
+template <auto min, const auto& Msg> RuleError AtLeast(decltype(min) v) {
+  return {v < min, Msg};
+}
+
+// Rejects NaN but accepts +/-inf.
+template <const auto& Msg> RuleError NotNan(double v) {
+  return {std::isnan(v), Msg};
+}
+
+template <const auto& Msg> RuleError Finite(double v) {
+  return {!std::isfinite(v), Msg};
+}
+
+template <auto Bad, const auto& Msg> RuleError NotEq(decltype(Bad) v) {
+  return {v == Bad, Msg};
+}
+
+// Accepts a value only if every rule accepts it; first rejection wins. Rules are function-pointer
+// NTTPs, e.g. Validated<int, Bounded<0, 9, kMsg>>.
+template <class T, auto... Rules> struct Validated : VNum<T> {
+  static_assert((std::same_as<decltype(Rules(std::declval<T>())), RuleError> && ...),
+                "a Validated rule must return RuleError");
+  static RuleError validate(T v) {
+    RuleError e;
+    (void)((e = Rules(v)).failed || ...);
+    return e;
   }
 };
 
+template <auto min, auto max> using FInt = Validated<decltype(min), InRange<min, max>>;
+
 template <class T> constexpr bool is_optional = false;
 
 template <class U> constexpr bool is_optional<std::optional<U>> = true;
@@ -119,7 +167,7 @@ struct CmdArgParser {
     INVALID_CASES,
     INVALID_NEXT,
     UNPROCESSED,
-    CUSTOM_ERROR  // should be the last one
+    CUSTOM_ERROR  // keep last
   };
 
   struct ErrorInfo {
@@ -128,7 +176,7 @@ struct CmdArgParser {
     std::string custom_msg;
 
     operator bool() const {
-      return type != ErrorType::NO_ERROR;
+      return type != NO_ERROR;
     }
     ErrorReply MakeReply() const;
   };
@@ -473,10 +521,15 @@ struct CmdArgParser {
     } else if constexpr (std::is_constructible_v<T, std::string_view>) {
       return static_cast<T>(SafeSV(idx));
     } else if constexpr (as_vnum<T>) {
-      using U = typename T::underlying_t;
+      using U = decltype(T::value);
       U val = Num<U>(idx);
-      if (!T::validate(val)) {
-        Report(std::is_floating_point_v<U> ? INVALID_FLOAT : INVALID_INT, idx);
+      if (error_)
+        return {};  // malformed number already reported
+      if (RuleError e = T::validate(val); e.failed) {
+        if (e.msg.empty())
+          Report(std::is_floating_point_v<U> ? INVALID_FLOAT : INVALID_INT, idx);
+        else
+          Report(CUSTOM_ERROR, idx, std::string{e.msg});
         return {};
       }
       return T{val};
diff --git a/src/facade/cmd_arg_parser_test.cc b/src/facade/cmd_arg_parser_test.cc
index ad70d0231288..987abae78e57 100644
--- a/src/facade/cmd_arg_parser_test.cc
+++ b/src/facade/cmd_arg_parser_test.cc
@@ -495,6 +495,127 @@ TEST_F(CmdArgParserTest, ApplyOptional) {
   }
 }
 
+TEST_F(CmdArgParserTest, ValidatedRules) {
+  static constexpr char kOverflow[] = "overflow";
+  static constexpr char kNaN[] = "not a number";
+  static constexpr char kNonFinite[] = "not finite";
+
+  // NotEq: rejects only the sentinel value, with a custom message.
+  using NoMin = Validated<int64_t, NotEq<INT64_MIN, kOverflow>>;
+  {
+    auto parser = Make({"5"});
+    EXPECT_EQ(static_cast<int64_t>(parser.Next<NoMin>()), 5);
+    EXPECT_FALSE(parser.HasError());
+  }
+  {
+    auto parser = Make({"-9223372036854775808"});  // INT64_MIN
+    parser.Next<NoMin>();
+    auto err = parser.TakeError();
+    EXPECT_EQ(err.type, CmdArgParser::CUSTOM_ERROR);
+    EXPECT_EQ(err.MakeReply().ToSv(), "overflow");
+  }
+  {  // A malformed number reports the generic INVALID_INT, not the rule message.
+    auto parser = Make({"abc"});
+    parser.Next<NoMin>();
+    EXPECT_EQ(parser.TakeError().type, CmdArgParser::INVALID_INT);
+  }
+
+  // NotNan accepts +/-inf but rejects NaN; Finite rejects both.
+  {
+    auto parser = Make({"inf", "nan"});
+    EXPECT_TRUE(std::isinf(static_cast<double>(parser.Next<Validated<double, NotNan<kNaN>>>())));
+    EXPECT_FALSE(parser.HasError());
+    parser.Next<Validated<double, NotNan<kNaN>>>();
+    EXPECT_EQ(parser.TakeError().MakeReply().ToSv(), "not a number");
+  }
+  {
+    auto parser = Make({"inf"});
+    parser.Next<Validated<double, Finite<kNonFinite>>>();
+    EXPECT_EQ(parser.TakeError().MakeReply().ToSv(), "not finite");
+  }
+
+  // Composition: rules run in order, first non-empty message wins (NotNan rejects NaN first).
+  {
+    auto parser = Make({"nan"});
+    parser.Next<Validated<double, NotNan<kNaN>, Finite<kNonFinite>>>();
+    EXPECT_EQ(parser.TakeError().MakeReply().ToSv(), "not a number");
+  }
+
+  // The rules are reusable to validate a value parsed by other means (e.g. NextWithPrefix).
+  {
+    auto parser = Make({"#-9223372036854775808"});
+    bool prefixed = false;
+    int64_t off = parser.NextWithPrefix<int64_t>("#", &prefixed);
+    EXPECT_TRUE(prefixed);
+    if (auto e = NotEq<INT64_MIN, kOverflow>(off); e.failed)
+      parser.ReportCustom(std::string{e.msg});
+    EXPECT_EQ(parser.TakeError().MakeReply().ToSv(), "overflow");
+  }
+}
+
+TEST_F(CmdArgParserTest, BoundedRule) {
+  static constexpr char kMsg[] = "out of range";
+
+  // Integer range: in-range passes; below/above report the custom message; a non-integer stays
+  // generic INVALID_INT.
+  using Pct = Validated<int, Bounded<0, 100, kMsg>>;
+  {
+    auto parser = Make({"0", "100", "50"});
+    EXPECT_EQ(static_cast<int>(parser.Next<Pct>()), 0);
+    EXPECT_EQ(static_cast<int>(parser.Next<Pct>()), 100);
+    EXPECT_EQ(static_cast<int>(parser.Next<Pct>()), 50);
+    EXPECT_FALSE(parser.HasError());
+  }
+  {
+    auto parser = Make({"101"});
+    parser.Next<Pct>();
+    EXPECT_EQ(parser.TakeError().MakeReply().ToSv(), "out of range");
+  }
+  {
+    auto parser = Make({"abc"});
+    parser.Next<Pct>();
+    EXPECT_EQ(parser.TakeError().type, CmdArgParser::INVALID_INT);
+  }
+
+  // Floating-point range rejects NaN and +/-inf via the custom message.
+  using UnitInterval = Validated<double, Bounded<0.0, 1.0, kMsg>>;
+  {
+    auto parser = Make({"0.5", "nan", "inf"});
+    EXPECT_DOUBLE_EQ(static_cast<double>(parser.Next<UnitInterval>()), 0.5);
+    EXPECT_FALSE(parser.HasError());
+    parser.Next<UnitInterval>();
+    EXPECT_EQ(parser.TakeError().MakeReply().ToSv(), "out of range");
+    auto p2 = Make({"inf"});
+    p2.Next<UnitInterval>();
+    EXPECT_EQ(p2.TakeError().MakeReply().ToSv(), "out of range");
+  }
+}
+
+TEST_F(CmdArgParserTest, AtLeastRule) {
+  static constexpr char kNeg[] = "must not be negative";
+  using NonNeg = Validated<float, AtLeast<0.0f, kNeg>>;
+
+  {
+    auto parser = Make({"0", "3.5"});
+    EXPECT_FLOAT_EQ(static_cast<float>(parser.Next<NonNeg>()), 0.0f);
+    EXPECT_FLOAT_EQ(static_cast<float>(parser.Next<NonNeg>()), 3.5f);
+    EXPECT_FALSE(parser.HasError());
+  }
+  {
+    auto parser = Make({"-0.1"});
+    parser.Next<NonNeg>();
+    EXPECT_EQ(parser.TakeError().MakeReply().ToSv(), "must not be negative");
+  }
+  {  // NaN is accepted (matches a plain `v < 0` guard); a non-float stays generic INVALID_FLOAT.
+    auto parser = Make({"nan"});
+    parser.Next<NonNeg>();
+    EXPECT_FALSE(parser.HasError());
+    auto p2 = Make({"abc"});
+    p2.Next<NonNeg>();
+    EXPECT_EQ(p2.TakeError().type, CmdArgParser::INVALID_FLOAT);
+  }
+}
+
 TEST_F(CmdArgParserTest, FixedRangeInt) {
   {
     auto parser = Make({"10", "-10", "12"});
@@ -520,11 +641,10 @@ TEST_F(CmdArgParserTest, FixedRangeInt) {
   }
 }
 
-// A user-defined as_vnum: VNum<double> + a validate() predicate. Exercises the generic
-// validated-number path of Convert<T>() for a floating-point underlying type.
+// A user-defined validated number: VNum<double> + validate() reporting the generic INVALID_FLOAT.
 struct PositiveFinite : VNum<double> {
-  static bool validate(double v) {
-    return v > 0 && std::isfinite(v);
+  static RuleError validate(double v) {
+    return {!(v > 0 && std::isfinite(v)), {}};
   }
 };
 
diff --git a/src/facade/error.h b/src/facade/error.h
index 865d7c812be8..0f80f2589277 100644
--- a/src/facade/error.h
+++ b/src/facade/error.h
@@ -22,6 +22,7 @@ inline constexpr char kKeyNotFoundErr[] = "no such key";
 inline constexpr char kInvalidIntErr[] = "value is not an integer or out of range";
 inline constexpr char kInvalidFloatErr[] = "value is not a valid float";
 inline constexpr char kUintErr[] = "value is out of range, must be positive";
+inline constexpr char kTimeoutNegativeErr[] = "timeout is negative";
 inline constexpr char kInvalidNumFields[] = "Number of fields must be a positive integer";
 inline constexpr char kIncrOverflow[] = "increment or decrement would overflow";
 inline constexpr char kDbIndOutOfRangeErr[] = "DB index is out of range";
diff --git a/src/server/bitops_family.cc b/src/server/bitops_family.cc
index 128007585364..14d74a10aa41 100644
--- a/src/server/bitops_family.cc
+++ b/src/server/bitops_family.cc
@@ -34,6 +34,7 @@ namespace {
 
 using ShardStringResults = vector<OpResult<string>>;
 const int32_t OFFSET_FACTOR = 8;  // number of bits in byte
+constexpr char kBitArgErr[] = "The bit argument must be 1 or 0";
 const char* OR_OP_NAME = "OR";
 const char* XOR_OP_NAME = "XOR";
 const char* AND_OP_NAME = "AND";
@@ -525,40 +526,11 @@ void BitPos(facade::CmdArgParser parser, CommandContext* cmd_cntx) {
   auto* builder = cmd_cntx->rb();
 
   auto key = parser.Next<string_view>();
-  auto value = parser.Next<int32_t>();
+  int32_t value = parser.Next<Validated<int32_t, Bounded<int32_t{0}, int32_t{1}, kBitArgErr>>>();
 
-  if (auto err = parser.TakeError(); err) {
-    return builder->SendError(err.MakeReply());
-  }
-
-  if (value != 0 && value != 1) {
-    return builder->SendError("The bit argument must be 1 or 0");
-  }
-
-  int64_t start = 0;
-  int64_t end = std::numeric_limits<int64_t>::max();
-  bool as_bit = false;
-
-  if (parser.HasNext()) {
-    start = parser.Next<int64_t>();
-    if (auto err = parser.TakeError(); err) {
-      return builder->SendError(err.MakeReply());
-    }
-
-    if (parser.HasNext()) {
-      end = parser.Next<int64_t>();
-      if (auto err = parser.TakeError(); err) {
-        return builder->SendError(err.MakeReply());
-      }
-
-      if (parser.HasNext()) {
-        as_bit = parser.MapNext("BIT", true, "BYTE", false);
-        if (auto err = parser.TakeError(); err) {
-          return builder->SendError(kSyntaxErr);
-        }
-      }
-    }
-  }
+  int64_t start = parser.NextOrDefault<int64_t>();
+  int64_t end = parser.NextOrDefault<int64_t>(std::numeric_limits<int64_t>::max());
+  bool as_bit = parser.HasNext() ? parser.MapNext("BIT", true, "BYTE", false) : false;
 
   if (!parser.Finalize()) {
     return builder->SendError(parser.TakeError().MakeReply());
diff --git a/src/server/bloom_family.cc b/src/server/bloom_family.cc
index ddfb1bb85bc1..de0e7650e5ee 100644
--- a/src/server/bloom_family.cc
+++ b/src/server/bloom_family.cc
@@ -232,10 +232,7 @@ void CmdMAdd(CmdArgParser parser, CommandContext* cmd_cntx) {
 void CmdScanDump(CmdArgParser parser, CommandContext* cmd_cntx) {
   auto* rb = static_cast<RedisReplyBuilder*>(cmd_cntx->rb());
   const string_view key = parser.Next();
-  const int64_t cursor = parser.Next<int64_t>();
-  if (cursor < 0)
-    return rb->SendError(kInvalidIntErr);
-
+  const int64_t cursor = parser.Next<FInt<int64_t{0}, std::numeric_limits<int64_t>::max()>>();
   if (const auto err = parser.TakeError(); err)
     return rb->SendError(err.MakeReply());
 
@@ -268,15 +265,11 @@ void CmdLoadChunk(CmdArgParser parser, CommandContext* cmd_cntx) {
   auto* rb = static_cast<RedisReplyBuilder*>(cmd_cntx->rb());
   const std::string_view key = parser.Next();
 
-  const int64_t cursor = parser.Next<int64_t>();
+  const int64_t cursor = parser.Next<FInt<int64_t{1}, std::numeric_limits<int64_t>::max()>>();
+  const std::string_view blob = parser.Next();
   if (const auto err = parser.TakeError(); err)
     return rb->SendError(err.MakeReply());
 
-  if (cursor <= 0)
-    return rb->SendError(kInvalidIntErr);
-
-  const std::string_view blob = parser.Next();
-
   const auto cb = [&](Transaction* t, EngineShard* shard) {
     return OpLoadChunk(t->GetOpArgs(shard), blob, key, cursor);
   };
diff --git a/src/server/cuckoo_filter_family.cc b/src/server/cuckoo_filter_family.cc
index 642adaac403d..080c9007fdb4 100644
--- a/src/server/cuckoo_filter_family.cc
+++ b/src/server/cuckoo_filter_family.cc
@@ -21,6 +21,11 @@ namespace {
 
 constexpr uint64_t kDefaultCapacity = 1024;
 
+constexpr char kCapacityErr[] = "CF: capacity must be greater than 0";
+constexpr char kBucketSizeErr[] = "CF: bucket size must be between 1 and 255";
+constexpr char kMaxIterationsErr[] = "CF: max iterations must be between 1 and 65535";
+constexpr char kExpansionErr[] = "CF: expansion must be between 0 and 32767";
+
 struct CuckooInfo {
   size_t size = 0;
   uint64_t num_buckets = 0;
@@ -185,17 +190,16 @@ OpStatus OpReserve(const OpArgs& op_args, string_view key, const CuckooFilterOpt
 
 void CmdReserve(CmdArgParser parser, CommandContext* cmd_cntx) {
   string_view key = parser.Next();
-  uint64_t capacity = parser.Next<uint64_t>();
+  uint64_t capacity = parser.Next<Validated<uint64_t, NotEq<uint64_t{0}, kCapacityErr>>>();
   auto* rb = static_cast<RedisReplyBuilder*>(cmd_cntx->rb());
-  RETURN_ON_PARSE_ERROR(parser, rb);
-
-  if (capacity == 0) {
-    return rb->SendError("CF: capacity must be greater than 0");
-  }
 
-  uint8_t bucket_size = CuckooFilterOptions::kDefaultSlotsPerBucket;
-  uint16_t max_iterations = CuckooFilterOptions::kDefaultMaxIterations;
-  uint16_t expansion = CuckooFilterOptions::kDefaultExpansion;
+  // The uint8_t/uint16_t widths reject over-max values already; the rules add CF's zero/cap bounds.
+  Validated<uint8_t, NotEq<uint8_t{0}, kBucketSizeErr>> bucket_size{
+      CuckooFilterOptions::kDefaultSlotsPerBucket};
+  Validated<uint16_t, NotEq<uint16_t{0}, kMaxIterationsErr>> max_iterations{
+      CuckooFilterOptions::kDefaultMaxIterations};
+  Validated<uint16_t, Bounded<uint16_t{0}, uint16_t{32767}, kExpansionErr>> expansion{
+      CuckooFilterOptions::kDefaultExpansion};
 
   parser.Apply(Tag("BUCKETSIZE", &bucket_size), Tag("MAXITERATIONS", &max_iterations),
                Tag("EXPANSION", &expansion));
@@ -204,19 +208,6 @@ void CmdReserve(CmdArgParser parser, CommandContext* cmd_cntx) {
     return rb->SendError(parser.TakeError().MakeReply());
   }
 
-  // The parser already rejects values that overflow the field width above (e.g. bucketsize
-  // 256) with the standard "value is not an integer or out of range" error. Only the
-  // business-rule bounds below (zero, and CF's tighter expansion cap) need manual checks.
-  if (bucket_size == 0) {
-    return rb->SendError("CF: bucket size must be between 1 and 255");
-  }
-  if (max_iterations == 0) {
-    return rb->SendError("CF: max iterations must be between 1 and 65535");
-  }
-  if (expansion > 32767) {
-    return rb->SendError("CF: expansion must be between 0 and 32767");
-  }
-
   CuckooFilterOptions options{capacity, bucket_size, max_iterations, expansion};
 
   const auto cb = [&](Transaction* t, EngineShard* shard) {
diff --git a/src/server/generic_family.cc b/src/server/generic_family.cc
index 76a64f87c441..914a448e2574 100644
--- a/src/server/generic_family.cc
+++ b/src/server/generic_family.cc
@@ -277,8 +277,8 @@ OpResult<RestoreArgs> RestoreArgs::TryFrom(facade::ParsedArgs args) {
 
   // args[0] = key (skip); args[1] = ttl; args[2] = serialized value (skip).
   parser.Skip(1);
-  out_args.expiration_ = parser.Next<int64_t>();
-  if (parser.TakeError() || out_args.expiration_ < 0)
+  out_args.expiration_ = parser.Next<FInt<int64_t{0}, std::numeric_limits<int64_t>::max()>>();
+  if (parser.TakeError())
     return OpStatus::INVALID_INT;
   parser.Skip(1);
 
@@ -1343,11 +1343,8 @@ void GenericFamily::Delex(facade::CmdArgParser parser, CommandContext* cmd_cntx)
 }
 
 void GenericFamily::Ping(facade::CmdArgParser parser, CommandContext* cmd_cntx) {
-  string_view msg;
   const bool has_msg = parser.HasNext();
-  if (has_msg) {
-    msg = parser.Next();
-  }
+  string_view msg = parser.NextOrDefault();
   if (parser.HasNext()) {
     return cmd_cntx->SendError(facade::WrongNumArgsError("ping"), kSyntaxErrType);
   }
diff --git a/src/server/hset_family.cc b/src/server/hset_family.cc
index 860ace93a4df..08731027a844 100644
--- a/src/server/hset_family.cc
+++ b/src/server/hset_family.cc
@@ -1173,13 +1173,9 @@ void CmdHIncrBy(CmdArgParser parser, CommandContext* cmd_cntx) {
 void CmdHIncrByFloat(CmdArgParser parser, CommandContext* cmd_cntx) {
   string_view key = parser.Next();
   string_view field = parser.Next();
-  double dval = parser.Next<double>();
+  double dval = parser.Next<Validated<double, Finite<kNanOrInfDuringIncr>>>();
   RETURN_ON_PARSE_ERROR(parser, cmd_cntx);
 
-  if (isnan(dval) || isinf(dval)) {
-    return cmd_cntx->SendError(kNanOrInfDuringIncr);
-  }
-
   IncrByParam param{dval};
 
   auto cb = [&](Transaction* t, EngineShard* shard) {
diff --git a/src/server/list_family.cc b/src/server/list_family.cc
index 92213be3f756..b688bda1bc21 100644
--- a/src/server/list_family.cc
+++ b/src/server/list_family.cc
@@ -882,14 +882,11 @@ void RPopLPush(CmdArgParser parser, CommandContext* cmd_cntx) {
 
 void BRPopLPush(CmdArgParser parser, CommandContext* cmd_cntx) {
   auto [src, dest] = parser.Next<string_view, string_view>();
-  float timeout = parser.Next<float>();
+  float timeout = parser.Next<Validated<float, AtLeast<0.0f, kTimeoutNegativeErr>>>();
   auto* builder = static_cast<RedisReplyBuilder*>(cmd_cntx->rb());
   if (auto err = parser.TakeError(); err)
     return cmd_cntx->SendError(err.MakeReply());
 
-  if (timeout < 0)
-    return cmd_cntx->SendError("timeout is negative");
-
   BPopPusher bpop_pusher(src, dest, ListDir::RIGHT, ListDir::LEFT);
   OpResult<string> op_res =
       bpop_pusher.Run(unsigned(timeout * 1000), cmd_cntx->tx(), cmd_cntx->server_conn_cntx());
@@ -914,14 +911,11 @@ void BLMove(CmdArgParser parser, CommandContext* cmd_cntx) {
   auto [src, dest] = parser.Next<string_view, string_view>();
   ListDir src_dir = ParseDir(&parser);
   ListDir dest_dir = ParseDir(&parser);
-  float timeout = parser.Next<float>();
+  float timeout = parser.Next<Validated<float, AtLeast<0.0f, kTimeoutNegativeErr>>>();
   auto* builder = static_cast<RedisReplyBuilder*>(cmd_cntx->rb());
   if (auto err = parser.TakeError(); err)
     return cmd_cntx->SendError(err.MakeReply());
 
-  if (timeout < 0)
-    return cmd_cntx->SendError("timeout is negative");
-
   BPopPusher bpop_pusher(src, dest, src_dir, dest_dir);
   OpResult<string> op_res =
       bpop_pusher.Run(unsigned(timeout * 1000), cmd_cntx->tx(), cmd_cntx->server_conn_cntx());
@@ -1042,12 +1036,8 @@ void PushGeneric(ListDir dir, bool skip_notexists, ParsedArgs args, CommandConte
 void PopGeneric(ListDir dir, CmdArgParser parser, CommandContext* cmd_cntx) {
   string_view key = parser.Next();
 
-  uint32_t count = 1;
-  bool return_arr = false;
-  if (parser.HasNext()) {
-    count = parser.Next<uint32_t>();
-    return_arr = true;
-  }
+  bool return_arr = parser.HasNext();
+  uint32_t count = parser.NextOrDefault<uint32_t>(1);
 
   RETURN_ON_PARSE_ERROR(parser, cmd_cntx);
 
@@ -1233,13 +1223,10 @@ void CmdLMPop(CmdArgParser parser, CommandContext* cmd_cntx) {
 void CmdBLMPop(CmdArgParser parser, CommandContext* cmd_cntx) {
   auto* response_builder = static_cast<RedisReplyBuilder*>(cmd_cntx->rb());
 
-  float timeout = parser.Next<float>();
+  float timeout = parser.Next<Validated<float, AtLeast<0.0f, kTimeoutNegativeErr>>>();
   if (auto err = parser.TakeError(); err)
     return cmd_cntx->SendError(err.MakeReply());
 
-  if (timeout < 0)
-    return cmd_cntx->SendError("timeout is negative");
-
   parser.Skip(parser.Next<size_t>());  // Skip numkeys and keys
   ListDir dir = parser.MapNext("LEFT", ListDir::LEFT, "RIGHT", ListDir::RIGHT);
 
diff --git a/src/server/search/search_family.cc b/src/server/search/search_family.cc
index a19a16c14c06..606f4df34fa9 100644
--- a/src/server/search/search_family.cc
+++ b/src/server/search/search_family.cc
@@ -180,13 +180,14 @@ ParseResult<std::string> ParseLanguageArg(CmdArgParser* parser) {
   return lang;
 }
 
-void ParseTextWeight(CmdArgParser* parser, search::SchemaField::TextParams* params) {
-  double weight = parser->Next<double>("Invalid WEIGHT value");
-  if (!search::SchemaField::TextParams::IsValidWeight(weight)) {
-    parser->ReportCustom("Invalid WEIGHT value");
-    return;
+struct ValidTextWeight : facade::VNum<double> {
+  static facade::RuleError validate(double v) {
+    return {!search::SchemaField::TextParams::IsValidWeight(v), {}};
   }
-  params->weight = weight;
+};
+
+void ParseTextWeight(CmdArgParser* parser, search::SchemaField::TextParams* params) {
+  params->weight = parser->Next<ValidTextWeight>("Invalid WEIGHT value");
 }
 
 ParseResult<search::SchemaField::TextParams> ParseTextParams(CmdArgParser* parser) {
@@ -1809,16 +1810,16 @@ struct HybridDocEntry {
 
 using HybridDocMap = absl::flat_hash_map<string, HybridDocEntry>;
 
-// Validated doubles for FT.HYBRID RANGE, used via CmdArgParser::Next<T>(). validate() runs inside
-// the parser so callers don't re-check the parsed value.
+// Validated doubles for FT.HYBRID RANGE; the empty message leaves INVALID_FLOAT for each call site
+// to replace via Next<T>("...").
 struct NonNegativeDouble : facade::VNum<double> {
-  static bool validate(double v) {
-    return v >= 0 && std::isfinite(v);
+  static facade::RuleError validate(double v) {
+    return {!(v >= 0 && std::isfinite(v)), {}};
   }
 };
 struct HnswRangeEpsilon : facade::VNum<double> {
-  static bool validate(double v) {
-    return search::SchemaField::VectorParams::IsValidRuntimeHnswEpsilon(v);
+  static facade::RuleError validate(double v) {
+    return {!search::SchemaField::VectorParams::IsValidRuntimeHnswEpsilon(v), {}};
   }
 };
 
diff --git a/src/server/set_family.cc b/src/server/set_family.cc
index a1bc23aeed17..c43615753ef6 100644
--- a/src/server/set_family.cc
+++ b/src/server/set_family.cc
@@ -1376,7 +1376,7 @@ void CmdSRandMember(CmdArgParser parser, CommandContext* cmd_cntx) {
   string_view key = parser.Next();
 
   bool is_count = parser.HasNext();
-  int count = is_count ? parser.Next<int>() : 1;
+  int count = parser.NextOrDefault<int>(1);
 
   if (parser.HasNext())
     return cmd_cntx->SendError(WrongNumArgsError("SRANDMEMBER"));
diff --git a/src/server/string_family.cc b/src/server/string_family.cc
index 10dfa52cdd5c..49de05cf56e6 100644
--- a/src/server/string_family.cc
+++ b/src/server/string_family.cc
@@ -1456,15 +1456,11 @@ cmd::CmdR CmdDecr(CmdArgParser parser, CommandContext* cmd_cntx) {
 }
 
 cmd::CmdR CmdDecrBy(CmdArgParser parser, CommandContext* cmd_cntx) {
-  auto [key, val] = parser.Next<string_view, int64_t>();
+  auto [key, val] = parser.Next<string_view, Validated<int64_t, NotEq<INT64_MIN, kIncrOverflow>>>();
   if (auto err = parser.TakeError(); err) {
     cmd_cntx->SendError(err.MakeReply());
     return cmd::kAborted;
   }
-  if (val == INT64_MIN) {
-    cmd_cntx->SendError(kIncrOverflow);
-    return cmd::kAborted;
-  }
 
   return IncrByGeneric(cmd_cntx, key, -val);
 }
diff --git a/src/server/topk_family.cc b/src/server/topk_family.cc
index b05daf871443..24effa683706 100644
--- a/src/server/topk_family.cc
+++ b/src/server/topk_family.cc
@@ -33,6 +33,9 @@ struct TopkFamily {
 
 namespace {
 
+constexpr char kDecayRangeErr[] = "decay must be between 0 and 1";
+constexpr char kIncrRangeErr[] = "increment must be between 1 and 100000";
+
 OpStatus OpReserve(const OpArgs& op_args, string_view key, uint32_t k, uint32_t width,
                    uint32_t depth, double decay) {
   auto& db_slice = op_args.GetDbSlice();
@@ -193,7 +196,7 @@ void TopkFamily::Reserve(facade::CmdArgParser parser, CommandContext* cmd_cntx)
   if (parser.HasNext()) {
     width = parser.Next<uint32_t>();
     depth = parser.Next<uint32_t>();
-    decay = parser.Next<double>();
+    decay = parser.Next<Validated<double, Bounded<0.0, 1.0, kDecayRangeErr>>>();
     RETURN_ON_PARSE_ERROR(parser, rb);
 
     if ((width == 0) || (depth == 0)) {
@@ -208,10 +211,6 @@ void TopkFamily::Reserve(facade::CmdArgParser parser, CommandContext* cmd_cntx)
       return rb->SendError(absl::StrCat("width must not exceed ", kMaxWidth,
                                         " and depth must not exceed ", kMaxDepth));
     }
-
-    if (!std::isfinite(decay) || (decay < 0.0) || (decay > 1.0)) {
-      return rb->SendError("decay must be between 0 and 1");
-    }
   }
 
   if (parser.HasNext()) {
@@ -271,11 +270,9 @@ void TopkFamily::IncrBy(facade::CmdArgParser parser, CommandContext* cmd_cntx) {
     if (!parser.HasNext()) {
       return cmd_cntx->SendError(kSyntaxErr);
     }
-    uint32_t incr = parser.Next<uint32_t>();
+    uint32_t incr =
+        parser.Next<Validated<uint32_t, Bounded<uint32_t{1}, uint32_t{100000}, kIncrRangeErr>>>();
     RETURN_ON_PARSE_ERROR(parser, rb);
-    if (incr < 1 || incr > 100000) {  // Redis limits increment to [1, 100000]
-      return cmd_cntx->SendError("increment must be between 1 and 100000");
-    }
     items.emplace_back(item, incr);
   }
 
diff --git a/src/server/zset_family.cc b/src/server/zset_family.cc
index 5187e043eae0..9d7a38f0de3c 100644
--- a/src/server/zset_family.cc
+++ b/src/server/zset_family.cc
@@ -47,7 +47,7 @@ using CI = CommandId;
 const char kNxXxErr[] = "XX and NX options at the same time are not compatible";
 const char kLexRangeErr[] = "min or max not valid string range item";
 const char kFloatRangeErr[] = "min or max is not a float";
-const char kScoreNaN[] = "resulting score is not a number (NaN)";
+constexpr char kScoreNaN[] = "resulting score is not a number (NaN)";
 
 using MScoreResponse = std::vector<std::optional<double>>;
 using ScoredMember = ZSetFamily::ScoredMember;
@@ -2441,16 +2441,12 @@ void CmdZIncrBy(CmdArgParser parser, CommandContext* cmd_cntx) {
 
   string_view key = parser.Next();
   ScoredMemberView scored_member;
-  scored_member.first = parser.Next<double>();
+  scored_member.first = parser.Next<Validated<double, NotNan<kScoreNaN>>>();
   scored_member.second = parser.Next();
 
   if (auto err = parser.TakeError(); err)
     return rb->SendError(err.MakeReply());
 
-  if (isnan(scored_member.first)) {
-    return rb->SendError(kScoreNaN);
-  }
-
   ZSetFamily::ZParams zparams;
   zparams.flags = ZADD_IN_INCR;
 
@@ -2786,7 +2782,7 @@ void CmdZRandMember(CmdArgParser parser, CommandContext* cmd_cntx) {
   string_view key = parser.Next();
 
   bool is_count = parser.HasNext();
-  int count = is_count ? parser.Next<int>() : 1;
+  int count = parser.NextOrDefault<int>(1);
 
   ZSetFamily::RangeParams params;
   params.with_scores = static_cast<bool>(parser.Check("WITHSCORES"));

From 4fd3974666cb8c0a9a05a63547655a963e154c0d Mon Sep 17 00:00:00 2001
From: Borys <borys@dragonflydb.io>
Date: Thu, 2 Jul 2026 12:48:09 +0300
Subject: [PATCH 2/2] refactor: address comments

---
 src/facade/cmd_arg_parser.h       | 30 +++++++++----------
 src/facade/cmd_arg_parser_test.cc | 44 ++++++++++++++++++----------
 src/facade/error.h                |  2 ++
 src/server/list_family.cc         | 47 +++++++++++++++++-------------
 src/server/list_family_test.cc    | 48 +++++++++++++++++++++++++++++--
 src/server/topk_family.cc         |  6 +++-
 6 files changed, 124 insertions(+), 53 deletions(-)

diff --git a/src/facade/cmd_arg_parser.h b/src/facade/cmd_arg_parser.h
index a23bccdbbf57..fa001183a5fa 100644
--- a/src/facade/cmd_arg_parser.h
+++ b/src/facade/cmd_arg_parser.h
@@ -115,23 +115,24 @@ template <auto min, auto max> RuleError InRange(decltype(min) v) {
   return {v < min || v > max, {}};
 }
 
-// Out of [min, max] -> custom Msg; also rejects NaN (safe for floating-point ranges).
+// Out of [min, max] -> custom Msg. Integer bounds only (floating-point NTTPs need clang 18+; use a
+// dedicated rule for float ranges).
 template <auto min, auto max, const auto& Msg> RuleError Bounded(decltype(min) v) {
   static_assert(std::is_same_v<decltype(min), decltype(max)>, "inconsistent types");
   return {!(v >= min && v <= max), Msg};
 }
 
-// v < min -> custom Msg; NaN is accepted (matches a plain `v < min` guard).
-template <auto min, const auto& Msg> RuleError AtLeast(decltype(min) v) {
-  return {v < min, Msg};
+// v < 0 -> custom Msg; NaN is accepted (matches a plain `v < 0` guard).
+template <const auto& Msg, class V> RuleError NonNegative(V v) {
+  return {v < 0, Msg};
 }
 
 // Rejects NaN but accepts +/-inf.
-template <const auto& Msg> RuleError NotNan(double v) {
+template <const auto& Msg, std::floating_point V> RuleError NotNan(V v) {
   return {std::isnan(v), Msg};
 }
 
-template <const auto& Msg> RuleError Finite(double v) {
+template <const auto& Msg, std::floating_point V> RuleError Finite(V v) {
   return {!std::isfinite(v), Msg};
 }
 
@@ -139,11 +140,9 @@ template <auto Bad, const auto& Msg> RuleError NotEq(decltype(Bad) v) {
   return {v == Bad, Msg};
 }
 
-// Accepts a value only if every rule accepts it; first rejection wins. Rules are function-pointer
-// NTTPs, e.g. Validated<int, Bounded<0, 9, kMsg>>.
-template <class T, auto... Rules> struct Validated : VNum<T> {
-  static_assert((std::same_as<decltype(Rules(std::declval<T>())), RuleError> && ...),
-                "a Validated rule must return RuleError");
+// Accepts a value only if every rule accepts it; first rejection wins. Each rule is a function
+// T -> RuleError, e.g. Validated<int, Bounded<0, 9, kMsg>>.
+template <class T, RuleError (*... Rules)(T)> struct Validated : VNum<T> {
   static RuleError validate(T v) {
     RuleError e;
     (void)((e = Rules(v)).failed || ...);
@@ -290,13 +289,14 @@ struct CmdArgParser {
     }
   }
 
-  // Like Next<T>(), but on a failed read (non-numeric, out-of-range FInt, or missing arg) replaces
-  // the generic error with a caller-supplied CUSTOM_ERROR message. Pair with FInt<lo,hi> to attach
-  // a custom out-of-range / non-integer message: parser.Next<FInt<1u, 99u>>("bad f").
+  // Like Next<T>(), but replaces any read failure (bad value, missing arg, ...) with a caller-
+  // supplied CUSTOM_ERROR message. A rule's own message from a Validated<T, Rules...> passes
+  // through unchanged, so parser.Next<Timeout>(kNotAFloat) reports kNotAFloat for a non-float but
+  // keeps each rule's out-of-range / negative message.
   template <class T = std::string_view> auto Next(std::string_view err_msg) {
     bool prior = bool(error_);
     auto val = Next<T>();
-    if (!prior && error_ && !err_msg.empty()) {
+    if (!prior && !err_msg.empty() && error_ && error_.type != CUSTOM_ERROR) {
       error_.type = CUSTOM_ERROR;
       error_.custom_msg = std::string{err_msg};
     }
diff --git a/src/facade/cmd_arg_parser_test.cc b/src/facade/cmd_arg_parser_test.cc
index 987abae78e57..5d6167fe3b74 100644
--- a/src/facade/cmd_arg_parser_test.cc
+++ b/src/facade/cmd_arg_parser_test.cc
@@ -576,24 +576,11 @@ TEST_F(CmdArgParserTest, BoundedRule) {
     parser.Next<Pct>();
     EXPECT_EQ(parser.TakeError().type, CmdArgParser::INVALID_INT);
   }
-
-  // Floating-point range rejects NaN and +/-inf via the custom message.
-  using UnitInterval = Validated<double, Bounded<0.0, 1.0, kMsg>>;
-  {
-    auto parser = Make({"0.5", "nan", "inf"});
-    EXPECT_DOUBLE_EQ(static_cast<double>(parser.Next<UnitInterval>()), 0.5);
-    EXPECT_FALSE(parser.HasError());
-    parser.Next<UnitInterval>();
-    EXPECT_EQ(parser.TakeError().MakeReply().ToSv(), "out of range");
-    auto p2 = Make({"inf"});
-    p2.Next<UnitInterval>();
-    EXPECT_EQ(p2.TakeError().MakeReply().ToSv(), "out of range");
-  }
 }
 
-TEST_F(CmdArgParserTest, AtLeastRule) {
+TEST_F(CmdArgParserTest, NonNegativeRule) {
   static constexpr char kNeg[] = "must not be negative";
-  using NonNeg = Validated<float, AtLeast<0.0f, kNeg>>;
+  using NonNeg = Validated<float, NonNegative<kNeg>>;
 
   {
     auto parser = Make({"0", "3.5"});
@@ -616,6 +603,33 @@ TEST_F(CmdArgParserTest, AtLeastRule) {
   }
 }
 
+TEST_F(CmdArgParserTest, NextWithMessageOverride) {
+  static constexpr char kParseMsg[] = "custom parse error";
+  static constexpr char kRuleMsg[] = "must not be negative";
+  using NonNeg = Validated<float, NonNegative<kRuleMsg>>;
+
+  {  // A non-numeric value reports the caller message instead of the generic type error.
+    auto parser = Make({"abc"});
+    parser.Next<NonNeg>(kParseMsg);
+    EXPECT_EQ(parser.TakeError().MakeReply().ToSv(), "custom parse error");
+  }
+  {  // A rule violation keeps its own message; the caller message does not override it.
+    auto parser = Make({"-1"});
+    parser.Next<NonNeg>(kParseMsg);
+    EXPECT_EQ(parser.TakeError().MakeReply().ToSv(), "must not be negative");
+  }
+  {  // FInt out-of-range is a generic error, so the caller message applies to it too.
+    auto parser = Make({"5"});
+    parser.Next<FInt<0, 3>>(kParseMsg);
+    EXPECT_EQ(parser.TakeError().MakeReply().ToSv(), "custom parse error");
+  }
+  {  // A valid value produces no error.
+    auto parser = Make({"2.5"});
+    EXPECT_FLOAT_EQ(static_cast<float>(parser.Next<NonNeg>(kParseMsg)), 2.5f);
+    EXPECT_FALSE(parser.HasError());
+  }
+}
+
 TEST_F(CmdArgParserTest, FixedRangeInt) {
   {
     auto parser = Make({"10", "-10", "12"});
diff --git a/src/facade/error.h b/src/facade/error.h
index 0f80f2589277..73f62ba8f362 100644
--- a/src/facade/error.h
+++ b/src/facade/error.h
@@ -23,6 +23,8 @@ inline constexpr char kInvalidIntErr[] = "value is not an integer or out of rang
 inline constexpr char kInvalidFloatErr[] = "value is not a valid float";
 inline constexpr char kUintErr[] = "value is out of range, must be positive";
 inline constexpr char kTimeoutNegativeErr[] = "timeout is negative";
+inline constexpr char kTimeoutNotFloatErr[] = "timeout is not a float or out of range";
+inline constexpr char kTimeoutOutOfRangeErr[] = "timeout is out of range";
 inline constexpr char kInvalidNumFields[] = "Number of fields must be a positive integer";
 inline constexpr char kIncrOverflow[] = "increment or decrement would overflow";
 inline constexpr char kDbIndOutOfRangeErr[] = "DB index is out of range";
diff --git a/src/server/list_family.cc b/src/server/list_family.cc
index b688bda1bc21..5b971615cbb3 100644
--- a/src/server/list_family.cc
+++ b/src/server/list_family.cc
@@ -9,6 +9,8 @@ extern "C" {
 #include <absl/functional/overload.h>
 #include <absl/strings/numbers.h>
 
+#include <limits>
+
 #include "base/flags.h"
 #include "base/logging.h"
 #include "core/detail/listpack.h"
@@ -84,6 +86,17 @@ using time_point = Transaction::time_point;
 
 namespace {
 
+// unsigned(timeout * 1000) feeds the blocking layer's millisecond counter, so cap the seconds to
+// keep the float->unsigned conversion in range. Redis reports oversized timeouts as out of range.
+constexpr float kMaxBlockingTimeoutSec = std::numeric_limits<unsigned>::max() / 1000;
+
+RuleError WithinTimeoutLimit(float v) {
+  return {v > kMaxBlockingTimeoutSec, kTimeoutOutOfRangeErr};  // also rejects +inf
+}
+
+using Timeout = Validated<float, NotNan<kTimeoutNotFloatErr>, NonNegative<kTimeoutNegativeErr>,
+                          WithinTimeoutLimit>;
+
 void OffloadListNode(QList* ql, QList::Node* node) {
   TieredStorage* ts = EngineShard::tlocal()->tiered_storage();
   DCHECK(ts);
@@ -882,10 +895,10 @@ void RPopLPush(CmdArgParser parser, CommandContext* cmd_cntx) {
 
 void BRPopLPush(CmdArgParser parser, CommandContext* cmd_cntx) {
   auto [src, dest] = parser.Next<string_view, string_view>();
-  float timeout = parser.Next<Validated<float, AtLeast<0.0f, kTimeoutNegativeErr>>>();
-  auto* builder = static_cast<RedisReplyBuilder*>(cmd_cntx->rb());
+  float timeout = parser.Next<Timeout>(kTimeoutNotFloatErr);
   if (auto err = parser.TakeError(); err)
     return cmd_cntx->SendError(err.MakeReply());
+  auto* builder = static_cast<RedisReplyBuilder*>(cmd_cntx->rb());
 
   BPopPusher bpop_pusher(src, dest, ListDir::RIGHT, ListDir::LEFT);
   OpResult<string> op_res =
@@ -911,10 +924,10 @@ void BLMove(CmdArgParser parser, CommandContext* cmd_cntx) {
   auto [src, dest] = parser.Next<string_view, string_view>();
   ListDir src_dir = ParseDir(&parser);
   ListDir dest_dir = ParseDir(&parser);
-  float timeout = parser.Next<Validated<float, AtLeast<0.0f, kTimeoutNegativeErr>>>();
-  auto* builder = static_cast<RedisReplyBuilder*>(cmd_cntx->rb());
+  float timeout = parser.Next<Timeout>(kTimeoutNotFloatErr);
   if (auto err = parser.TakeError(); err)
     return cmd_cntx->SendError(err.MakeReply());
+  auto* builder = static_cast<RedisReplyBuilder*>(cmd_cntx->rb());
 
   BPopPusher bpop_pusher(src, dest, src_dir, dest_dir);
   OpResult<string> op_res =
@@ -1063,17 +1076,11 @@ void PopGeneric(ListDir dir, CmdArgParser parser, CommandContext* cmd_cntx) {
   }
 }
 
-void BPopGeneric(ListDir dir, ParsedArgs args, CommandContext* cmd_cntx) {
-  DCHECK_GE(args.size(), 2u);
-
-  float timeout;
-  auto timeout_str = args[args.size() - 1];
-  if (!absl::SimpleAtof(timeout_str, &timeout)) {
-    return cmd_cntx->SendError("timeout is not a float or out of range");
-  }
-  if (timeout < 0) {
-    return cmd_cntx->SendError("timeout is negative");
-  }
+void BPopGeneric(ListDir dir, CmdArgParser parser, CommandContext* cmd_cntx) {
+  parser.Skip(parser.UnparsedArgs().size() - 1);  // keys are handled by the command key spec
+  float timeout = parser.Next<Timeout>(kTimeoutNotFloatErr);
+  if (auto err = parser.TakeError(); err)
+    return cmd_cntx->SendError(err.MakeReply());
   VLOG(1) << "BPop timeout(" << timeout << ")";
 
   std::string popped_value;
@@ -1143,7 +1150,7 @@ optional<pair<string_view, bool>> GetFirstNonEmptyKeyFound(EngineShard* shard, T
 void CmdLMPop(CmdArgParser parser, CommandContext* cmd_cntx) {
   auto* response_builder = static_cast<RedisReplyBuilder*>(cmd_cntx->rb());
 
-  parser.Skip(parser.Next<size_t>());  // skip numkeys and keys
+  parser.NextRange();  // numkeys + keys, handled by the command key spec
 
   ListDir dir = parser.MapNext("LEFT", ListDir::LEFT, "RIGHT", ListDir::RIGHT);
   size_t pop_count = 1;
@@ -1223,11 +1230,11 @@ void CmdLMPop(CmdArgParser parser, CommandContext* cmd_cntx) {
 void CmdBLMPop(CmdArgParser parser, CommandContext* cmd_cntx) {
   auto* response_builder = static_cast<RedisReplyBuilder*>(cmd_cntx->rb());
 
-  float timeout = parser.Next<Validated<float, AtLeast<0.0f, kTimeoutNegativeErr>>>();
+  float timeout = parser.Next<Timeout>(kTimeoutNotFloatErr);
   if (auto err = parser.TakeError(); err)
     return cmd_cntx->SendError(err.MakeReply());
 
-  parser.Skip(parser.Next<size_t>());  // Skip numkeys and keys
+  parser.NextRange();  // numkeys + keys, handled by the command key spec
   ListDir dir = parser.MapNext("LEFT", ListDir::LEFT, "RIGHT", ListDir::RIGHT);
 
   size_t pop_count = 1;
@@ -1469,11 +1476,11 @@ void CmdLSet(CmdArgParser parser, CommandContext* cmd_cntx) {
 }
 
 void CmdBLPop(CmdArgParser parser, CommandContext* cmd_cntx) {
-  BPopGeneric(ListDir::LEFT, parser.UnparsedArgs(), cmd_cntx);
+  BPopGeneric(ListDir::LEFT, std::move(parser), cmd_cntx);
 }
 
 void CmdBRPop(CmdArgParser parser, CommandContext* cmd_cntx) {
-  BPopGeneric(ListDir::RIGHT, parser.UnparsedArgs(), cmd_cntx);
+  BPopGeneric(ListDir::RIGHT, std::move(parser), cmd_cntx);
 }
 
 void CmdLMove(CmdArgParser parser, CommandContext* cmd_cntx) {
diff --git a/src/server/list_family_test.cc b/src/server/list_family_test.cc
index 7c25c7bbfb05..9d11a0619255 100644
--- a/src/server/list_family_test.cc
+++ b/src/server/list_family_test.cc
@@ -8,6 +8,7 @@
 
 #include "base/gtest.h"
 #include "base/logging.h"
+#include "facade/error.h"
 #include "facade/facade_test.h"
 #include "server/blocking_controller.h"
 #include "server/conn_context.h"
@@ -104,11 +105,11 @@ TEST_F(ListFamilyTest, BLMPopInvalidSyntax) {
 
   // Timeout is not a float
   resp = Run({"blmpop", "foo", "1", kKey1, "LEFT", "COUNT", "1"});
-  EXPECT_THAT(resp, ErrArg("value is not a valid float"));
+  EXPECT_THAT(resp, ErrArg(facade::kTimeoutNotFloatErr));
 
   // Negative timeout
   resp = Run({"blmpop", "-0.01", "1", kKey1, "LEFT", "COUNT", "1"});
-  EXPECT_THAT(resp, ErrArg("timeout is negative"));
+  EXPECT_THAT(resp, ErrArg(facade::kTimeoutNegativeErr));
 
   // Zero keys
   resp = Run({"blmpop", "0.01", "0", "LEFT", "COUNT", "1"});
@@ -919,6 +920,49 @@ TEST_F(ListFamilyTest, BLMove) {
   ASSERT_THAT(resp.GetVec(), ElementsAre("val1", "val2"));
 }
 
+// NaN / +-inf / negative timeouts are rejected (Redis-compatible) instead of being cast to unsigned
+// milliseconds, which is undefined behavior.
+TEST_F(ListFamilyTest, BlockingTimeoutValidation) {
+  EXPECT_THAT(Run({"brpoplpush", "x", "y", "abc"}), ErrArg(facade::kTimeoutNotFloatErr));
+  EXPECT_THAT(Run({"brpoplpush", "x", "y", "nan"}), ErrArg(facade::kTimeoutNotFloatErr));
+  EXPECT_THAT(Run({"brpoplpush", "x", "y", "inf"}), ErrArg(facade::kTimeoutOutOfRangeErr));
+  EXPECT_THAT(Run({"brpoplpush", "x", "y", "-inf"}), ErrArg(facade::kTimeoutNegativeErr));
+  EXPECT_THAT(Run({"brpoplpush", "x", "y", "-1"}), ErrArg(facade::kTimeoutNegativeErr));
+
+  EXPECT_THAT(Run({"blmove", "x", "y", "LEFT", "RIGHT", "abc"}),
+              ErrArg(facade::kTimeoutNotFloatErr));
+  EXPECT_THAT(Run({"blmove", "x", "y", "LEFT", "RIGHT", "nan"}),
+              ErrArg(facade::kTimeoutNotFloatErr));
+  EXPECT_THAT(Run({"blmove", "x", "y", "LEFT", "RIGHT", "inf"}),
+              ErrArg(facade::kTimeoutOutOfRangeErr));
+  EXPECT_THAT(Run({"blmove", "x", "y", "LEFT", "RIGHT", "-inf"}),
+              ErrArg(facade::kTimeoutNegativeErr));
+  EXPECT_THAT(Run({"blmove", "x", "y", "LEFT", "RIGHT", "-1"}),
+              ErrArg(facade::kTimeoutNegativeErr));
+
+  EXPECT_THAT(Run({"blmpop", "abc", "1", "k", "LEFT"}), ErrArg(facade::kTimeoutNotFloatErr));
+  EXPECT_THAT(Run({"blmpop", "nan", "1", "k", "LEFT"}), ErrArg(facade::kTimeoutNotFloatErr));
+  EXPECT_THAT(Run({"blmpop", "inf", "1", "k", "LEFT"}), ErrArg(facade::kTimeoutOutOfRangeErr));
+  EXPECT_THAT(Run({"blmpop", "-inf", "1", "k", "LEFT"}), ErrArg(facade::kTimeoutNegativeErr));
+  EXPECT_THAT(Run({"blmpop", "-1", "1", "k", "LEFT"}), ErrArg(facade::kTimeoutNegativeErr));
+
+  EXPECT_THAT(Run({"blpop", "k", "abc"}), ErrArg(facade::kTimeoutNotFloatErr));
+  EXPECT_THAT(Run({"blpop", "k", "nan"}), ErrArg(facade::kTimeoutNotFloatErr));
+  EXPECT_THAT(Run({"blpop", "k", "inf"}), ErrArg(facade::kTimeoutOutOfRangeErr));
+  EXPECT_THAT(Run({"blpop", "k", "-inf"}), ErrArg(facade::kTimeoutNegativeErr));
+  EXPECT_THAT(Run({"blpop", "k", "-1"}), ErrArg(facade::kTimeoutNegativeErr));
+  EXPECT_THAT(Run({"brpop", "k", "abc"}), ErrArg(facade::kTimeoutNotFloatErr));
+
+  // A finite timeout so large that timeout * 1000 overflows the unsigned millisecond counter is
+  // rejected instead of triggering undefined float->unsigned conversion.
+  EXPECT_THAT(Run({"blpop", "k", "1e10"}), ErrArg(facade::kTimeoutOutOfRangeErr));
+  EXPECT_THAT(Run({"blmpop", "1e10", "1", "k", "LEFT"}), ErrArg(facade::kTimeoutOutOfRangeErr));
+
+  // A large-but-representable timeout is accepted (returns immediately since the key exists).
+  Run({"rpush", "k", "v"});
+  EXPECT_THAT(Run({"blpop", "k", "4000000"}).GetVec(), ElementsAre("k", "v"));
+}
+
 // Wake two BLMOVEs on the same shard simultaneously
 TEST_F(ListFamilyTest, BLMoveSimultaneously) {
   EXPECT_EQ(Shard("src1", shard_set->size()),
diff --git a/src/server/topk_family.cc b/src/server/topk_family.cc
index 24effa683706..756e643d894c 100644
--- a/src/server/topk_family.cc
+++ b/src/server/topk_family.cc
@@ -36,6 +36,10 @@ namespace {
 constexpr char kDecayRangeErr[] = "decay must be between 0 and 1";
 constexpr char kIncrRangeErr[] = "increment must be between 1 and 100000";
 
+RuleError DecayRange(double v) {
+  return {!(v >= 0 && v <= 1), kDecayRangeErr};
+}
+
 OpStatus OpReserve(const OpArgs& op_args, string_view key, uint32_t k, uint32_t width,
                    uint32_t depth, double decay) {
   auto& db_slice = op_args.GetDbSlice();
@@ -196,7 +200,7 @@ void TopkFamily::Reserve(facade::CmdArgParser parser, CommandContext* cmd_cntx)
   if (parser.HasNext()) {
     width = parser.Next<uint32_t>();
     depth = parser.Next<uint32_t>();
-    decay = parser.Next<Validated<double, Bounded<0.0, 1.0, kDecayRangeErr>>>();
+    decay = parser.Next<Validated<double, DecayRange>>();
     RETURN_ON_PARSE_ERROR(parser, rb);
 
     if ((width == 0) || (depth == 0)) {