forked from TryGhost/NQL
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Changed regex operators to only work with STRINGs
refs: TryGhost#22 - The main use case for regex operators is doing type-ahead type matches on values. These kinds of values will start with a single char, contain spaces, and contain characters that should be interpretted literally. - To support this, the right value type in NQL is STRING, not LITERAL. LITERALs are designed for matching tag slugs and similar computed values. This commit changes the behaviour completely to only support STRING. - We also need to be super careful about how we handle regex characters and strings with SQL There I've added some more tests to demonstrate that the right thing is happening. I reordered some regex chars as the order ${} means something in JS template literals so this made it easier to test. And I added a few test cases for some scary-looking sqli attempts which should not work thanks to our usage of knex. NOTE: In testing this, I realised we don't need to be using whereRaw, as knex does actually have whereLike and whereILike which work correctly.
- Loading branch information
Showing
9 changed files
with
142 additions
and
24 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| require('../utils'); | ||
|
|
||
| const nql = require('../../'); | ||
| const knex = require('knex')({client: 'mysql'}); | ||
|
|
||
| describe('NQL -> SQL', function () { | ||
| describe('Can handle regexes safely', function () { | ||
| it('can handle regex STRING with escaped quotes', function () { | ||
| let query; | ||
|
|
||
| query = nql(`name:~'John O\\'Nolan'`); | ||
|
|
||
| query.lex().should.eql([ | ||
| {token: 'PROP', matched: 'name:'}, | ||
| {token: 'CONTAINS', matched: '~'}, | ||
| {token: 'STRING', matched: '\'John O\\\'Nolan\''} | ||
| ]); | ||
|
|
||
| query.toJSON().should.eql({name: {$regex: /John O'Nolan/i}}); | ||
| query.querySQL(knex('users')).toQuery().should.eql('select * from `users` where lower(`users`.`name`) like \'%john o\\\'nolan%\''); | ||
|
|
||
| query = nql(`name:~'John O\\"Nolan'`); | ||
| query.toJSON().should.eql({name: {$regex: /John O"Nolan/i}}); | ||
|
|
||
| query.lex().should.eql([ | ||
| {token: 'PROP', matched: 'name:'}, | ||
| {token: 'CONTAINS', matched: '~'}, | ||
| {token: 'STRING', matched: '\'John O\\"Nolan\''} | ||
| ]); | ||
|
|
||
| query.querySQL(knex('users')).toQuery().should.eql('select * from `users` where lower(`users`.`name`) like \'%john o\\"nolan%\''); | ||
|
|
||
| query = nql(`name:~'A\\'B\\"C\\"D\\''`); | ||
| query.toJSON().should.eql({name: {$regex: /A'B"C"D'/i}}); | ||
|
|
||
| query.lex().should.eql([ | ||
| {token: 'PROP', matched: 'name:'}, | ||
| {token: 'CONTAINS', matched: '~'}, | ||
| {token: 'STRING', matched: '\'A\\\'B\\"C\\"D\\\'\''} | ||
| ]); | ||
|
|
||
| query.querySQL(knex('users')).toQuery().should.eql('select * from `users` where lower(`users`.`name`) like \'%a\\\'b\\"c\\"d\\\'%\''); | ||
| }); | ||
|
|
||
| it('errors for unescaped quotes / injection patterns', function () { | ||
| (function () { | ||
| nql(`name:~'bad';'`).lex(); | ||
| }).should.throw(); | ||
|
|
||
| (function () { | ||
| nql(`name:~'';'`).lex(); | ||
| }).should.throw(); | ||
|
|
||
| let query; | ||
| // we can't force bad quotes, it errors as above | ||
| // query = nql("name:~'';select * from `settings` where `value` like ''"); | ||
|
|
||
| // Can we trick SQL? | ||
| query = nql("name:~'\\';select * from `settings` where `value` like \\''"); // eslint-disable-line quotes | ||
|
|
||
| // The regex only has the regex chars escaped, not quotes | ||
| query.toJSON().should.eql({name: {$regex: /';select \* from `settings` where `value` like '/i}}); | ||
|
|
||
| //SQL still ends up correctly escaped. This is all handled by knex... but having a test feels right | ||
| query.querySQL(knex('users')).toQuery().should.eql('select * from `users` where lower(`users`.`name`) like \'%\\\';select * from `settings` where `value` like \\\'%\''); | ||
| }); | ||
| }); | ||
| }); |