flesh out safe_strto* util funcs, and make incrdecr use them

5 files changed, 91 insertions, 23 deletions

diff --git a/internal_tests.c b/internal_tests.c
index 836b012..4df068a 100644
--- a/internal_tests.c
+++ b/internal_tests.c
@@ -6,17 +6,57 @@
 
 #include "memcached.h"
 
-int main(int argc, char **argv) {
-  unsigned long long ull;
-  assert(safe_strtoull("123", &ull));
-  assert(ull == 123);
+static void test_safe_strtoull(void);
+static void test_safe_strtoll(void);
+
+static void test_safe_strtoull() {
+  unsigned long long val;
+  assert(safe_strtoull("123", &val));
+  assert(val == 123);
+  assert(safe_strtoull("+123", &val));
+  assert(val == 123);
+  assert(!safe_strtoull("", &val));  // empty
+  assert(!safe_strtoull("123BOGUS", &val));  // non-numeric
+  assert(!safe_strtoull("92837498237498237498029383", &val)); // out of range
+
+  // extremes:
+  assert(safe_strtoull("18446744073709551615", &val)); // 2**64 - 1
+  assert(val == 18446744073709551615ULL);
+  assert(!safe_strtoull("18446744073709551616", &val)); // 2**64
+  assert(!safe_strtoull("-1", &val));  // negative
+}
 
-  // Empty:
-  assert(!safe_strtoull("", &ull));
+static void test_safe_strtoll() {
+  long long val;
+  assert(safe_strtoll("123", &val));
+  assert(val == 123);
+  assert(safe_strtoll("+123", &val));
+  assert(val == 123);
+  assert(safe_strtoll("-123", &val));
+  assert(val == -123);
+  assert(!safe_strtoll("", &val));  // empty
+  assert(!safe_strtoll("123BOGUS", &val));  // non-numeric
+  assert(!safe_strtoll("92837498237498237498029383", &val)); // out of range
 
-  // Bogus:
-  assert(!safe_strtoull("123BOGUS", &ull));
+  // extremes:
+  assert(!safe_strtoll("18446744073709551615", &val)); // 2**64 - 1
+  assert(safe_strtoll("9223372036854775807", &val)); // 2**63 - 1
+  assert(val == 9223372036854775807LL);
+  /*
+  assert(safe_strtoll("-9223372036854775808", &val)); // -2**63
+  assert(val == -9223372036854775808LL);
+  */
+  assert(!safe_strtoll("-9223372036854775809", &val)); // -2**63 - 1
 
+  // We'll allow space to terminate the string.  And leading space.
+  assert(safe_strtoll(" 123 foo", &val));
+  assert(val == 123);
+
+}
+
+int main(int argc, char **argv) {
+  test_safe_strtoull();
+  test_safe_strtoll();
   printf("OK.\n");
   return 0;
 }
diff --git a/memcached.c b/memcached.c
index aa766cd..b342e6a 100644
--- a/memcached.c
+++ b/memcached.c
@@ -2622,12 +2622,12 @@ static void process_update_command(conn *c, token_t *tokens, const size_t ntoken
     vlen = strtol(tokens[4].value, NULL, 10);
 
     // does cas value exist?
-    if(handle_cas) {
-      req_cas_id = strtoull(tokens[5].value, NULL, 10);
+    if (handle_cas) {
+        req_cas_id = strtoull(tokens[5].value, NULL, 10);
     }
 
-    if(errno == ERANGE || ((flags == 0 || exptime == 0) && errno == EINVAL)
-       || vlen < 0) {
+    if (errno == ERANGE || ((flags == 0 || exptime == 0) && errno == EINVAL)
+        || vlen < 0) {
         out_string(c, "CLIENT_ERROR bad command line format");
         return;
     }
@@ -2679,7 +2679,7 @@ static void process_arithmetic_command(conn *c, token_t *tokens, const size_t nt
 
     set_noreply_maybe(c, tokens, ntokens);
 
-    if(tokens[KEY_TOKEN].length > KEY_MAX_LENGTH) {
+    if (tokens[KEY_TOKEN].length > KEY_MAX_LENGTH) {
         out_string(c, "CLIENT_ERROR bad command line format");
         return;
     }
@@ -2687,10 +2687,8 @@ static void process_arithmetic_command(conn *c, token_t *tokens, const size_t nt
     key = tokens[KEY_TOKEN].value;
     nkey = tokens[KEY_TOKEN].length;
 
-    delta = strtoll(tokens[2].value, NULL, 10);
-
-    if(errno == ERANGE) {
-        out_string(c, "CLIENT_ERROR bad command line format");
+    if (!safe_strtoll(tokens[2].value, &delta)) {
+        out_string(c, "CLIENT_ERROR invalid numeric delta argument");
         return;
     }
 
@@ -2729,11 +2727,8 @@ char *do_add_delta(conn *c, item *it, const bool incr, const int64_t delta, char
     int res;
 
     ptr = ITEM_data(it);
-    while ((*ptr != '\0') && (*ptr < '0' && *ptr > '9')) ptr++;    // BUG: can't be true
-
-    value = strtoull(ptr, NULL, 10);
 
-    if(errno == ERANGE) {
+    if (!safe_strtoull(ptr, &value)) {
         return "CLIENT_ERROR cannot increment or decrement non-numeric value";
     }
 
diff --git a/t/incrdecr.t b/t/incrdecr.t
index bce9af3..83153c7 100755
--- a/t/incrdecr.t
+++ b/t/incrdecr.t
@@ -61,5 +61,7 @@ is(scalar <$sock>, "NOT_FOUND\r\n", "can't incr bogus key");
 print $sock "set text 0 0 2\r\nhi\r\n";
 is(scalar <$sock>, "STORED\r\n", "stored text");
 print $sock "incr text 1\r\n";
-is(scalar <$sock>, "1\r\n", "hi - 1 = 0");
+is(scalar <$sock>,
+   "CLIENT_ERROR cannot increment or decrement non-numeric value\r\n",
+   "hi - 1 = 0");
 
diff --git a/util.c b/util.c
index dbd62a1..f580669 100644
--- a/util.c
+++ b/util.c
@@ -1,16 +1,45 @@
 #include <stdlib.h>
 #include <assert.h>
+#include <ctype.h>
+#include <errno.h>
+#include <string.h>
+#include <stdlib.h>
 
 #include "memcached.h"
 
 bool safe_strtoull(const char *str, unsigned long long *out) {
   assert(out != NULL);
+  errno = 0;
   *out = 0;
   char *endptr;
   unsigned long long ull = strtoull(str, &endptr, 10);
-  if (*endptr == '\0' && endptr != str) {
+  if (errno == ERANGE)
+    return false;
+  if (isspace(*endptr) || (*endptr == '\0' && endptr != str)) {
+    if ((long long) ull < 0) {
+      // only check for negative signs in the uncommon case when the unsigned
+      // number is so big that it's negative as a signed number.
+      if (strchr(str, '-') != NULL) {
+        return false;
+      }
+    }
     *out = ull;
     return true;
   }
   return false;
 }
+
+bool safe_strtoll(const char *str, long long *out) {
+  assert(out != NULL);
+  errno = 0;
+  *out = 0;
+  char *endptr;
+  long long ll = strtoll(str, &endptr, 10);
+  if (errno == ERANGE)
+    return false;
+  if (isspace(*endptr) || (*endptr == '\0' && endptr != str)) {
+    *out = ll;
+    return true;
+  }
+  return false;
+}
diff --git a/util.h b/util.h
index 24b0da6..38f0e4c 100644
--- a/util.h
+++ b/util.h
@@ -5,4 +5,6 @@
  * returns true if conversion succeeded.
  */
 bool safe_strtoull(const char *str, unsigned long long *out);
+bool safe_strtoll(const char *str, long long *out);
+