obfuscate: treat strings as strings#473
Conversation
This change reverts a previous change where quoted strings were incorrectly treated as identifiers by the tokenizer to avoid their obfuscation. Now, they are still identified as ID (identifier) but are scanned as strings. This matches the behavior of the original `youtube/vitess` package [here](https://github.com/youtube/vitess/blob/addf2c2907240612cb07a5fda2b47068b2e414c9/go/vt/sqlparser/token.go#L603).
| case '\'': | ||
| return tkn.scanString(ch, String) | ||
| case '"': | ||
| return tkn.scanString(ch, ID) |
There was a problem hiding this comment.
Wait, what is that change supposed to do? Because in MySQL apparently you can have both modes: https://stackoverflow.com/questions/11321491/when-to-use-single-quotes-double-quotes-and-back-ticks-in-mysql/14123649#14123649 so globally handling backquote and double-quotes the same way seems right but other RDBMs such as Pg have different semantics, AFAIK in the Pg case double quotes mean "this is a column identifier and it is case sensitive" https://stackoverflow.com/questions/20878932/are-postgresql-column-names-case-sensitive/20880247#20880247 si for Pg, treating double-quotes enclosed strings as identifiers is the right thing to do.
There was a problem hiding this comment.
That difference is not important to us. All we care about here is that between quotes there is a string.
Before this change, whatever was between the quotes was treated as an identifier because our quantizer obfuscates all strings. I believe @palazzem did that as a work around to avoid double-quoted strings from being obfuscated (e.g. DATE_FORMAT parameters). This is problematic because scanning quoted text as an identifier will cause the tokenizer to fail when encountering characters such as % which are found in "DATE_FORMAT" strings. However this is a valid character.
With this change, strings are scanned as strings but will be marked ID (identifier) by the tokenizer (see the second parameter to scanString) and will continue to pass the obfuscator.
I hope my explanation makes sense.
There was a problem hiding this comment.
I keep reading my explanation above and I don't like it. It's not too clear. I'll try again:
- Previously, strings between double quotes were treated as identifiers to avoid their obfuscation. This was a problem because characters such as
%are invalid in identifiers, but valid in strings. - With this change, double quoted strings are scanned as strings but marked as identifiers, which will stop them from being obfuscated.
The test presents a valid SQL test case which was failing the tokenizer before. The youtube/vitess tokenizer also works like this (ours is copied from there) which further confirms that this is the correct way to scan double quoted strings.
There was a problem hiding this comment.
OK, GTM, sorry about all that noise, thanks for the thorough explanation.
2 participants
This change reverts a previous change where quoted strings were
incorrectly treated as identifiers by the tokenizer to avoid their
obfuscation. Now, they are still identified as ID (identifier) but are
scanned as strings.
This matches the behavior of the original
youtube/vitesspackagehere.