fix(django-google-spanner): escape tzname in datetime sql helpers#17559
Open
saddamr3e wants to merge 1 commit into
Open
fix(django-google-spanner): escape tzname in datetime sql helpers#17559saddamr3e wants to merge 1 commit into
saddamr3e wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before the fix, the time zone name lands inside the quoted string literal that the datetime helpers build:
datetime_extract_sql,datetime_trunc_sql,time_trunc_sql,datetime_cast_date_sqlanddatetime_cast_time_sqlinlinetznameinto a quoted literal (... "%s", and'%s'for theFORMAT_TIMESTAMPone) 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 aTrunc/Extractexpression, 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_defaultpaths inschema.pyalready use, so the name stays inside its literal and an invalid zone is rejected at execution instead of running as SQL.