Escape single quotes in string literals pushed to Cassandra and Geode#5075
Escape single quotes in string literals pushed to Cassandra and Geode#5075Samin061 wants to merge 2 commits into
Conversation
xiedeyantu
left a comment
xiedeyantu
left a comment
There was a problem hiding this comment.
I think the idea is good, but the code is not good.
| SqlTypeName typeName = field.getType().getSqlTypeName(); | ||
| if (typeName != SqlTypeName.CHAR) { | ||
| valueString = "'" + valueString + "'"; | ||
| valueString = "'" + valueString.replace("'", "''") + "'"; |
There was a problem hiding this comment.
If the value is I''m fine, I think replace will return an unexpected results.
There was a problem hiding this comment.
String.replace here is literal, not regex, so it doubles every embedded quote independently. For the value I''m fine (two adjacent quotes) it produces 'I''''m fine', which the CQL/OQL parser reads back as exactly I''m fine, so it round-trips fine. This is the same doubling core already does in SqlDialect.quoteStringLiteral (val.replace(literalEndQuoteString, literalEscapedQuote)). I pushed a Geode test with that exact value to lock it in.
There was a problem hiding this comment.
Apologies, I misunderstood the situation. I think this change is good; you can create a Jira ticket and—following the example of other PRs—update the title and adjust the commit message accordingly.
Signed-off-by: bibi samina <sam@bugqore.com>
|
Replied on the inline thread. The quote doubling mirrors what core does in SqlDialect.quoteStringLiteral, and I added a Geode test using your I''m fine example to show it round-trips. Happy to reshape it further if you have a specific form in mind. |
|
2 participants
Jira Link
No Jira filed yet; happy to open one under CALCITE if you'd prefer to track it there.
Changes Proposed
When the Cassandra and Geode adapters push a filter down to the backend, a SQL string literal is wrapped in single quotes to build the CQL/OQL predicate, but embedded single quotes are not escaped.
CassandraFilter.translateOp2andGeodeFilter.quoteCharLiteralemitcol = '<value>'verbatim, so a value that itself contains a quote breaks out of the string literal. A legitimate value such asO'Brienproduces malformed CQL/OQL, and a runtime-supplied literal can inject additional predicate syntax into the query that reachessession.execute/QueryService.newQuery.The Geode test for
WHERE city = 'a''b'currently makes the OQL parser fail withunexpected token: b, which shows the value leaking out of the literal. Doubling the embedded quote ('->'') keeps the value inside the literal for both dialects; this is the same escaping the Druid adapter already applies inDruidExpressions.stringLiteral.Added a Geode test that asserts the generated OQL is
... WHERE city = 'a''b', and a Cassandra test that a quoted filter value executes cleanly.