Skip to content

fix(django-google-spanner): escape tzname in datetime sql helpers#17559

Open
saddamr3e wants to merge 1 commit into
googleapis:mainfrom
saddamr3e:spanner-escape-tzname
Open

fix(django-google-spanner): escape tzname in datetime sql helpers#17559
saddamr3e wants to merge 1 commit into
googleapis:mainfrom
saddamr3e:spanner-escape-tzname

Conversation

@saddamr3e

Copy link
Copy Markdown
Contributor

Before the fix, the time zone name lands inside the quoted string literal that the datetime helpers build:

>>> ops.datetime_extract_sql("year", "col", None, 'UTC" AS x, (SELECT secret) AS y FROM t -- ')[0]
EXTRACT(year FROM col AT TIME ZONE "UTC" AS x, (SELECT secret) AS y FROM t -- ")

datetime_extract_sql, datetime_trunc_sql, time_trunc_sql, datetime_cast_date_sql and datetime_cast_time_sql inline tzname into a quoted literal (... "%s", and '%s' for the FORMAT_TIMESTAMP one) without escaping, so a tzname carrying a quote closes the literal and the remainder runs as SQL. It is reachable when an explicit tzinfo is passed to a Trunc/Extract expression, e.g. datetime.timezone(offset, name='UTC" ...'), whose name Django forwards verbatim. Django core binds tzname as a query parameter for this reason; this backend inlines it, so it needs to escape it.

The fix escapes the backslash and both quote characters before interpolation, the same GoogleSQL escaping the generated-column and db_default paths in schema.py already use, so the name stays inside its literal and an invalid zone is rejected at execution instead of running as SQL.

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

@saddamr3e saddamr3e requested a review from a team as a code owner June 24, 2026 05:38

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces a helper function _escape_tzname to escape backslashes, single quotes, and double quotes in time zone names, preventing potential SQL injection vulnerabilities in Cloud Spanner string literals. This escaping logic is integrated into several SQL generation methods in DatabaseOperations, including datetime_extract_sql, datetime_trunc_sql, time_trunc_sql, datetime_cast_date_sql, and datetime_cast_time_sql. Corresponding unit tests have been added to verify the escaping behavior across these methods. I have no feedback to provide as the changes are well-implemented and covered by tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant