Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Improved isDecimal and isInt validations, don't leak xss #39

Merged
merged 5 commits into from

2 participants

Christoph Tavan Chris O'Hara
Christoph Tavan

I added these two patches by @MaVo159 since I think isDecimal should

  • Disallow empty strings
  • Allow leading zeros like 01.123
  • Allow scientific notation like 2.2250738585072011e-308

and isInt should

  • Allow leading zeros like 01
  • Allow just zeros like 000

We also discovered an error in the test-suite: Basically all checks for failing values are useless, since they are entirely wrapped in a try catch block which means that also the exceptions raised by assert.ok(false) are always caught which results in none of these exceptions ever showing up.

We have fixed that for the isDecimal and isInt tests, see ctavan@b11c522#L1R229

Do you think you might fix the remaining tests at some point?

Cheers,
Christoph

ctavan added some commits
Christoph Tavan ctavan Improve isDecimal validation
- Disallow empty strings
- Allow leading zeros like 01.123
- Allow scientific notation like 2.2250738585072011e-308
b11c522
Christoph Tavan ctavan Improve isInt validation
- Allow leading zeros like 01
- Allow just zeros like 000

Original patch by MaVo159
1ec97aa
Christoph Tavan ctavan Fix unescaped - 79952c2
Christoph Tavan ctavan Convert filter to unix file format 8784c2f
Christoph Tavan ctavan Don't leak xss into global scope 5b29f6d
Christoph Tavan

I've added another fix which prevents the variable xss from being leaked into the global score.

Chris O'Hara
Owner

Thanks for this. I'll leave #40 open and get around to fixing the other tests when I have some time.

Chris O'Hara chriso merged commit d5d1262 into from
Tom Boutell boutell referenced this pull request from a commit in boutell/node-validator
Tom Boutell boutell Fixed https://github.com/chriso/node-validator/issues/77 semicolon in…
… wrong place when ensuring semicolon in an entity like '
e819669
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 27, 2011
  1. Christoph Tavan

    Improve isDecimal validation

    ctavan authored
    - Disallow empty strings
    - Allow leading zeros like 01.123
    - Allow scientific notation like 2.2250738585072011e-308
  2. Christoph Tavan

    Improve isInt validation

    ctavan authored
    - Allow leading zeros like 01
    - Allow just zeros like 000
    
    Original patch by MaVo159
Commits on Oct 28, 2011
  1. Christoph Tavan

    Fix unescaped -

    ctavan authored
  2. Christoph Tavan
  3. Christoph Tavan
This page is out of date. Refresh to see the latest.
Showing with 128 additions and 110 deletions.
  1. +102 −102 lib/filter.js
  2. +2 −2 lib/validator.js
  3. +24 −6 test/validator.test.js
