Repository navigation
fix: escape embedded backticks in column names (#316) #679
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
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 |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| using ClickHouse.Driver.Utility; | ||
|
|
||
| namespace ClickHouse.Driver.Tests.Utility; | ||
|
|
||
| [TestFixture] | ||
| public class QuotedStringTests | ||
| { | ||
| [Test] | ||
| public void EncloseColumnName_EscapesInteriorBackticks() | ||
| { | ||
| var result = "weird`name".EncloseColumnName(); | ||
|
|
||
| Assert.That(result, Is.EqualTo("`weird\\`name`")); | ||
| } | ||
|
|
||
| [Test] | ||
| public void EncloseColumnName_AlreadyEnclosedButNeedsEscape_EscapesInterior() | ||
| { | ||
| var result = "`col`name`".EncloseColumnName(); | ||
|
|
||
| Assert.That(result, Is.EqualTo("`\\`col\\`name\\``")); | ||
| } | ||
|
|
||
| [Test] | ||
| public void EncloseColumnName_HandlesNullOrEmpty() | ||
| { | ||
| Assert.That(StringExtensions.EncloseColumnName(null), Is.Null); | ||
| Assert.That(StringExtensions.EncloseColumnName(string.Empty), Is.EqualTo(string.Empty)); | ||
| } | ||
|
|
||
| [Test] | ||
| public void QuoteSingle_EscapesQuotesAndAddsOuterQuotes() | ||
| { | ||
| var result = "'O'Reilly'".QuoteSingle(); | ||
|
|
||
| Assert.That(result, Is.EqualTo("'\\'O\\'Reilly\\''")); | ||
| } | ||
|
|
||
| [Test] | ||
| public void QuoteSingle_EmptyString_ReturnsQuotedEmptyString() | ||
| { | ||
| Assert.That(string.Empty.QuoteSingle(), Is.EqualTo("''")); | ||
| } | ||
|
|
||
| [Test] | ||
| public void QuoteDouble_EscapesQuotesAndAddsOuterQuotes() | ||
| { | ||
| var input = "\"Product \"Items\"\""; | ||
| var result = input.QuoteDouble(); | ||
|
|
||
| Assert.That(result, Is.EqualTo("\"\\\"Product \\\"Items\\\"\\\"\"")); | ||
| } | ||
|
|
||
| [Test] | ||
| public void QuoteDouble_EmptyString_ReturnsQuotedEmptyString() | ||
| { | ||
| Assert.That(string.Empty.QuoteDouble(), Is.EqualTo("\"\"")); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,10 +9,10 @@ internal static class StringExtensions | |
| public static string Escape(this string str) => str.Replace("\\", "\\\\").Replace("\'", "\\\'").Replace("\n", "\\n").Replace("\t", "\\t"); | ||
|
|
||
| [SuppressMessage("Performance", "CA1865:Use char overload", Justification = "Not available in net462")] | ||
| public static string QuoteSingle(this string str) => str.StartsWith("'", StringComparison.InvariantCulture) && str.EndsWith("'", StringComparison.InvariantCulture) ? str : $"'{str}'"; | ||
| public static string QuoteSingle(this string str) => $"'{str.Replace("'", "\\'")}'"; | ||
|
|
||
| [SuppressMessage("Performance", "CA1865:Use char overload", Justification = "Not available in net462")] | ||
| public static string QuoteDouble(this string str) => str.StartsWith("\"", StringComparison.InvariantCulture) && str.EndsWith("\"", StringComparison.InvariantCulture) ? str : $"\"{str}\""; | ||
| public static string QuoteDouble(this string str) => $"\"{str.Replace("\"", "\\\"")}\""; | ||
|
|
||
| /// <summary> | ||
| /// Appends a value prefixed with its length, so that a concatenation of appended values maps back to | ||
|
|
@@ -26,16 +26,14 @@ public static StringBuilder AppendLengthPrefixed(this StringBuilder builder, str | |
|
|
||
| /// <summary> | ||
| /// Encloses column name in backticks (`). Escapes ` symbol if met inside name | ||
| /// Does nothing if column is already enclosed | ||
| /// The input is always treated as an unquoted column name. | ||
|
Collaborator
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. ❌ Blocker: Document the breaking column-name contract This changes the public A reply here is read on the next review: an explanation that holds up drops the finding. |
||
| /// </summary> | ||
| /// <param name="str">Column name</param> | ||
| /// <returns>Backticked column name</returns> | ||
| public static string EncloseColumnName(this string str) | ||
| { | ||
| if (string.IsNullOrEmpty(str)) | ||
| return str; | ||
| if (str[0] == '`' && str[str.Length - 1] == '`') | ||
| return str; // Early return if already enclosed | ||
|
|
||
| var builder = new StringBuilder(); | ||
|
bjsneto marked this conversation as resolved.
Resolved
|
||
| builder.Append('`'); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| * Fixed `QuoteSingle`, `QuoteDouble` and `EncloseColumnName` failing to escape delimiters inside already-delimited input ([#316](https://github.com/ClickHouse/clickhouse-cs/issues/316)). |
Uh oh!
There was an error while loading. Please reload this page.