-
Notifications
You must be signed in to change notification settings - Fork 6
Proper fixup for " when generating CQL query #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,8 +78,8 @@ void toXCQLInternal(XCQLBuilder b, int level, List<CQLPrefix> prefixes, | |
|
|
||
| @Override | ||
| public String toCQL() { | ||
| String quotedIndex = maybeQuote(index); | ||
| String quotedTerm = maybeQuote(term); | ||
| String quotedIndex = toCQLTerm(index); | ||
| String quotedTerm = toCQLTerm(term); | ||
| String res = quotedTerm; | ||
|
|
||
| if (index != null && | ||
|
|
@@ -192,7 +192,7 @@ public String toPQF(Properties config) throws PQFTranslationException { | |
| if (isResultSetIndex(index)) { | ||
| // Special case: ignore relation, modifiers, wildcards, etc. | ||
| // There's parallel code in toType1BER() | ||
| return "@set " + maybeQuote(term); | ||
| return "@set " + toCQLTerm(term); | ||
| } | ||
|
|
||
| List<String> attrs = getAttrs(config); | ||
|
|
@@ -219,28 +219,38 @@ public String toPQF(Properties config) throws PQFTranslationException { | |
| text = text.substring(0, len - 1); | ||
| } | ||
|
|
||
| return s + maybeQuote(text); | ||
| return s + toCQLTerm(text); | ||
| } | ||
|
|
||
| static String maybeQuote(String str) { | ||
| if (str == null) | ||
| // ensure that a term is properly quoted for CQL output if necessary. | ||
| // If the term has a bare double-quote (") it will be | ||
| // escaped with a backslash. | ||
| static String toCQLTerm(String str) { | ||
| if (str == null) { | ||
| return null; | ||
|
|
||
| // There _must_ be a better way to make this test ... | ||
| if (str.length() == 0 || | ||
| str.indexOf('"') != -1 || | ||
| str.indexOf(' ') != -1 || | ||
| str.indexOf('\t') != -1 || | ||
| str.indexOf('=') != -1 || | ||
| str.indexOf('<') != -1 || | ||
| str.indexOf('>') != -1 || | ||
| str.indexOf('/') != -1 || | ||
| str.indexOf('(') != -1 || | ||
| str.indexOf(')') != -1) { | ||
| str = '"' + str.replaceAll("(?<!\\\\)\"", "\\\\\"") + '"'; | ||
| } | ||
|
|
||
| return str; | ||
| boolean quote = str.isEmpty(); | ||
| boolean escaped = false; | ||
| StringBuilder sb = new StringBuilder(); | ||
| for (char ch : str.toCharArray()) { | ||
| if (CQLLexer.OPS_AND_WHITESPACE.indexOf(ch) >= 0) { | ||
| quote = true; | ||
| } | ||
| if (ch == '"' && !escaped) { | ||
| sb.append('\\'); | ||
| } | ||
| escaped = ch == '\\' && !escaped; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @adamdickmeiss what if
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will be left as is.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But the resulting query will be invalid no? |
||
| sb.append(ch); | ||
| } | ||
| if (escaped) { | ||
| // trailing backslash - escape it | ||
| sb.append('\\'); | ||
| } | ||
| if (quote) { | ||
| return "\"" + sb.toString() + "\""; | ||
| } else { | ||
| return sb.toString(); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| package org.z3950.zing.cql; | ||
|
|
||
| import org.junit.Test; | ||
| import static org.junit.Assert.*; | ||
|
|
||
| public class CQLTermNodeTest { | ||
| @Test | ||
| public void TestTCQLTermQuoteNull() { | ||
| assertNull(CQLTermNode.toCQLTerm(null)); | ||
| } | ||
|
|
||
| @Test | ||
| public void TestTCQLTermQuoteEmpty() { | ||
| assertEquals("\"\"", CQLTermNode.toCQLTerm("")); | ||
| } | ||
|
|
||
| @Test | ||
| public void TestTCQLTermQuoteRelation() { | ||
| assertEquals("\"<\"", CQLTermNode.toCQLTerm("<")); | ||
| } | ||
|
|
||
| @Test | ||
| public void TestTCQLTermQuoteSimple() { | ||
| assertEquals("simple", CQLTermNode.toCQLTerm("simple")); | ||
| } | ||
|
|
||
| @Test | ||
| public void TestTCQLTermQuoteBlank() { | ||
| assertEquals("\"a b\"", CQLTermNode.toCQLTerm("a b")); | ||
| } | ||
|
|
||
| @Test | ||
| public void TestTCQLTermQuoteQuote1() { | ||
| assertEquals("a\\\"", CQLTermNode.toCQLTerm("a\"")); | ||
| } | ||
|
|
||
| @Test | ||
| public void TestTCQLTermQuoteQuote2() { | ||
| assertEquals("a\\\"", CQLTermNode.toCQLTerm("a\\\"")); | ||
| } | ||
|
|
||
| @Test | ||
| public void TestTCQLTermQuoteQuote3() { | ||
| assertEquals("a" + "\\\\" + "\\\"", CQLTermNode.toCQLTerm("a" + "\\\\" + "\"")); | ||
| } | ||
|
|
||
| @Test | ||
| public void TestTCQLTermQuoteQuote4() { | ||
| assertEquals("a" + "\\\\" + "\\\"", CQLTermNode.toCQLTerm("a" + "\\\\" + "\\\"")); | ||
| } | ||
|
|
||
| @Test | ||
| public void TestTCQLTermQuoteBackSlashTrail1() { | ||
| assertEquals("a\\\\", CQLTermNode.toCQLTerm("a\\")); | ||
| } | ||
|
|
||
| @Test | ||
| public void TestTCQLTermQuoteBackSlashTrail2() { | ||
| assertEquals("\"a \\\\\"", CQLTermNode.toCQLTerm("a \\")); | ||
| } | ||
|
|
||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamdickmeiss this function name is confusing since it both quotes and escapes. Btw, we should do the same in cql-go.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, the function name is bad.
There will never be a bare " as the result of a query in the current form. But it did happen in latest release until #5. This is just extra precaution to ensure that if it is bare, it will be escaped. That's all.