MongoDB adapter should escape embedded quotes in field references#5109
MongoDB adapter should escape embedded quotes in field references#5109Samin061 wants to merge 2 commits into
Conversation
|
We do not oppose using AI tools to contribute code. Multiple reviewers have repeatedly advised creating a Jira ticket before submitting contributions—you acknowledged this but have yet to follow through. Moving forward, non-compliant PRs will no longer be reviewed. |
| import static org.hamcrest.MatcherAssert.assertThat; | ||
|
|
||
| /** Tests for {@link MongoRules}. */ | ||
| class MongoRulesTest { |
There was a problem hiding this comment.
I am not crazy about these tests, but a complete query covering these new rules whose output is validated on Mongo would be nice.
There was a problem hiding this comment.
Fair. Dropped MongoRulesTest and replaced it with an end-to-end test in MongoAdapterTest: the datatypes collection now carries two fields whose names are x', injected: {$literal: 1}, y: 'z and a\, and the test selects them through _MAP, checks the generated $project stage, and checks the values Mongo returns.
On the old code the first query parses as {$project: {C: '$x', injected: {$literal: 1}, y: 'z'}}, so the injected field lands in the pipeline and the query fails; with the escaping it returns C=quoted. :mongodb:test is green (53 passed, 6 pre-existing skips).
|
Also still on the Jira: my ASF account request hasn't come through yet, so the PR title isn't prefixed with a CALCITE- id. I'll file the ticket and retitle this and my other open PRs as soon as it does. |
|
You don't need an ASF account, just a JIRA account to file issues. |
|
|
Got it, I had the two mixed up. Requesting the JIRA account now and I'll file the ticket for this one, then retitle this PR (and #5075, #5106) with the CALCITE- ids. Nothing else pending on the code side: the unit test you weren't keen on is gone, replaced by an end-to-end case in MongoAdapterTest that runs the query against Mongo and checks both the generated $project stage and the returned values. |
3 participants
Changes Proposed
MongoDB pipeline injection through unescaped quotes in pushed-down field references
The Mongo adapter renders
$project/$groupstages as strings that are later parsed withBsonDocument.parse.MongoRules.quotewrapped a value in single quotes but left embedded single quotes and backslashes untouched (the old// TODO: handle embedded quotes), and theITEMpath that handles_MAP['key']built'$key'the same way. Since the item key and field names come straight from the SQL statement, a value carrying a single quote closes the quoted token early and the rest is parsed as further pipeline syntax.Reproduction: a projection such as
SELECT _MAP['x'', evil: {$literal: 1}, y: ''z'] FROM mongogenerates{$project: {..., evil: {$literal: 1}, ...}}, so an extra stage field lands in the pipeline sent to MongoDB. After the fix the key stays a single string.The escaping lives in
quotebecause that is the one place every field reference passes through; the item-key branch now routes through it, and the aggregate paths already call it. Added a unit test that fails on the old code.