-
Notifications
You must be signed in to change notification settings - Fork 7
Fix sql LIKE usage to properly escape special characters #6837
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
5888fa9
662dc3e
855f809
1b1884b
b6e16b3
d8f0766
cb0b41b
008ae8c
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 |
---|---|---|
|
@@ -27,6 +27,7 @@ | |
import org.labkey.api.collections.CsvSet; | ||
import org.labkey.api.collections.Sets; | ||
import org.labkey.api.data.ColumnInfo; | ||
import org.labkey.api.data.CompareType; | ||
import org.labkey.api.data.ConnectionWrapper; | ||
import org.labkey.api.data.CoreSchema; | ||
import org.labkey.api.data.DatabaseTableType; | ||
|
@@ -535,6 +536,40 @@ public SQLFragment appendInClauseSql(SQLFragment sql, @NotNull Collection<?> par | |
return DEFAULT_GENERATOR.appendInClauseSql(sql, params); | ||
} | ||
|
||
public SQLFragment appendCaseInsensitiveLikeClause(SQLFragment sql, @NotNull String matchStr, @Nullable String wildcardPrefix, @Nullable String wildcardSuffix, char escapeChar) | ||
{ | ||
String prefix = wildcardPrefix != null ? wildcardPrefix : ""; | ||
String suffix = wildcardSuffix != null ? wildcardSuffix : ""; | ||
String prefixLike = prefix + CompareType.escapeLikePattern(matchStr, escapeChar) + suffix; | ||
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. Should this be using 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. If matchStr were not constant (e.g. SQLFragment) then yes, but since these are all constant strings, this makes more sense. |
||
String escapeToken = " ESCAPE '" + escapeChar + "'"; | ||
sql.append(" ") | ||
.append(getCaseInsensitiveLikeOperator()) | ||
.append(" ") | ||
.appendValue(prefixLike) | ||
.append(escapeToken); | ||
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. I think escapeToken is always !, but it would be better to use quoteStringLiteral() 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. For sql server, quoteStringLiteral adds a "N" prefix and that doesn't work for the escape character. 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. Got it |
||
return sql; | ||
} | ||
|
||
public SQLFragment appendCaseInsensitiveLikeClause(SQLFragment sql, @NotNull String matchStr, @Nullable String wildcardPrefix, @Nullable String wildcardSuffix) | ||
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. I recommend unit tests for each of these incantations. 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. junit tests added |
||
{ | ||
return appendCaseInsensitiveLikeClause(sql, matchStr, wildcardPrefix, wildcardSuffix, '!'); | ||
} | ||
|
||
public SQLFragment appendCaseInsensitiveLikeClause(SQLFragment sql, @NotNull String matchStr) | ||
{ | ||
return appendCaseInsensitiveLikeClause(sql, matchStr, "%", "%", '!'); | ||
} | ||
|
||
public SQLFragment appendCaseInsensitiveStartsWith(SQLFragment sql, @NotNull String matchStr) | ||
{ | ||
return appendCaseInsensitiveLikeClause(sql, matchStr, null, "%", '!'); | ||
} | ||
|
||
public SQLFragment appendCaseInsensitiveEndsWith(SQLFragment sql, @NotNull String matchStr) | ||
{ | ||
return appendCaseInsensitiveLikeClause(sql, matchStr, "%", null, '!'); | ||
} | ||
|
||
public abstract boolean requiresStatementMaxRows(); | ||
|
||
/** | ||
|
@@ -2110,6 +2145,7 @@ public void testScopes() | |
this.s = scope; | ||
this.d = scope.getSqlDialect(); | ||
testDialectStringHandler(); | ||
testLikeOperator(); | ||
}); | ||
} | ||
|
||
|
@@ -2136,5 +2172,17 @@ void testDialectStringHandler() | |
for (String v : Arrays.asList("\\b", "\\f", "\\n", "\\r", "\\t", "\\1", "\\22", "\\333", "\\xf", "\\x20", "\\1234", "\\U12345678")) | ||
testEquals(v, new SQLFragment("SELECT ").appendStringLiteral(v, d)); | ||
} | ||
|
||
void testLikeOperator() | ||
{ | ||
String stringLiteralPrefix = d.isSqlServer() ? " N" : " "; | ||
assertEquals("SELECT * FROM A WHERE Name " + d.getCaseInsensitiveLikeOperator() + stringLiteralPrefix + "'ABC%' ESCAPE '!'", d.appendCaseInsensitiveStartsWith(new SQLFragment("SELECT * FROM A WHERE Name"), "ABC").toDebugString()); | ||
assertEquals("SELECT * FROM A WHERE Name " + d.getCaseInsensitiveLikeOperator() + stringLiteralPrefix + "'a!%bc%' ESCAPE '!'", d.appendCaseInsensitiveStartsWith(new SQLFragment("SELECT * FROM A WHERE Name"), "a%bc").toDebugString()); | ||
assertEquals("SELECT * FROM A WHERE Name " + d.getCaseInsensitiveLikeOperator() + stringLiteralPrefix + "'%ab!_C' ESCAPE '!'", d.appendCaseInsensitiveEndsWith(new SQLFragment("SELECT * FROM A WHERE Name"), "ab_C").toDebugString()); | ||
assertEquals("SELECT * FROM A WHERE Name " + d.getCaseInsensitiveLikeOperator() + stringLiteralPrefix + "'%a![b]C%' ESCAPE '!'", d.appendCaseInsensitiveLikeClause(new SQLFragment("SELECT * FROM A WHERE Name"), "a[b]C").toDebugString()); | ||
assertEquals("SELECT * FROM A WHERE Name " + d.getCaseInsensitiveLikeOperator() + stringLiteralPrefix + "'a![b]C_' ESCAPE '!'", d.appendCaseInsensitiveLikeClause(new SQLFragment("SELECT * FROM A WHERE Name"), "a[b]C", null, "_").toDebugString()); | ||
assertEquals("SELECT * FROM A WHERE Name " + d.getCaseInsensitiveLikeOperator() + stringLiteralPrefix + "'_a!_![b]C%' ESCAPE '!'", d.appendCaseInsensitiveLikeClause(new SQLFragment("SELECT * FROM A WHERE Name"), "a_[b]C", "_", "%").toDebugString()); | ||
assertEquals("SELECT * FROM A WHERE Name " + d.getCaseInsensitiveLikeOperator() + stringLiteralPrefix + "'_a[_[[b]C!d%' ESCAPE '['", d.appendCaseInsensitiveLikeClause(new SQLFragment("SELECT * FROM A WHERE Name"), "a_[b]C!d", "_", "%", '[').toDebugString()); | ||
} | ||
} | ||
} |
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.
nit: It would be nice to get
ConvertType.QClause.toSQLFragment()
converted over to using this.