Merge pull request #382 from SaveTheRbtz/helpers

lib/helpers: fixed is_valid_packet()
statsd · Feb 11, 2014 · 5b90071 · 5b90071
2 parents 9cf48db + 63fb16c
commit 5b90071
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 21 deletions.
diff --git a/lib/helpers.js b/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;
     }
 
 };

diff --git a/test/helpers_tests.js b/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);