lib/helpers: fixed is_valid_packet()

This commit introduces following changes:
 * Be liberal in what you recieve - do not rely on regexps for packet
   parsing but instead accept anything that can be parsed by JavaScript
   as a number e.g: +1e-17, -0511, 0xDEADbeef, etc
 * Accept both positive and negative counters with explicit / implicit
   sign
 * Provides more strict error checking then regexp, for example
   following strings match previously used '([\-\+\d\.]+' regexp:
   .
   .123.
   .\-+\0\+-.1-

Also while here added more tests.

Closes: statsd#350 statsd#357

Signed-off-by: Alexey Ivanov <SaveTheRbtz@GMail.com>

lib/helpers.js

@@ -7,33 +7,37 @@
  *
  * Returns true for a valid packet and false otherwise
  */
+function isNumber(str) {
+    return Boolean(str && !isNaN(str));
+}
+
 function is_valid_packet(fields) {
 
     // test for existing metrics type
     if (fields[1] === undefined) {
         return false;
     }
-    // filter out invalid metrics values
-    else if (fields[1] == 's') {
-        return true;
-    }
-    else if (fields[1] == 'g') {
-        if (!fields[0].match(/^([\-\+\d\.]+$)/)) {
+
+    // filter out malformed sample rates
+    if (fields[2] !== undefined) {
+        if (fields[2].length <= 1 || fields[2][0] != '@' || !isNumber(fields[2].substring(1))) {
             return false;
-        } else {
-            return true;
         }
     }
-    else if (!fields[0].match(/^([\d\.]+$)/)) {
-        return false;
-    }
-    // filter out malformed sample rates
-    else if (fields[2] && !fields[2].match(/^@([\d\.]+$)/)) {
-        return false;
-    }
-    // looks like we're good
-    else {
-        return true;
+
+    // filter out invalid metrics values
+    switch(fields[1]) {
+        case 's':
+            return true;
+        case 'g':
+            return isNumber(fields[0]);
+        case 'ms':
+            return isNumber(fields[0]) && Number(fields[0]) > 0;
+        default:
+            if (!isNumber(fields[0])) {
+                return false;
+            }
+            return true;
     }
 
 };

test/helpers_tests.js

@@ -20,14 +20,32 @@ module.exports = {
     test.done();
   },
 
-  counter_deltas_positive_are_not_valid: function (test) {
-    var res = helpers.is_valid_packet(['+10', 'c']);
+  counter_empty_are_invalid: function (test) {
+    var res = helpers.is_valid_packet(['', 'c']);
     test.equals(res, false);
     test.done();
   },
 
-  counter_deltas_negative_are_not_valid: function (test) {
+  counter_deltas_scientific_are_valid: function (test) {
+    var res = helpers.is_valid_packet(['+10e1', 'c']);
+    test.equals(res, true);
+    test.done();
+  },
+
+  counter_deltas_positive_are_valid: function (test) {
+    test.equals(helpers.is_valid_packet(['+10', 'c']), true);
+    test.equals(helpers.is_valid_packet(['+10e1', 'c']), true);
+    test.done();
+  },
+
+  counter_deltas_negative_are_valid: function (test) {
     var res = helpers.is_valid_packet(['-10', 'c']);
+    test.equals(res, true);
+    test.done();
+  },
+
+  gauges_empty_are_invalid: function (test) {
+    var res = helpers.is_valid_packet(['', 'g']);
     test.equals(res, false);
     test.done();
   },
@@ -56,6 +74,26 @@ module.exports = {
     test.done();
   },
 
+  sampling_is_invalid: function (test) {
+    test.equals(helpers.is_valid_packet(['345345', 'ms', '']), false);
+    test.equals(helpers.is_valid_packet(['345345', 'ms', 'b']), false);
+    test.equals(helpers.is_valid_packet(['345345', 'ms', 'blah']), false);
+    test.equals(helpers.is_valid_packet(['345345', 'ms', '@']), false);
+    test.equals(helpers.is_valid_packet(['345345', 'ms', '@blah']), false);
+    test.equals(helpers.is_valid_packet(['345345', 'ms', '@.']), false);
+    test.equals(helpers.is_valid_packet(['345345', 'ms', '@.1.']), false);
+    test.equals(helpers.is_valid_packet(['345345', 'ms', '@.1.2.3']), false);
+    test.done();
+  },
+
+  sampling_is_valid: function (test) {
+    test.equals(helpers.is_valid_packet(['345345', 'ms', '@2']), true);
+    test.equals(helpers.is_valid_packet(['345345', 'ms', '@.2']), true);
+    test.equals(helpers.is_valid_packet(['345345', 'ms', '@0.2']), true);
+    test.equals(helpers.is_valid_packet(['345345', 'ms', '@2e-1']), true);
+    test.done();
+  },
+
   correct_packet: function (test) {
     var res = helpers.is_valid_packet(['345345', 'ms', '@1.0']);
     test.equals(res, true);