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

Open
sakthivelmanii wants to merge 1 commit into
base: main
Choose a base branch
from
+46 −4
Open

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
38 changes: 34 additions & 4 deletions packages/google-cloud-spanner/google/cloud/spanner_dbapi/parse_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,15 +384,45 @@ def ensure_where_clause(sql):

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,

@olavloite olavloite Jul 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``

@sakthivelmanii sakthivelmanii Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

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 "." in name:

@olavloite olavloite Jul 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:

import re

# 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)

def escape_name(name):
    if not name:
        return name

    # 1. Check if the string is ALREADY a valid GoogleSQL quoted identifier
    if _QUOTED_IDENTIFIER_RE.match(name):
        return name

    # 2. Handle qualified identifiers (e.g., schema.table or `schema`.table)
    if "." in name:
        parts = name.split(".")
        return ".".join(escape_name(part) for part in parts)

    # 3. Check if it is a regular ASCII identifier and not a reserved keyword
    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

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

if len(name) >= 2 and name.startswith("`") and name.endswith("`"):
inner = name[1:-1]
i = 0
is_properly_quoted = True
while i < len(inner):
if inner[i] == "\\":
i += 2
elif inner[i] == "`":
is_properly_quoted = False
break
else:
i += 1
if is_properly_quoted:
return name

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
12 changes: 12 additions & 0 deletions packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_parse_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,18 @@ 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`"),
)
for name, want in cases:
with self.subTest(name=name):
Expand Down
Loading