Modified code to support ANSI_QUOTES enabled servers #31
Conversation
…s that it still functions if the MySQL server has ANSI Quotes flag enabled
|
|
||
| utils.stringLiteral = function stringLiteral(val, escapeCharacter) { | ||
| var regex = new RegExp(escapeCharacter, 'g'); | ||
| return escapeCharacter+val.replace(regex, '\\'+escapeCharacter)+escapeCharacter; |
There was a problem hiding this comment.
I'm concerned about this approach. Consider the javascript string literal "bl\'ue" turning into 'bl\\\'ue' in SQL.
There was a problem hiding this comment.
What other approach do you recommend? I don't see anything wrong with it personally. I welcome your thoughts though.
There was a problem hiding this comment.
The concern is that malicious users can unescape the escape characters by inserting unescaped slashes. Escaping string literals should honestly be done in reference to the connection (see https://www.npmjs.com/package/mysql#escaping-query-values), but I don't think that will be simply achieved here. In the meantime, perhaps we should make use of mysql.escape? @tjwebb ?
|
Hey guys the whole backtick vs single quote vs double quote debate was the reason for the We should make the sql options configurable in the adapters instead of being hardcoded. |
|
This has bitten me in the arse again at work today. I will take a look this weekend and make another PR. |
|
@zivc can I continue your work on this? |
This whole library made use of double quotes. On MySQL servers with ANSI_QUOTES enabled, this was causing the following issue: https://github.com/balderdashy/sails-mysql/issues/195
I've also added a
utils.stringLiteralmethod so instead of having'"'+value+'"'around the place, it is now a little more tidier.I've also added an
identifierCharacteroption which defaults to the ` character.More info on ANSI_QUOTES here: http://dev.mysql.com/doc/refman/5.0/en/sql-mode.html#sqlmode_ansi_quotes