Skip to content

fix(spanner): escape embedded backticks in dbapi escape_name#17810

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.

Sign up for GitHub

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

Merged
sakthivelmanii merged 1 commit into from
Jul 27, 2026
+43 −4
Merged

fix(spanner): escape embedded backticks in dbapi escape_name #17810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions packages/google-cloud-spanner/google/cloud/spanner_dbapi/parse_utils.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -382,17 +382,38 @@ def ensure_where_clause(sql):
return sql + " WHERE 1=1"


_QUOTED_IDENTIFIER_RE = re.compile(r"\A`(\\.|[^`\\])*`\Z", re.DOTALL)


def escape_name(name):
"""
Apply backticks to the name that either contain '-' or
' ', or is a Cloud Spanner's reserved keyword.
Apply backticks to the name if it is not a valid regular ASCII identifier,

@olavloiteolavloiteJul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more corner cases that need to be thought through here:

  1. Backslash (see also below): Backslash starts an escape sequence in GoogleSQL, unless itself is escaped. Should we escape backslashes as well to be consistent with how we handle existing backticks in identifiers? E.g. what should we do with a string like this: "test\"
  2. Identifiers with multiple elements (dot operator): The identifier my_schema.my_table is now turned into this: `my_schema.my_table`.
  3. Already quoted identifiers: `my_table` becomes ``my_table``

@sakthivelmaniisakthivelmaniiJul 27, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

or if it is a Cloud Spanner's reserved keyword.

:type name: str
:param name: Name to escape.

:rtype: str
:returns: Name escaped if it has to be escaped.
"""
if "-" in name or " " in name or name.upper() in SPANNER_RESERVED_KEYWORDS:
return "`" + name + "`"
if not name:
return name

if _QUOTED_IDENTIFIER_RE.match(name):
return name

if "." in name:

@olavloiteolavloiteJul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check should be done after the check whether the identifier is already quoted. Otherwise, a dot inside a validly quoted identifier would break up that identifier into multiple (invalid) parts.

Also, the current loop below actually allows similar SQL injection attacks that this is intended to fix. The following string would be seen as a validly quoted identifier:

# String literal with 1 literal backslash before the internal backtick:payload="`col"+"\\"+"`; DROP TABLE users; --`"# Represented in Python string repr as: '`col\\`; DROP TABLE users; --`'

This results in this SQL:

`col\`; DROP TABLE users; --`

Could we instead use a regex to prevent this type of SQL injection attack, and also simplify the implementation a bit:

importre# re.DOTALL ensures '.' in '\\.' matches newlines inside valid multiline backtick identifiers.# \A and \Z enforce strict start and end anchors._QUOTED_IDENTIFIER_RE=re.compile(r"\A`(\\.|[^`\\])*`\Z", re.DOTALL)
defescape_name(name):
ifnotname:
returnname# 1. Check if the string is ALREADY a valid GoogleSQL quoted identifierif_QUOTED_IDENTIFIER_RE.match(name):
returnname# 2. Handle qualified identifiers (e.g., schema.table or `schema`.table)if"."inname:
parts=name.split(".")
return".".join(escape_name(part) forpartinparts)
# 3. Check if it is a regular ASCII identifier and not a reserved keywordis_valid_regular_identifier= (
(name[0].isalpha() orname[0] =="_")
andall(c.isalnum() orc=="_"forcinname)
andname.isascii()
)
ifnotis_valid_regular_identifierorname.upper() inSPANNER_RESERVED_KEYWORDS:
escaped=name.replace("\\", "\\\\").replace("`", "\\`")
returnf"`{escaped}`"returnname

@sakthivelmaniisakthivelmaniiJul 27, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

parts = name.split(".")
return ".".join(escape_name(part) for part in parts)

is_valid_regular_identifier = (
(name[0].isalpha() or name[0] == "_")
and all(c.isalnum() or c == "_" for c in name)
and name.isascii()
)

if not is_valid_regular_identifier or name.upper() in SPANNER_RESERVED_KEYWORDS:
escaped = name.replace("\\", "\\\\").replace("`", "\\`")
return f"`{escaped}`"

return name
18 changes: 18 additions & 0 deletions packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_parse_utils.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -403,6 +403,24 @@ def test_escape_name(self):
("with space", "`with space`"),
("name", "name"),
("", ""),
("col`; DROP TABLE t; -- x", "`col\\`; DROP TABLE t; -- x`"),
("table`name", "`table\\`name`"),
("`", "`\\``"),
("col/*comment*/name", "`col/*comment*/name`"),
("123column", "`123column`"),
("col;select", "`col;select`"),
("col\nname", "`col\nname`"),
("test\\", "`test\\\\`"),
("my_schema.my_table", "my_schema.my_table"),
("my-schema.my-table", "`my-schema`.`my-table`"),
("`my_table`", "`my_table`"),
("`my_schema`.`my_table`", "`my_schema`.`my_table`"),
("`table.with.dots`", "`table.with.dots`"),
("`col\\`; DROP TABLE users; --`", "`col\\`; DROP TABLE users; --`"),
(
"`col\\\\`; DROP TABLE users; --`",
"`\\`col\\\\\\\\\\`; DROP TABLE users; --\\``",
),
)
for name, want in cases:
with self.subTest(name=name):
Expand Down
Loading