Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backport #1782 to 3.x #1785

Merged
merged 1 commit into from
Oct 28, 2019
Merged

backport #1782 to 3.x #1785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ exports.findIndexOf = findIndexOf;
exports.collectTargetIds = collectTargetIds;
exports.idName = idName;
exports.rankArrayElements = rankArrayElements;
exports.escapeRegExp = escapeRegExp;

const g = require('strong-globalize')();
const traverse = require('traverse');
Expand Down Expand Up @@ -315,6 +316,36 @@ function isProhibited(key, prohibitedKeys) {
return false;
}

/**
* Accept an operator key and return whether it is used for a regular expression query or not
* @param {string} operator
* @returns {boolean}
*/
function isRegExpOperator(operator) {
return ['like', 'nlike', 'ilike', 'nilike', 'regexp'].includes(operator);
}

/**
* Accept a RegExp string and make sure that any special characters for RegExp are escaped in case they
* create an invalid Regexp
* @param {string} str
* @returns {string}
*/
function escapeRegExp(str) {
assert.strictEqual(typeof str, 'string', 'String required for regexp escaping');
try {
new RegExp(str); // try to parse string as regexp
return str;
} catch (unused) {
console.warn(
'Auto-escaping invalid RegExp value %j supplied by the caller. ' +
'Please note this behavior may change in the future.',
str
);
return str.replace(/[\-\[\]\/\{\}\(\)\+\?\.\\\^\$\|]/g, '\\$&');
}
}

/**
* Sanitize the query object
* @param query {object} The query object
Expand Down Expand Up @@ -390,6 +421,10 @@ function sanitizeQuery(query, options) {
return x;
}

if (isRegExpOperator(this.key) && typeof x === 'string') { // we have regexp supporting operator and string to escape
return escapeRegExp(x);
}

return x;
});

Expand Down
15 changes: 14 additions & 1 deletion test/basic-querying.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe('basic-querying', function() {
birthday: {type: Date, index: true},
role: {type: String, index: true},
order: {type: Number, index: true, sort: true},
tag: {type: String, index: true},
vip: {type: Boolean},
address: {
street: String,
Expand Down Expand Up @@ -591,6 +592,12 @@ describe('basic-querying', function() {
});
});

it('should sanitize invalid usage of like', async () => {
const users = await User.find({where: {tag: {like: '['}}});
users.should.have.length(1);
users[0].should.have.property('name', 'John Lennon');
});

it('should support "like" that is not satisfied',
function(done) {
User.find({where: {name: {like: 'Bob'}}},
Expand All @@ -616,6 +623,11 @@ describe('basic-querying', function() {
done();
});
});

it('should properly sanitize invalid ilike filter', async () => {
const users = await User.find({where: {name: {ilike: '['}}});
users.should.be.empty();
});
});

const itWhenNilikeSupported = connectorCapabilities.nilike !== false;
Expand Down Expand Up @@ -820,7 +832,7 @@ describe('basic-querying', function() {

sample({name: true}).expect(['name']);
sample({name: false}).expect([
'id', 'seq', 'email', 'role', 'order', 'birthday', 'vip', 'address', 'friends', 'addressLoc',
'id', 'seq', 'email', 'role', 'order', 'birthday', 'vip', 'address', 'friends', 'addressLoc', 'tag',
]);
sample({name: false, id: true}).expect(['id']);
sample({id: true}).expect(['id']);
Expand Down Expand Up @@ -1363,6 +1375,7 @@ function seed(done) {
birthday: new Date('1980-12-08'),
order: 2,
vip: true,
tag: '[singer]',
address: {
street: '123 A St',
city: 'San Jose',
Expand Down
15 changes: 14 additions & 1 deletion test/memory.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ describe('Memory connector', function() {
birthday: {type: Date, index: true},
role: {type: String, index: true},
order: {type: Number, index: true, sort: true},
tag: {type: String, index: true},
vip: {type: Boolean},
address: {
street: String,
Expand Down Expand Up @@ -229,6 +230,12 @@ describe('Memory connector', function() {
});
});

it('should properly sanitize like invalid query', async () => {
const users = await User.find({where: {tag: {like: '['}}});
users.should.have.length(1);
users[0].should.have.property('name', 'John Lennon');
});

it('should allow to find using like with regexp', function(done) {
User.find({where: {name: {like: /.*St.*/}}}, function(err, posts) {
should.not.exist(err);
Expand All @@ -253,6 +260,11 @@ describe('Memory connector', function() {
});
});

it('should sanitize nlike invalid query', async () => {
const users = await User.find({where: {name: {nlike: '['}}});
users.should.have.length(6);
});

it('should allow to find using nlike with regexp', function(done) {
User.find({where: {name: {nlike: /.*St.*/}}}, function(err, posts) {
should.not.exist(err);
Expand Down Expand Up @@ -513,7 +525,7 @@ describe('Memory connector', function() {
});

it('should support the regexp operator with regex strings', function(done) {
User.find({where: {name: {regexp: '^J'}}}, function(err, users) {
User.find({where: {name: {regexp: 'non$'}}}, function(err, users) {
should.not.exist(err);
users.length.should.equal(1);
users[0].name.should.equal('John Lennon');
Expand Down Expand Up @@ -562,6 +574,7 @@ describe('Memory connector', function() {
role: 'lead',
birthday: new Date('1980-12-08'),
vip: true,
tag: '[singer]',
address: {
street: '123 A St',
city: 'San Jose',
Expand Down
22 changes: 22 additions & 0 deletions test/util.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,28 @@ describe('util.sanitizeQuery', function() {
sanitizeQuery(q2, {prohibitedKeys: ['secret']});
q2.should.eql({and: [{}, {x: 1}]});
});

it('should allow proper structured regexp string', () => {
const q1 = {where: {name: {like: '^J'}}};
sanitizeQuery(q1).should.eql({where: {name: {like: '^J'}}});
});

it('should properly sanitize regexp string operators', () => {
const q1 = {where: {name: {like: '['}}};
sanitizeQuery(q1).should.eql({where: {name: {like: '\\['}}});

const q2 = {where: {name: {nlike: '['}}};
sanitizeQuery(q2).should.eql({where: {name: {nlike: '\\['}}});

const q3 = {where: {name: {ilike: '['}}};
sanitizeQuery(q3).should.eql({where: {name: {ilike: '\\['}}});

const q4 = {where: {name: {nilike: '['}}};
sanitizeQuery(q4).should.eql({where: {name: {nilike: '\\['}}});

const q5 = {where: {name: {regexp: '['}}};
sanitizeQuery(q5).should.eql({where: {name: {regexp: '\\['}}});
});
});

describe('util.parseSettings', function() {
Expand Down