fix(django-google-spanner): escape tzname in datetime sql helpers#17559
fix(django-google-spanner): escape tzname in datetime sql helpers#17559saddamr3e wants to merge 2 commits into
Conversation
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.
|
The cover failure was the old 100% coverage gate, which #17607 has since relaxed to 80 for this package. Pushed a new head so CI reruns against current main; the workflow runs just need approval. |
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.