-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fix MSSQL escaping #3382
Conversation
@@ -207,7 +207,15 @@ Object.assign(Client_MSSQL.prototype, { | |||
}, | |||
|
|||
wrapIdentifierImpl(value) { | |||
return value !== '*' ? `[${value.replace(/\[/g, '[')}]` : '*'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.replace(/\[/g, '[')}]
path was removed because it doesn't seem to do anything. Please correct me if that's wrong.
test/unit/dialects/mssql.js
Outdated
.toSQL(); | ||
console.log(sql); | ||
expect(sql.sql).to.equal( | ||
'select * from [projects] where "id] = 1 UNION SELECT 1, @@version -- --" = ?' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like this to be verified with integration test + to test identifier that has both "
and []
chars in it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I had no idea that mssql supports "
quoting for identifiers.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[] 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.
…tgriesser/knex into fix/mssql-escape � Conflicts: � CHANGELOG.md � package.json
No description provided.