Skip to content

Commit

Permalink
Rewrite rtrim() to avoid a regex DoS
Browse files Browse the repository at this point in the history
  • Loading branch information
chriso committed Jul 21, 2016
1 parent 22426b0 commit 057d3b0
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 18 deletions.
10 changes: 8 additions & 2 deletions lib/rtrim.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@ function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { de

function rtrim(str, chars) {
(0, _assertString2.default)(str);
var pattern = chars ? new RegExp('[' + chars + ']+$', 'g') : /\s+$/g;
return str.replace(pattern, '');
var pattern = chars ? new RegExp('[' + chars + ']') : /\s/;

var idx = str.length - 1;
while (idx >= 0 && pattern.test(str[idx])) {
idx--;
}

return idx < str.length ? str.substr(0, idx + 1) : str;
}
module.exports = exports['default'];
12 changes: 7 additions & 5 deletions lib/trim.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,17 @@ Object.defineProperty(exports, "__esModule", {
});
exports.default = trim;

var _assertString = require('./util/assertString');
var _rtrim = require('./rtrim');

var _assertString2 = _interopRequireDefault(_assertString);
var _rtrim2 = _interopRequireDefault(_rtrim);

var _ltrim = require('./ltrim');

var _ltrim2 = _interopRequireDefault(_ltrim);

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

function trim(str, chars) {
(0, _assertString2.default)(str);
var pattern = chars ? new RegExp('^[' + chars + ']+|[' + chars + ']+$', 'g') : /^\s+|\s+$/g;
return str.replace(pattern, '');
return (0, _rtrim2.default)((0, _ltrim2.default)(str, chars), chars);
}
module.exports = exports['default'];
10 changes: 8 additions & 2 deletions src/lib/rtrim.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ import assertString from './util/assertString';

export default function rtrim(str, chars) {
assertString(str);
const pattern = chars ? new RegExp(`[${chars}]+$`, 'g') : /\s+$/g;
return str.replace(pattern, '');
const pattern = chars ? new RegExp(`[${chars}]`) : /\s/;

let idx = str.length - 1;
while (idx >= 0 && pattern.test(str[idx])) {
idx--;
}

return idx < str.length ? str.substr(0, idx + 1) : str;
}
15 changes: 12 additions & 3 deletions test/sanitizers.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,26 @@ describe('Sanitizers', function () {
it('should trim whitespace', function () {
test({
sanitizer: 'trim',
expect: { ' \r\n\tfoo \r\n\t ': 'foo' },
expect: {
' \r\n\tfoo \r\n\t ': 'foo',
' \r': '',
},
});

test({
sanitizer: 'ltrim',
expect: { ' \r\n\tfoo \r\n\t ': 'foo \r\n\t ' },
expect: {
' \r\n\tfoo \r\n\t ': 'foo \r\n\t ',
' \t \n': '',
},
});

test({
sanitizer: 'rtrim',
expect: { ' \r\n\tfoo \r\n\t ': ' \r\n\tfoo' },
expect: {
' \r\n\tfoo \r\n\t ': ' \r\n\tfoo',
' \r\n \t': '',
},
});
});

Expand Down
14 changes: 9 additions & 5 deletions validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -1001,14 +1001,18 @@

function rtrim(str, chars) {
assertString(str);
var pattern = chars ? new RegExp('[' + chars + ']+$', 'g') : /\s+$/g;
return str.replace(pattern, '');
var pattern = chars ? new RegExp('[' + chars + ']') : /\s/;

var idx = str.length - 1;
while (idx >= 0 && pattern.test(str[idx])) {
idx--;
}

return idx < str.length ? str.substr(0, idx + 1) : str;
}

function trim(str, chars) {
assertString(str);
var pattern = chars ? new RegExp('^[' + chars + ']+|[' + chars + ']+$', 'g') : /^\s+|\s+$/g;
return str.replace(pattern, '');
return rtrim(ltrim(str, chars), chars);
}

function escape(str) {
Expand Down
2 changes: 1 addition & 1 deletion validator.min.js

Large diffs are not rendered by default.

0 comments on commit 057d3b0

Please sign in to comment.