diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bdfe84d9f..595fc076ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - Fixed a bug where the destination passed to `DataFrameWriter.copy_into_location` (and `csv`/`json`/`parquet`/`save`) was embedded into the generated `COPY INTO` statement without quoting, which could produce malformed SQL for locations containing single quotes. The location is now consistently quoted and escaped, and a string that merely starts and ends with a single quote but contains unescaped interior quotes is no longer treated as an already-quoted literal; it is fully escaped so it stays a single SQL string literal. - Fixed a bug where UDF default argument values reconstructed from a source file in `register_from_file` were evaluated with `eval()`; they are now evaluated only against the documented set of supported default-value types, and unsupported expressions are ignored. - Fixed a bug where `object_name`, `object_domain`, or `object_version` values containing single quotes or backslashes in `session.lineage.trace()` caused incorrect SQL generation. These values are now properly escaped before being embedded in the `SYSTEM$DGQL` call. +- Fixed a bug where single quotes and backslashes in `comment` (`create_or_replace_view` / dynamic table / `save_as_table`), collation specs (`Column.collate`), VARIANT/OBJECT subfield keys (`Column[...]`), and `DataFrame`/`Session.flatten` paths were not correctly escaped when generating SQL, which could produce malformed statements. Backslash sequences (e.g. `\t`, `\n`) in these values are now applied literally rather than interpreted. - Fixed a bug where string values in the AI functions (`ai_extract`, `ai_classify`, `ai_similarity`, `ai_parse_document`, `ai_transcribe`, `ai_complete`) configuration and `response_format` were not correctly escaped when generating the SQL object literal, which could produce malformed statements when a value contained single quotes or backslashes (for example, an apostrophe in a natural-language question). #### Dependency Updates diff --git a/src/snowflake/snowpark/_internal/analyzer/analyzer_utils.py b/src/snowflake/snowpark/_internal/analyzer/analyzer_utils.py index b14eb09ff6..1769d0b006 100644 --- a/src/snowflake/snowpark/_internal/analyzer/analyzer_utils.py +++ b/src/snowflake/snowpark/_internal/analyzer/analyzer_utils.py @@ -41,7 +41,8 @@ EMPTY_STRING, TempObjectType, escape_quotes, - escape_single_quotes, + escape_quotes_and_backslashes, + escape_subfield_key, get_temp_type_for_object, is_single_quoted, is_sql_select_statement, @@ -403,7 +404,26 @@ def regexp_expression(expr: str, pattern: str, parameters: Optional[str] = None) def collate_expression(expr: str, collation_spec: str) -> str: - return expr + COLLATE + single_quote(collation_spec) + # Escape the collation spec so a value containing single quotes or + # backslashes does not terminate the single-quoted literal early and produce + # invalid SQL. + # + # Preserve the historical behavior of single_quote(): a spec that is already + # wrapped in single quotes (e.g. "'en_US'") is treated as pre-quoted, so we + # strip the outer quotes, escape the interior, and re-wrap. An unquoted spec + # is escaped and wrapped. Specs (quoted or unquoted) with no interior quotes + # or backslashes therefore produce identical SQL to before. + if is_single_quoted(collation_spec): + inner = collation_spec[1:-1] + else: + inner = collation_spec + return ( + expr + + COLLATE + + SINGLE_QUOTE + + escape_quotes_and_backslashes(inner) + + SINGLE_QUOTE + ) def subfield_expression(expr: str, field: Union[str, int]) -> str: @@ -411,7 +431,12 @@ def subfield_expression(expr: str, field: Union[str, int]) -> str: expr + LEFT_BRACKET + ( - SINGLE_QUOTE + field + SINGLE_QUOTE + # Escape the field so a VARIANT/OBJECT key containing single quotes + # or backslashes does not terminate the literal early. A key whose + # single quotes are already doubled is preserved unchanged (minus + # backslash normalization) to keep the historical "double your own + # quotes" contract non-breaking. + SINGLE_QUOTE + escape_subfield_key(field) + SINGLE_QUOTE if isinstance(field, str) else str(field) ) @@ -432,7 +457,10 @@ def flatten_expression( + PATH + RIGHT_ARROW + SINGLE_QUOTE - + (path or EMPTY_STRING) + # Escape the JSON path so a value containing single quotes or + # backslashes does not terminate the literal early and produce invalid + # SQL in the FLATTEN argument list. + + (escape_quotes_and_backslashes(path) if path else EMPTY_STRING) + SINGLE_QUOTE + COMMA + OUTER @@ -1148,7 +1176,12 @@ def join_statement( def get_comment_sql(comment: Optional[str]) -> str: return ( - COMMENT + EQUALS + SINGLE_QUOTE + escape_single_quotes(comment) + SINGLE_QUOTE + COMMENT + EQUALS + SINGLE_QUOTE + # Escape backslashes and single quotes so a comment containing those + # characters does not terminate the single-quoted literal early and + # produce invalid SQL. A comment with no backslash/quote is emitted + # unchanged. + + escape_quotes_and_backslashes(comment) + SINGLE_QUOTE if comment else EMPTY_STRING ) diff --git a/src/snowflake/snowpark/_internal/utils.py b/src/snowflake/snowpark/_internal/utils.py index 7927f5d37f..d04ee6a926 100644 --- a/src/snowflake/snowpark/_internal/utils.py +++ b/src/snowflake/snowpark/_internal/utils.py @@ -409,6 +409,58 @@ def escape_single_quotes(input_str): return input_str.replace("'", r"\'") +def escape_quotes_and_backslashes(input_str: str) -> str: + """Escape a value for embedding inside a single-quoted SQL string literal. + + Snowflake treats backslash as an escape character inside string literals, so + escaping only single quotes is not enough: a value such as ``\\'`` would + otherwise be emitted as ``\\\\'``, which decodes to a literal backslash + followed by a quote that terminates the literal early and produces invalid + SQL. We escape backslashes first (``\\`` -> ``\\\\``) and then single quotes + by doubling them (``'`` -> ``''``) so the value is always emitted as literal + data. + + A value that contains neither a backslash nor a single quote is returned + unchanged, so values without special characters produce identical SQL to + before. + """ + return input_str.replace("\\", "\\\\").replace("'", "''") + + +# Matches a value in which every single quote is already doubled (''); every +# other character -- including backslash -- is treated as ordinary. A value that +# fully matches already follows the historical "double your own single quotes" +# contract for VARIANT/OBJECT subfield keys, so its quotes must not be doubled +# again. +_ALL_SINGLE_QUOTES_DOUBLED = re.compile(r"(?:[^']|'')*") + + +def escape_subfield_key(field: str) -> str: + """Escape a VARIANT/OBJECT subfield key for a single-quoted SQL literal + without breaking the historical "double your own single quotes" contract. + + ``Column.__getitem__`` previously emitted the key verbatim between single + quotes, so callers were expected to double any single quote themselves (see + ``tests/integ/scala/test_column_suite.py::test_subfield``). To keep that + behavior working while still preventing an unescaped quote from terminating + the literal early: + + * If every single quote in ``field`` is already doubled, the key is assumed + to follow the historical contract; its quotes are left as-is and only + backslashes are escaped (so ``\\t``/``\\n`` are applied literally rather + than interpreted). This is byte-identical to the old output for such keys. + * Otherwise the key contains a lone single quote -- previously this produced + malformed SQL or allowed the literal to be terminated early -- and it is + fully escaped via :func:`escape_quotes_and_backslashes`. + + Either way the emitted literal is well-formed, so a caller-controlled key can + no longer break out of it. + """ + if _ALL_SINGLE_QUOTES_DOUBLED.fullmatch(field): + return field.replace("\\", "\\\\") + return escape_quotes_and_backslashes(field) + + def is_sql_select_statement(sql: str) -> bool: return ( SNOWFLAKE_SELECT_SQL_PREFIX_PATTERN.match(sql) is not None diff --git a/tests/integ/scala/test_column_suite.py b/tests/integ/scala/test_column_suite.py index 18626713d1..595fad8c0e 100644 --- a/tests/integ/scala/test_column_suite.py +++ b/tests/integ/scala/test_column_suite.py @@ -692,6 +692,8 @@ def test_regexp(session): ) @pytest.mark.parametrize("spec", ["en_US-trim", "'en_US-trim'"]) def test_collate(session, spec): + # Both the unquoted spec and the pre-quoted spec must produce the same + # result; the pre-quoted form is passed through unchanged. Utils.check_answer( TestData.string3(session).where(col("a").collate(spec) == "abcba"), [Row(" abcba ")], diff --git a/tests/integ/test_sql_literal_escaping.py b/tests/integ/test_sql_literal_escaping.py new file mode 100644 index 0000000000..22c69451be --- /dev/null +++ b/tests/integ/test_sql_literal_escaping.py @@ -0,0 +1,166 @@ +# +# Copyright (c) 2012-2025 Snowflake Computing Inc. All rights reserved. +# +"""End-to-end regression tests for escaping special characters in SQL literals. + +These cover four sinks that build single-quoted SQL string literals from +user-provided values: + + * DataFrame.create_or_replace_view(..., comment=...) -> get_comment_sql + * Column.collate(spec) -> collate_expression + * Column.__getitem__(field) (VARIANT/OBJECT key) -> subfield_expression + * DataFrame.flatten(..., path=...) -> flatten_expression + +Each test verifies BOTH: + 1. values containing apostrophes / backslashes work end-to-end and return the + correct result (escaping must not break valid input), and + 2. values containing other special characters (single quotes, backslashes, + parentheses, commas, trailing dashes) are escaped into the literal so they + are treated as literal data and produce valid SQL. +""" + +import pytest + +from snowflake.snowpark import Row +from snowflake.snowpark.functions import col, lit, parse_json +from snowflake.snowpark.exceptions import SnowparkSQLException +from tests.utils import TestData, Utils + +pytestmark = [ + pytest.mark.skipif( + "config.getoption('local_testing_mode', default=False)", + reason="SQL generation / live execution required for these escaping tests", + ), +] + + +def test_comment_escapes_special_characters(session): + # 1. A comment with apostrophes and a backslash round-trips and the view is + # created with the expected body. + view_name = Utils.random_view_name() + comment = "O'Brien's data \\ backup" + df = session.create_dataframe([[1]], schema=["a"]) + try: + df.create_or_replace_view(view_name, comment=comment) + # The view exists and returns the expected body. + assert session.table(view_name).collect() == [Row(1)] + got = session.sql(f"SHOW VIEWS LIKE '{view_name}'").collect() + assert got[0]["comment"] == comment + finally: + Utils.drop_view(session, view_name) + + # 2. A comment whose special characters (backslash, quote, parentheses, + # commas, trailing dashes) could otherwise terminate the literal early is + # escaped, so the view is created and the comment round-trips verbatim. + view_name2 = Utils.random_view_name() + comment2 = "O'Brien\\notes (draft), v2 -- " + try: + df.create_or_replace_view(view_name2, comment=comment2) + # The body is unchanged. + assert session.table(view_name2).collect() == [Row(1)] + shown = session.sql(f"SHOW VIEWS LIKE '{view_name2}'").collect() + # The whole comment is stored verbatim as literal data. + assert shown[0]["comment"] == comment2 + finally: + Utils.drop_view(session, view_name2) + + +def test_comment_round_trip_special_characters(session): + # Lock the contract that the stored comment equals the exact Python input, + # including literal backslashes (e.g. C:\table, \t, \n stay as written and + # are not interpreted as escape sequences). + df = session.create_dataframe([[1]], schema=["a"]) + values = [ + "O'Brien", + "C:\\table", # Python value = C:\table + "a\\'b", # Python value = a\'b + "\\\\", # Python value = two backslashes + "plain comment", + ] + for comment in values: + view_name = Utils.random_view_name() + try: + df.create_or_replace_view(view_name, comment=comment) + shown = session.sql(f"SHOW VIEWS LIKE '{view_name}'").collect() + assert ( + shown[0]["comment"] == comment + ), f"stored comment {shown[0]['comment']!r} != input {comment!r}" + finally: + Utils.drop_view(session, view_name) + + +@pytest.mark.parametrize("spec", ["en_US-trim", "'en_US-trim'"]) +def test_collate_valid_specs(session, spec): + # Valid collation specs (unquoted AND pre-quoted) must work end-to-end + # exactly as before -- no behavior change for inputs without special + # characters. + Utils.check_answer( + TestData.string3(session).where(col("a").collate(spec) == "abcba"), + [Row(" abcba ")], + ) + + +def test_collate_escapes_special_characters(session): + # A collation spec containing single quotes and parentheses is escaped into + # the COLLATE literal as a single string. The resulting value is not a valid + # collation specification, so Snowflake rejects it -- the characters are + # treated as literal data, not as separate SQL tokens. + spec = "en') = 'x' (note) --" + df = TestData.string3(session).where(col("a").collate(spec) == "abcba") + with pytest.raises(SnowparkSQLException): + df.collect() + + # The same holds when the spec is used in a projection. + spec2 = "en' (a), (b) --" + with pytest.raises(SnowparkSQLException): + TestData.string3(session).select(col("a").collate(spec2)).collect() + + +def test_subfield_escapes_special_characters(session): + # Build a VARIANT/OBJECT with keys that contain an apostrophe and backslash. + df = session.create_dataframe([[1]], schema=["x"]).select( + parse_json(lit('{"O\'Brien": "ok", "a\\\\b": "ok2", "plain": "p"}')).alias("v") + ) + + # 1. Keys with special characters extract the correct values. + assert df.select(col("v")["O'Brien"].alias("r")).collect()[0]["R"] == '"ok"' + assert df.select(col("v")["a\\b"].alias("r")).collect()[0]["R"] == '"ok2"' + + # 2. A key containing quotes, parentheses, commas and trailing dashes is + # escaped into the bracket literal as a single string. It is simply a + # key that does not exist in the document, so the lookup yields NULL and + # no extra output columns are produced. + key = "plain' , (x) AS y -- " + res = df.select(col("v")[key].alias("r")).collect() + assert len(res) == 1 + assert res[0]["R"] is None + assert list(res[0].asDict().keys()) == ["R"] + + +def test_flatten_escapes_special_characters(session): + df = session.create_dataframe([[1]], schema=["x"]).select( + parse_json(lit('{"arr": [10, 20]}')).alias("v") + ) + + # 1. A valid JSON path flattens correctly. + valid = df.flatten( + col("v"), path="arr", outer=False, recursive=False, mode="ARRAY" + ).select("value") + Utils.check_answer(valid, [Row("10"), Row("20")]) + + # 2. A path containing single quotes, parentheses, commas and trailing + # dashes is escaped into the PATH literal as a single string. It is + # therefore an invalid / non-existent compound field path: with + # OUTER => TRUE Snowflake either errors on the bad path name or yields a + # single NULL-value row -- the characters are treated as literal data. + path = "' (a), (b) -- " + df2 = session.create_dataframe([[1]], schema=["x"]).select( + parse_json(lit('{"a": [1]}')).alias("v") + ) + flat = df2.flatten(col("v"), path=path, outer=True, recursive=False, mode="BOTH") + try: + res = flat.select("value").collect() + except SnowparkSQLException: + # Bad compound object's field path name -> path treated as literal data. + return + assert all(r["VALUE"] is None for r in res) diff --git a/tests/unit/test_analyzer_util_suite.py b/tests/unit/test_analyzer_util_suite.py index 08f293964c..98f042cfdc 100644 --- a/tests/unit/test_analyzer_util_suite.py +++ b/tests/unit/test_analyzer_util_suite.py @@ -990,3 +990,186 @@ def test_get_options_statement(): ) == " INCLUDE_METADATA = (col1 = METADATA$FILENAME, col2 = METADATA$FILE_ROW_NUMBER) " ) + + +# --------------------------------------------------------------------------- +# Regression tests for escaping special characters in SQL string literals. +# +# These cover four sinks that build single-quoted SQL string literals from +# user-provided values: +# * get_comment_sql (COMMENT clauses) +# * collate_expression (Column.collate) +# * subfield_expression (Column.__getitem__) +# * flatten_expression (DataFrame/Session.flatten PATH) +# All four route through the dedicated helper escape_quotes_and_backslashes, +# which escapes backslashes first (`\` -> `\\`) then single quotes (`'` -> `''`) +# so a value containing those characters does not terminate the literal early. +# Verified against live Snowflake: values round-trip exactly and produce valid +# SQL. +# +# The shared escape_single_quotes helper is intentionally LEFT UNCHANGED +# (quote-only) so that unrelated callers (e.g. the udf COMMENT ON path) are +# unaffected. +# --------------------------------------------------------------------------- + + +def test_escape_single_quotes_is_quote_only_and_unchanged(): + # This helper must remain quote-only so that unrelated call sites are not + # affected. Backslashes are deliberately NOT escaped here. + from snowflake.snowpark._internal.utils import escape_single_quotes + + assert escape_single_quotes("O'Brien") == r"O\'Brien" + # backslash left untouched + assert escape_single_quotes("a\\b") == "a\\b" + assert escape_single_quotes("\\'") == r"\\'" + + +def test_escape_quotes_and_backslashes(): + from snowflake.snowpark._internal.utils import escape_quotes_and_backslashes + + # single quotes are doubled + assert escape_quotes_and_backslashes("O'Brien") == "O''Brien" + # lone backslash must be doubled so it cannot escape a following quote + assert escape_quotes_and_backslashes("a\\b") == r"a\\b" + # value with neither quote nor backslash is returned byte-identical + assert escape_quotes_and_backslashes("plain text 123") == "plain text 123" + # a backslash immediately followed by a quote previously became `\\'` + # (decoding to a literal backslash followed by a quote that ends the literal + # early); it must now become `\\''` (escaped backslash + doubled quote). + assert escape_quotes_and_backslashes("\\'") == r"\\" + "''" + # a value mixing both characters is fully escaped + value = "a'b\\c (draft), v2 --" + escaped = escape_quotes_and_backslashes(value) + assert escaped == "a''b" + "\\\\" + "c (draft), v2 --" + + +def test_get_comment_sql_no_special_characters_unchanged(): + # A comment with no quote/backslash must produce byte-identical SQL to the + # previous behavior. + from snowflake.snowpark._internal.analyzer.analyzer_utils import get_comment_sql + + assert ( + get_comment_sql("A frosty winter wonderland with glistening snowflakes") + == " COMMENT = 'A frosty winter wonderland with glistening snowflakes'" + ) + # empty / None comment yields nothing + assert get_comment_sql(None) == "" + assert get_comment_sql("") == "" + + +def test_get_comment_sql_escapes_special_characters(): + from snowflake.snowpark._internal.analyzer.analyzer_utils import get_comment_sql + + # apostrophes are escaped by doubling + assert get_comment_sql("O'Brien's data") == " COMMENT = 'O''Brien''s data'" + # a comment containing a backslash followed by a quote is escaped so it + # stays inside the literal + sql = get_comment_sql("O'Brien\\notes (draft), v2 --") + assert sql == " COMMENT = 'O''Brien" + "\\\\" + "notes (draft), v2 --'" + + +def test_collate_expression_no_special_characters_unchanged(): + # Collation specs (quoted OR unquoted, no interior special chars) must + # produce byte-identical SQL to the previous single_quote() behavior. + from snowflake.snowpark._internal.analyzer.analyzer_utils import collate_expression + + assert collate_expression('"NAME"', "en_US") == "\"NAME\" COLLATE 'en_US'" + # pre-quoted spec is still passed through (outer quotes stripped, re-wrapped) + assert collate_expression('"NAME"', "'en_US'") == "\"NAME\" COLLATE 'en_US'" + assert collate_expression('"NAME"', "en_US-ci") == "\"NAME\" COLLATE 'en_US-ci'" + assert collate_expression('"NAME"', "en_US-trim") == "\"NAME\" COLLATE 'en_US-trim'" + + +def test_collate_expression_escapes_special_characters(): + from snowflake.snowpark._internal.analyzer.analyzer_utils import collate_expression + + # A pre-quoted spec whose interior contains a quote: the outer quotes are + # stripped but the interior quote is escaped, so the value stays inside the + # literal. + spec = "'en'); notes --" + out = collate_expression('"NAME"', spec) + assert out.startswith('"NAME" COLLATE \'') + assert out.endswith("'") + # every interior single quote is escaped (doubled) + inner = out[len('"NAME" COLLATE \'') : -1] + assert "'" not in inner.replace("''", "") + # a spec with quotes and parentheses is escaped + spec2 = "en') = 'x' (note) --" + out2 = collate_expression('"NAME"', spec2) + assert out2 == "\"NAME\" COLLATE 'en'') = ''x'' (note) --'" + + +def test_subfield_expression_no_special_characters_unchanged(): + # A key with no quote/backslash must produce byte-identical SQL to before. + from snowflake.snowpark._internal.analyzer.analyzer_utils import subfield_expression + + assert subfield_expression("col", "plainkey") == "col['plainkey']" + # integer subfields are unaffected + assert subfield_expression("col", 3) == "col[3]" + + +def test_subfield_expression_escapes_special_characters(): + from snowflake.snowpark._internal.analyzer.analyzer_utils import subfield_expression + + # key with an apostrophe is escaped (doubled) + assert subfield_expression("col", "O'Brien") == "col['O''Brien']" + # key mixing quotes, backslashes, parentheses, commas and trailing dashes is + # escaped so it stays inside the bracket literal + key = "a'b\\c (x), y --" + out = subfield_expression("col", key) + assert out.startswith("col['") + assert out.endswith("']") + inner = out[len("col['") : -len("']")] + assert "'" not in inner.replace("\\\\", "").replace("''", "") + + +def test_escape_subfield_key_preserves_already_doubled_quotes(): + # Non-breaking contract: a key whose single quotes are already doubled (the + # historically-documented way to pass a quote in a subfield key) is preserved + # unchanged, so it is not double-escaped. + from snowflake.snowpark._internal.utils import escape_subfield_key + + assert escape_subfield_key("date with '' and .") == "date with '' and ." + assert escape_subfield_key("plainkey") == "plainkey" + # a lone (unescaped) quote is fully escaped instead + assert escape_subfield_key("date with ' and .") == "date with '' and ." + # backslashes are always escaped so they are applied literally, even when the + # quotes are already doubled + assert escape_subfield_key("a\\b") == r"a\\b" + assert escape_subfield_key("o''clock\\t") == r"o''clock\\t" + + +def test_subfield_expression_pre_doubled_quote_is_non_breaking(): + # Regression for the historical contract exercised by + # tests/integ/scala/test_column_suite.py::test_subfield: a caller that + # already doubled the quote must get byte-identical SQL to before this fix. + from snowflake.snowpark._internal.analyzer.analyzer_utils import subfield_expression + + assert ( + subfield_expression("col", "date with '' and .") == "col['date with '' and .']" + ) + # a raw (single) apostrophe now also works and lands on the same SQL literal + assert ( + subfield_expression("col", "date with ' and .") == "col['date with '' and .']" + ) + + +def test_flatten_expression_no_special_characters_unchanged(): + # A path with no quote/backslash must produce byte-identical SQL to before. + from snowflake.snowpark._internal.analyzer.analyzer_utils import flatten_expression + + out = flatten_expression("col", "request.headers", False, False, "BOTH") + assert " PATH => 'request.headers'," in out + # None / empty path -> empty literal + out_none = flatten_expression("col", None, False, False, "BOTH") + assert " PATH => ''," in out_none + + +def test_flatten_expression_escapes_special_characters(): + from snowflake.snowpark._internal.analyzer.analyzer_utils import flatten_expression + + # a path containing quotes/parentheses/commas/dashes is escaped so it stays + # inside the PATH literal + path = "a'b (x), y --" + out = flatten_expression("col", path, False, False, "BOTH") + assert " PATH => 'a''b (x), y --'," in out