[
https://issues.apache.org/jira/browse/CASSANDRA-21784?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18131157#comment-18131157
]
Caleb Rackliffe commented on CASSANDRA-21784:
---------------------------------------------
bq. As a smaller point on the same line, matcher.group(2) keeps the surrounding
quotes, so the recorded index name is "name" rather than name.
This seems like an oversight, and it probably reasonable to fix up, assuming
tests are happy with it, etc.
bq. CREATE INDEX "my index" ON ks.t(v) ... CREATE INDEX "a""b" ON ks.t(v)
This is where I'm not sure. Given the index name isn't really something we have
to arbitrarily accept, and is even auto-generated when not specifically
provided, I'd perhaps lean toward keeping the stricter validation. (i.e. It's
fine to be stricter here than in other parts of CQL if we want to?)
> CQLTester.createIndex() rejects legal quoted index names containing non-word
> characters
> ---------------------------------------------------------------------------------------
>
> Key: CASSANDRA-21784
> URL: https://issues.apache.org/jira/browse/CASSANDRA-21784
> Project: Apache Cassandra
> Issue Type: Bug
> Components: Test/unit
> Reporter: Dimitri BEKALE
> Assignee: Preeti Toppo
> Priority: Low
> Time Spent: 20m
> Remaining Estimate: 0h
>
> {{CQLTester}} parses {{CREATE INDEX}} statements to record the index name for
> cleanup. The name pattern at
> [CQLTester.java:295|https://github.com/apache/cassandra/blob/b11047097f52482c4d9c48908a294e0ff6b552c4/test/unit/org/apache/cassandra/cql3/CQLTester.java#L295]
> is:
> {code:java}
> private static final String CREATE_INDEX_NAME_REGEX = "(\s*(\w*|\"\w*\")\s*)";
> {code}
> The quoted alternative is {{{}"\w*"{}}}, which admits only word characters
> between the quotes. CQL allows any character except {{"}} inside a quoted
> identifier, with {{""}} as the escape, so legal statements such as:
> {code:java}
> CREATE INDEX "my index" ON ks.t(v)
> CREATE INDEX "a""b" ON ks.t(v)
> {code}
> do not match. A non-match throws at line 1415:
> {code:java}
> Matcher matcher = CREATE_INDEX_PATTERN.matcher(formattedQuery);
> if (!matcher.find())
> throw new IllegalArgumentException("Expected valid create index query but
> found: " + formattedQuery);
> {code}
> so a test that uses such a name fails with {{IllegalArgumentException:
> Expected valid create index query but found: ...}} instead of creating the
> index.
> As a smaller point on the same line, {{matcher.group(2)}} keeps the
> surrounding quotes, so the recorded index name is {{"name"}} rather than
> {{{}name{}}}.
> h3. Impact
> Test infrastructure only; no server behaviour is affected. The cost is that a
> test author using a quoted index name containing a space or punctuation gets
> a misleading parse error from the harness rather than a working index.
> Severity is low.
> h3. Origin and affected branches
> The regex was added by CASSANDRA-13385 ("Delegate utests index name creation
> to CQLTester.createIndex", commit {{{}cff8dadbe853{}}}, 2017-05-08). I
> verified the line on {{trunk}} at {{{}b11047097f52{}}}; it is also present on
> {{{}cassandra-4.0{}}}, {{{}cassandra-4.1{}}}, {{cassandra-5.0,}} and
> {{{}cassandra-6.0{}}}.
> h3. Proposed fix
> Accept any non-quote character, plus the {{""}} escape:
> {code:java}
> - private static final String CREATE_INDEX_NAME_REGEX =
> "(\s*(\w*|\"\w*\")\s*)";
> + private static final String CREATE_INDEX_NAME_REGEX =
> "(\s*(\w+|\"([^\"]|\"\")*\")\s*)";
> {code}
> I have not run Cassandra CI, so I am proposing the change rather than a
> CI-validated patch. If the issue is confirmed I am happy to open a PR against
> the lowest affected branch you nominate and let CI decide.
> h3. Provenance
> This was found during an academic study of LLM-assisted code review at
> Jeonbuk National University, Jeonju, South Korea. An automated LLM-based
> reviewer produced the initial analysis, and I verified it by reading the
> source at {{{}b11047097f52{}}}. I believe it is a genuine, if minor, defect
> and will respect the project's triage either way.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]