lib/helpers: fixed is_valid_packet() #382

Merged
merged 1 commit into from Feb 11, 2014
View
@@ -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;
@jeffyip
jeffyip Feb 19, 2014

Should this be Number(fields[0]) >= 0?

@SaveTheRbtz
SaveTheRbtz Feb 20, 2014 Contributor

Indeed it should, if you have time you can patch it and add test for that particular case. If not, I'll probably do it myself within a week or so.

+ default:
+ if (!isNumber(fields[0])) {
+ return false;
+ }
+ return true;
}
};
View
@@ -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);