204 lib/filter.js
View
@@ -1,102 +1,102 @@
-var entities = require('./entities');
- xss = require('./xss');
-
-var Filter = exports.Filter = function() {}
-
-var whitespace = '\\r\\n\\t\\s';
-
-Filter.prototype.modify = function(str) {
- this.str = str;
-}
-
-Filter.prototype.wrap = function (str) {
- return str;
-}
-
-Filter.prototype.value = function () {
- return this.str;
-}
-
-Filter.prototype.chain = function () {
- this.wrap = function () { return this };
- return this;
-}
-
-//Create some aliases - may help code readability
-Filter.prototype.convert = Filter.prototype.sanitize = function(str) {
- this.str = str;
- return this;
-}
-
-Filter.prototype.xss = function(is_image) {
- this.modify(xss.clean(this.str, is_image));
- return this.wrap(this.str);
-}
-
-Filter.prototype.entityDecode = function() {
- this.modify(entities.decode(this.str));
- return this.wrap(this.str);
-}
-
-Filter.prototype.entityEncode = function() {
- this.modify(entities.encode(this.str));
- return this.wrap(this.str);
-}
-
-Filter.prototype.ltrim = function(chars) {
- chars = chars || whitespace;
- this.modify(this.str.replace(new RegExp('^['+chars+']+', 'g'), ''));
- return this.wrap(this.str);
-}
-
-Filter.prototype.rtrim = function(chars) {
- chars = chars || whitespace;
- this.modify(this.str.replace(new RegExp('['+chars+']+$', 'g'), ''));
- return this.wrap(this.str);
-}
-
-Filter.prototype.trim = function(chars) {
- chars = chars || whitespace;
- this.modify(this.str.replace(new RegExp('^['+chars+']+|['+chars+']+$', 'g'), ''));
- return this.wrap(this.str);
-}
-
-Filter.prototype.ifNull = function(replace) {
- if (!this.str || this.str === '') {
- this.modify(replace);
- }
- return this.wrap(this.str);
-}
-
-Filter.prototype.toFloat = function() {
- this.modify(parseFloat(this.str));
- return this.wrap(this.str);
-}
-
-Filter.prototype.toInt = function(radix) {
- radix = radix || 10;
- this.modify(parseInt(this.str, radix));
- return this.wrap(this.str);
-}
-
-//Any strings with length > 0 (except for '0' and 'false') are considered true,
-//all other strings are false
-Filter.prototype.toBoolean = function() {
- if (!this.str || this.str == '0' || this.str == 'false' || this.str == '') {
- this.modify(false);
- } else {
- this.modify(true);
- }
- return this.wrap(this.str);
-}
-
-//String must be equal to '1' or 'true' to be considered true, all other strings
-//are false
-Filter.prototype.toBooleanStrict = function() {
- if (this.str == '1' || this.str == 'true') {
- this.modify(true);
- } else {
- this.modify(false);
- }
- return this.wrap(this.str);
-}
+var entities = require('./entities');
+var xss = require('./xss');
+
+var Filter = exports.Filter = function() {}
+
+var whitespace = '\\r\\n\\t\\s';
+
+Filter.prototype.modify = function(str) {
+ this.str = str;
+}
+
+Filter.prototype.wrap = function (str) {
+ return str;
+}
+
+Filter.prototype.value = function () {
+ return this.str;
+}
+
+Filter.prototype.chain = function () {
+ this.wrap = function () { return this };
+ return this;
+}
+
+//Create some aliases - may help code readability
+Filter.prototype.convert = Filter.prototype.sanitize = function(str) {
+ this.str = str;
+ return this;
+}
+
+Filter.prototype.xss = function(is_image) {
+ this.modify(xss.clean(this.str, is_image));
+ return this.wrap(this.str);
+}
+
+Filter.prototype.entityDecode = function() {
+ this.modify(entities.decode(this.str));
+ return this.wrap(this.str);
+}
+
+Filter.prototype.entityEncode = function() {
+ this.modify(entities.encode(this.str));
+ return this.wrap(this.str);
+}
+
+Filter.prototype.ltrim = function(chars) {
+ chars = chars || whitespace;
+ this.modify(this.str.replace(new RegExp('^['+chars+']+', 'g'), ''));
+ return this.wrap(this.str);
+}
+
+Filter.prototype.rtrim = function(chars) {
+ chars = chars || whitespace;
+ this.modify(this.str.replace(new RegExp('['+chars+']+$', 'g'), ''));
+ return this.wrap(this.str);
+}
+
+Filter.prototype.trim = function(chars) {
+ chars = chars || whitespace;
+ this.modify(this.str.replace(new RegExp('^['+chars+']+|['+chars+']+$', 'g'), ''));
+ return this.wrap(this.str);
+}
+
+Filter.prototype.ifNull = function(replace) {
+ if (!this.str || this.str === '') {
+ this.modify(replace);
+ }
+ return this.wrap(this.str);
+}
+
+Filter.prototype.toFloat = function() {
+ this.modify(parseFloat(this.str));
+ return this.wrap(this.str);
+}
+
+Filter.prototype.toInt = function(radix) {
+ radix = radix || 10;
+ this.modify(parseInt(this.str, radix));
+ return this.wrap(this.str);
+}
+
+//Any strings with length > 0 (except for '0' and 'false') are considered true,
+//all other strings are false
+Filter.prototype.toBoolean = function() {
+ if (!this.str || this.str == '0' || this.str == 'false' || this.str == '') {
+ this.modify(false);
+ } else {
+ this.modify(true);
+ }
+ return this.wrap(this.str);
+}
+
+//String must be equal to '1' or 'true' to be considered true, all other strings
+//are false
+Filter.prototype.toBooleanStrict = function() {
+ if (this.str == '1' || this.str == 'true') {
+ this.modify(true);
+ } else {
+ this.modify(false);
+ }
+ return this.wrap(this.str);
+}
4 lib/validator.js
View
@@ -113,14 +113,14 @@ Validator.prototype.isUppercase = function() {
}
Validator.prototype.isInt = function() {
- if (!this.str.match(/^(?:-?(?:0|[1-9][0-9]*))$/)) {
+ if (!this.str.match(/^(?:-?(?:[0-9][0-9]*)(?:\.?0+)?)$/)) {
return this.error(this.msg || 'Invalid integer');
}
return this;
}
Validator.prototype.isDecimal = function() {
- if (!this.str.match(/^(?:-?(?:0|[1-9][0-9]*))?(?:\.[0-9]*)?$/)) {
+ if (this.str === '' || !this.str.match(/^(?:-?(?:[0-9]+))?(?:\.[0-9]*)?(?:[eE][\+\-]?(?:[0-9]+))?$/)) {
return this.error(this.msg || 'Invalid decimal');
}
return this;
30 test/validator.test.js
View
@@ -198,12 +198,20 @@ module.exports = {
assert.ok(Validator.check(0).isInt());
assert.ok(Validator.check(123).isInt());
assert.ok(Validator.check('-0').isInt());
+ assert.ok(Validator.check('01').isInt());
+ assert.ok(Validator.check('-01').isInt());
+ assert.ok(Validator.check('000').isInt());
- ['123.123','01','000',' ',''].forEach(function(str) {
+ ['123.123',' ',''].forEach(function(str) {
try {
Validator.check(str).isInt();
- assert.ok(false, 'isInt failed');
- } catch (e) {}
+ assert.ok(false, 'falsepositive');
+ } catch (e) {
+ if (e.message == 'falsepositive') {
+ e.message = 'isInt had a false positive: ' + str;
+ throw e;
+ }
+ }
});
},
@@ -217,12 +225,22 @@ module.exports = {
assert.ok(Validator.check('.0').isDecimal());
assert.ok(Validator.check('0').isDecimal());
assert.ok(Validator.check('-0').isDecimal());
+ assert.ok(Validator.check('01.123').isDecimal());
+
+ assert.ok(Validator.check('2.2250738585072011e-308').isDecimal());
+ assert.ok(Validator.check('-0.22250738585072011e-307').isDecimal());
+ assert.ok(Validator.check('-0.22250738585072011E-307').isDecimal());
- ['-.123','01.123',' ',''].forEach(function(str) {
+ ['-.123',' ',''].forEach(function(str) {
try {
Validator.check(str).isDecimal();
- assert.ok(false, 'isDecimal failed');
- } catch (e) {}
+ assert.ok(false, 'falsepositive');
+ } catch (e) {
+ if (e.message == 'falsepositive') {
+ e.message = 'isDecimal had a false positive: ' + str;
+ throw e;
+ }
+ }
});
},
Something went wrong with that request. Please try again.