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.

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

Fix MSSQL escaping #3382

Merged
merged 3 commits into from Oct 6, 2019

Conversation

@kibertoad
Copy link
Collaborator

kibertoad commented Aug 4, 2019

No description provided.

@kibertoad kibertoad requested a review from elhigu Aug 4, 2019
@@ -207,7 +207,15 @@ Object.assign(Client_MSSQL.prototype, {
},

wrapIdentifierImpl(value) {
return value !== '*' ? `[${value.replace(/\[/g, '[')}]` : '*';

This comment has been minimized.

Copy link
@kibertoad

kibertoad Aug 4, 2019

Author Collaborator

.replace(/\[/g, '[')}] path was removed because it doesn't seem to do anything. Please correct me if that's wrong.

@kibertoad kibertoad requested a review from tgriesser Aug 4, 2019
.toSQL();
console.log(sql);
expect(sql.sql).to.equal(
'select * from [projects] where "id] = 1 UNION SELECT 1, @@version -- --" = ?'

This comment has been minimized.

Copy link
@elhigu

elhigu Aug 16, 2019

Member

I would like this to be verified with integration test + to test identifier that has both " and [] chars in it.

This comment has been minimized.

Copy link
@elhigu

elhigu Aug 16, 2019

Member

(I had no idea that mssql supports " quoting for identifiers.)

This comment has been minimized.

Copy link
@kibertoad

kibertoad Oct 6, 2019

Author Collaborator

[] are illegal symbols as per MSSQL documentation, apparently, so we don't need to test that. As advised by snyk people, I replaced our escaping logic with the one that sequelize uses, which removes [] in the first place.
And MSSQL does not support arrays.

kibertoad added 2 commits Oct 6, 2019
…tgriesser/knex into fix/mssql-escape

� Conflicts:
�	CHANGELOG.md
�	package.json
@kibertoad kibertoad merged commit 988fb24 into master Oct 6, 2019
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
@kibertoad kibertoad deleted the fix/mssql-escape branch Oct 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.