From 25e066e5bec911eb252da50340c0c130de0d3594 Mon Sep 17 00:00:00 2001 From: siddus Date: Mon, 4 May 2026 15:43:57 -0400 Subject: [PATCH 1/5] Fixed #24638 -- Added QuerySet.comment() to inject SQL comments. QuerySet.comment(message) emits an /* ... */ comment in the resulting SELECT or UPDATE statement: after the leading SELECT (and DISTINCT, if present), after UPDATE, or as a leading block before a UNION/INTERSECT/EXCEPT combinator. comment() raises ValueError if the message contains '/*' or '*/' to prevent SQL comment injection, and TypeError if the message is not a string. --- AUTHORS | 1 + django/db/models/query.py | 16 ++++ django/db/models/sql/compiler.py | 26 +++++-- django/db/models/sql/query.py | 2 + docs/ref/models/querysets.txt | 41 ++++++++++ docs/releases/6.2.txt | 3 +- tests/basic/tests.py | 1 + tests/queries/tests.py | 128 +++++++++++++++++++++++++++++++ 8 files changed, 212 insertions(+), 6 deletions(-) diff --git a/AUTHORS b/AUTHORS index a9403842b54e..8562696ecfb2 100644 --- a/AUTHORS +++ b/AUTHORS @@ -980,6 +980,7 @@ answer newbie questions, and generally made Django that much better: Shannon -jj Behrens Shawn Milochik Shreya Bamne + Sid Silvan Spross Simeon Visser Simon Blanchard diff --git a/django/db/models/query.py b/django/db/models/query.py index a2068044691e..ad30bca45b0f 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -1705,6 +1705,9 @@ def _combinator_query(self, combinator, *other_qs, all=False): clone.query.default_ordering = True self._clear_ordering_in_combined_queries(clone.query, other_qs) clone.query.clear_limits() + # Comments belong to the combined leg (`self.query`). Clear them on + # the outer combinator wrapper so they aren't emitted twice. + clone.query.comments = () clone.query.combinator = combinator clone.query.combinator_all = all return clone @@ -1980,6 +1983,19 @@ def fetch_mode(self, fetch_mode): clone._fetch_mode = fetch_mode return clone + def comment(self, message): + """Add an SQL comment to the query.""" + if not isinstance(message, str): + raise TypeError("QuerySet.comment() argument must be a string.") + if "/*" in message or "*/" in message: + raise ValueError( + "QuerySet.comment() cannot include '/*' or '*/'; strip or " + "escape these delimiters before calling comment()." + ) + clone = self._chain() + clone.query.comments = (*clone.query.comments, message) + return clone + ################################### # PUBLIC INTROSPECTION ATTRIBUTES # ################################### diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index bcf28f9ae16d..04bab0964579 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -625,7 +625,10 @@ def get_combinator_sql(self, combinator, all): sql_parts, args_parts = zip( *((braces.format(sql), args) for sql, args in parts) ) - result = [" {} ".format(combinator_sql).join(sql_parts)] + combined_sql = " {} ".format(combinator_sql).join(sql_parts) + if self.query.comments: + combined_sql = "%s %s" % (self.comments_sql(), combined_sql) + result = [combined_sql] params = [] for part in args_parts: params.extend(part) @@ -820,6 +823,9 @@ def as_sql(self, with_limits=True, with_col_aliases=False): result += distinct_result params += distinct_params + if self.query.comments: + result.append(self.comments_sql()) + out_cols = [] for _, (s_sql, s_params), alias in self.select + extra_select: if alias: @@ -1678,6 +1684,16 @@ def explain_query(self): else: yield value + def comments_sql(self): + """ + Return SQL comments for ``Query.comments`` as a single space-separated + string of ``/* ... */`` blocks. + + Validation in ``QuerySet.comment()`` already rejects strings containing + ``/*`` or ``*/``, so the values here cannot break out of the comment. + """ + return " ".join("/* %s */" % comment for comment in self.query.comments) + class SQLInsertCompiler(SQLCompiler): returning_fields = None @@ -2089,10 +2105,10 @@ def as_sql(self): values.append(f"{quoted_name} = %s") update_params.append(val) table = self.query.base_table - result = [ - "UPDATE %s SET" % qn(table), - ", ".join(values), - ] + result = ["UPDATE"] + if self.query.comments: + result.append(self.comments_sql()) + result += [qn(table), "SET", ", ".join(values)] try: where, params = self.compile(self.query.where) except FullResultSet: diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 22dd479d67d9..403129b098e3 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -302,6 +302,8 @@ class Query(BaseExpression): explain_info = None + comments = () + def __init__(self, model, alias_cols=True): self.model = model self.alias_refcount = {} diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index 93bddf90b050..ed96050dfa3c 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -1927,6 +1927,47 @@ For example: # queries the database with the 'backup' alias >>> Entry.objects.using("backup") +``comment()`` +~~~~~~~~~~~~~ + +.. versionadded:: 6.2 + +.. method:: comment(message) + +Returns a new ``QuerySet`` that injects ``message`` into the resulting SQL as +an ``/* … */`` comment, after ``SELECT`` (and ``DISTINCT`` if present) or +after ``UPDATE``. + +For example:: + + Entry.objects.comment("hello").all() + +… roughly produces: + +.. code-block:: sql + + SELECT /* hello */ "blog_entry"."id", … FROM "blog_entry" + +Calls can be chained; each call appends a separate block:: + + Entry.objects.comment("first").comment("second").update(headline="x") + +… produces: + +.. code-block:: sql + + UPDATE /* first */ /* second */ "blog_entry" SET "headline" = … + +``comment()`` raises :exc:`ValueError` if ``message`` contains ``/*`` or +``*/`` to prevent SQL comment injection, and :exc:`TypeError` if ``message`` +is not a string. + +``comment()`` only affects ``SELECT`` and ``UPDATE`` statements; ``DELETE``, +``INSERT``, and statements issued by migrations are not annotated. When +called on a combined ``QuerySet`` (produced by :meth:`union`, +:meth:`intersection`, or :meth:`difference`), the comment is emitted as a +leading ``/* … */`` block before the combined SQL. + ``select_for_update()`` ~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/releases/6.2.txt b/docs/releases/6.2.txt index ba9396313d71..772de574d91d 100644 --- a/docs/releases/6.2.txt +++ b/docs/releases/6.2.txt @@ -183,7 +183,8 @@ Migrations Models ~~~~~~ -* ... +* The new :meth:`.QuerySet.comment` method allows injecting an SQL comment + into the generated ``SELECT`` or ``UPDATE`` statement. Requests and Responses ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/basic/tests.py b/tests/basic/tests.py index ed655833e271..b833c178f128 100644 --- a/tests/basic/tests.py +++ b/tests/basic/tests.py @@ -815,6 +815,7 @@ class ManagerTest(SimpleTestCase): "aupdate", "aupdate_or_create", "fetch_mode", + "comment", ] def test_manager_methods(self): diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 169ca4924af4..8b567b004c7e 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -4641,6 +4641,134 @@ def test_ticket_23622(self): self.assertSequenceEqual(Ticket23605A.objects.filter(qx), [a2]) +class QuerySetCommentTests(TestCase): + """Tests for QuerySet.comment() (Trac #24638).""" + + def test_select_single_comment(self): + with CaptureQueriesContext(connection) as ctx: + list(NamedCategory.objects.comment("blog/views.py:50")) + self.assertIn("SELECT /* blog/views.py:50 */ ", ctx.captured_queries[0]["sql"]) + + def test_select_multiple_comments_preserve_order(self): + with CaptureQueriesContext(connection) as ctx: + list( + NamedCategory.objects.comment("first") + .comment("second") + .comment("third") + ) + self.assertIn( + "SELECT /* first */ /* second */ /* third */ ", + ctx.captured_queries[0]["sql"], + ) + + def test_select_empty_comment_allowed(self): + with CaptureQueriesContext(connection) as ctx: + list(NamedCategory.objects.comment("").comment("non-empty")) + self.assertIn("SELECT /* */ /* non-empty */ ", ctx.captured_queries[0]["sql"]) + + def test_select_with_distinct(self): + with CaptureQueriesContext(connection) as ctx: + list(NamedCategory.objects.distinct().comment("after distinct")) + sql = ctx.captured_queries[0]["sql"] + self.assertIn("DISTINCT", sql) + self.assertIn("/* after distinct */", sql) + # The comment must come after DISTINCT. + self.assertLess(sql.index("DISTINCT"), sql.index("/* after distinct */")) + + def test_select_with_filter_preserves_comment(self): + with CaptureQueriesContext(connection) as ctx: + list(NamedCategory.objects.comment("traceparent=00-x").filter(name="x")) + self.assertIn("/* traceparent=00-x */", ctx.captured_queries[0]["sql"]) + + def test_select_subquery(self): + with CaptureQueriesContext(connection) as ctx: + list( + NamedCategory.objects.annotate( + foo=( + Tag.objects.filter(category=OuterRef("id")) + .comment("inner subquery") + .values("name")[:1] + ) + ).comment("outer query") + ) + sql = ctx.captured_queries[0]["sql"] + self.assertIn("/* inner subquery */", sql) + self.assertIn("/* outer query */", sql) + + def test_aggregate_subquery(self): + with CaptureQueriesContext(connection) as ctx: + Tag.objects.comment("tag-aggregate").values("parent").annotate( + tag_per_parent=Count("pk") + ).aggregate(Max("tag_per_parent")) + sql = ctx.captured_queries[0]["sql"] + self.assertIn("/* tag-aggregate */", sql) + + def test_update(self): + NamedCategory.objects.create(name="initial") + with CaptureQueriesContext(connection) as ctx: + NamedCategory.objects.comment("update from worker-3").update(name="renamed") + update_sql = next( + q["sql"] for q in ctx.captured_queries if q["sql"].startswith("UPDATE") + ) + self.assertIn("UPDATE /* update from worker-3 */ ", update_sql) + + def test_clone_preserves_comment(self): + base = NamedCategory.objects.comment("shared") + derived = base.filter(name="x").comment("extra") + with CaptureQueriesContext(connection) as ctx: + list(base) + list(derived) + self.assertIn("/* shared */", ctx.captured_queries[0]["sql"]) + self.assertNotIn("/* extra */", ctx.captured_queries[0]["sql"]) + derived_sql = ctx.captured_queries[1]["sql"] + self.assertIn("/* shared */", derived_sql) + self.assertIn("/* extra */", derived_sql) + + def test_unicode_comment(self): + with CaptureQueriesContext(connection) as ctx: + list(NamedCategory.objects.comment("источник=cron")) + self.assertIn("/* источник=cron */", ctx.captured_queries[0]["sql"]) + + def test_comment_inside_union_leg(self): + qs = NamedCategory.objects.comment("first leg").union( + NamedCategory.objects.all() + ) + with CaptureQueriesContext(connection) as ctx: + list(qs) + sql = ctx.captured_queries[0]["sql"] + self.assertEqual(sql.count("/* first leg */"), 1) + + def test_comment_around_union(self): + qs = NamedCategory.objects.union(NamedCategory.objects.all()).comment( + "outer combinator" + ) + with CaptureQueriesContext(connection) as ctx: + list(qs) + sql = ctx.captured_queries[0]["sql"] + self.assertIn("/* outer combinator */", sql) + self.assertLess(sql.index("/* outer combinator */"), sql.index("UNION")) + + def test_rejects_non_string(self): + msg = "QuerySet.comment() argument must be a string." + for bad in [None, 42, b"bytes", ["list"], object()]: + with self.subTest(bad=bad), self.assertRaisesMessage(TypeError, msg): + NamedCategory.objects.comment(bad) + + def test_rejects_comment_delimiters(self): + msg = ( + "QuerySet.comment() cannot include '/*' or '*/'; strip or " + "escape these delimiters before calling comment()." + ) + for bad in [ + "foo /* bar", + "foo */ bar", + "*/-- DROP TABLE x;--/*", + "/* nested */", + ]: + with self.subTest(bad=bad), self.assertRaisesMessage(ValueError, msg): + NamedCategory.objects.comment(bad) + + class QuerySetCloningTests(TestCase): @classmethod def setUpTestData(cls): From c1e9d6bf846b6afeafce36d75712c60752630085 Mon Sep 17 00:00:00 2001 From: siddus Date: Mon, 4 May 2026 16:44:52 -0400 Subject: [PATCH 2/5] Fixed #24638 -- Cleaned QuerySet.comment() docs. --- docs/ref/models/querysets.txt | 12 ++++++------ tests/queries/tests.py | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index ed96050dfa3c..5bd9aeda39f6 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -1935,28 +1935,28 @@ For example: .. method:: comment(message) Returns a new ``QuerySet`` that injects ``message`` into the resulting SQL as -an ``/* … */`` comment, after ``SELECT`` (and ``DISTINCT`` if present) or +an ``/* ... */`` comment, after ``SELECT`` (and ``DISTINCT`` if present) or after ``UPDATE``. For example:: Entry.objects.comment("hello").all() -… roughly produces: +roughly produces: .. code-block:: sql - SELECT /* hello */ "blog_entry"."id", … FROM "blog_entry" + SELECT /* hello */ "blog_entry"."id" FROM "blog_entry" Calls can be chained; each call appends a separate block:: Entry.objects.comment("first").comment("second").update(headline="x") -… produces: +produces: .. code-block:: sql - UPDATE /* first */ /* second */ "blog_entry" SET "headline" = … + UPDATE /* first */ /* second */ "blog_entry" SET "headline" = 'x' ``comment()`` raises :exc:`ValueError` if ``message`` contains ``/*`` or ``*/`` to prevent SQL comment injection, and :exc:`TypeError` if ``message`` @@ -1966,7 +1966,7 @@ is not a string. ``INSERT``, and statements issued by migrations are not annotated. When called on a combined ``QuerySet`` (produced by :meth:`union`, :meth:`intersection`, or :meth:`difference`), the comment is emitted as a -leading ``/* … */`` block before the combined SQL. +leading ``/* ... */`` block before the combined SQL. ``select_for_update()`` ~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 8b567b004c7e..466eb3a533ba 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -4677,8 +4677,8 @@ def test_select_with_distinct(self): def test_select_with_filter_preserves_comment(self): with CaptureQueriesContext(connection) as ctx: - list(NamedCategory.objects.comment("traceparent=00-x").filter(name="x")) - self.assertIn("/* traceparent=00-x */", ctx.captured_queries[0]["sql"]) + list(NamedCategory.objects.comment("request-id=123").filter(name="x")) + self.assertIn("/* request-id=123 */", ctx.captured_queries[0]["sql"]) def test_select_subquery(self): with CaptureQueriesContext(connection) as ctx: From 672fd9fdbb3cdac54724e743a9921090389c6a1f Mon Sep 17 00:00:00 2001 From: siddus Date: Thu, 7 May 2026 14:52:12 -0400 Subject: [PATCH 3/5] Fixed #24638 -- Rejected null bytes in QuerySet.comment(). --- django/db/models/query.py | 6 +++--- django/db/models/sql/compiler.py | 3 ++- docs/ref/models/querysets.txt | 6 +++--- tests/queries/tests.py | 12 ++++++++++-- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index ad30bca45b0f..939d3b667890 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -1987,10 +1987,10 @@ def comment(self, message): """Add an SQL comment to the query.""" if not isinstance(message, str): raise TypeError("QuerySet.comment() argument must be a string.") - if "/*" in message or "*/" in message: + if "/*" in message or "*/" in message or "\x00" in message: raise ValueError( - "QuerySet.comment() cannot include '/*' or '*/'; strip or " - "escape these delimiters before calling comment()." + "QuerySet.comment() cannot include '/*', '*/', or null bytes; " + "remove them before calling comment()." ) clone = self._chain() clone.query.comments = (*clone.query.comments, message) diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 04bab0964579..ec37dbdc246d 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -1690,7 +1690,8 @@ def comments_sql(self): string of ``/* ... */`` blocks. Validation in ``QuerySet.comment()`` already rejects strings containing - ``/*`` or ``*/``, so the values here cannot break out of the comment. + ``/*``, ``*/``, or null bytes, so the values here cannot break out of + the comment or be truncated by database adapters. """ return " ".join("/* %s */" % comment for comment in self.query.comments) diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index 5bd9aeda39f6..b5a374d0d9c6 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -1958,9 +1958,9 @@ produces: UPDATE /* first */ /* second */ "blog_entry" SET "headline" = 'x' -``comment()`` raises :exc:`ValueError` if ``message`` contains ``/*`` or -``*/`` to prevent SQL comment injection, and :exc:`TypeError` if ``message`` -is not a string. +``comment()`` raises :exc:`ValueError` if ``message`` contains ``/*``, +``*/``, or null bytes to prevent SQL comment injection and avoid database +adapter parsing issues, and :exc:`TypeError` if ``message`` is not a string. ``comment()`` only affects ``SELECT`` and ``UPDATE`` statements; ``DELETE``, ``INSERT``, and statements issued by migrations are not annotated. When diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 466eb3a533ba..72ca79af219e 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -4756,8 +4756,8 @@ def test_rejects_non_string(self): def test_rejects_comment_delimiters(self): msg = ( - "QuerySet.comment() cannot include '/*' or '*/'; strip or " - "escape these delimiters before calling comment()." + "QuerySet.comment() cannot include '/*', '*/', or null bytes; " + "remove them before calling comment()." ) for bad in [ "foo /* bar", @@ -4768,6 +4768,14 @@ def test_rejects_comment_delimiters(self): with self.subTest(bad=bad), self.assertRaisesMessage(ValueError, msg): NamedCategory.objects.comment(bad) + def test_rejects_null_byte(self): + msg = ( + "QuerySet.comment() cannot include '/*', '*/', or null bytes; " + "remove them before calling comment()." + ) + with self.assertRaisesMessage(ValueError, msg): + NamedCategory.objects.comment("\x00") + class QuerySetCloningTests(TestCase): @classmethod From d190de98059132515ff16cde13fd64b9c30ec8dd Mon Sep 17 00:00:00 2001 From: siddus Date: Wed, 13 May 2026 12:59:20 -0400 Subject: [PATCH 4/5] Fixed #24638 -- Restricted QuerySet.comment() messages to safe characters. --- django/db/models/query.py | 8 ++++--- django/db/models/sql/compiler.py | 6 ++--- docs/ref/models/querysets.txt | 7 +++--- tests/queries/tests.py | 40 ++++++++++++++------------------ 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index 939d3b667890..58bbc60f96e0 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -39,6 +39,7 @@ from django.utils import timezone from django.utils.deprecation import RemovedInDjango70Warning, django_file_prefixes from django.utils.functional import cached_property +from django.utils.regex_helper import _lazy_re_compile # The maximum number of results to fetch in a get() query. MAX_GET_RESULTS = 21 @@ -47,6 +48,7 @@ REPR_OUTPUT_SIZE = 20 DEFAULT_FETCH_MODE = FETCH_ONE +SQL_COMMENT_RE = _lazy_re_compile(r"^[\w ._-]+$") class BaseIterable: @@ -1987,10 +1989,10 @@ def comment(self, message): """Add an SQL comment to the query.""" if not isinstance(message, str): raise TypeError("QuerySet.comment() argument must be a string.") - if "/*" in message or "*/" in message or "\x00" in message: + if not SQL_COMMENT_RE.fullmatch(message): raise ValueError( - "QuerySet.comment() cannot include '/*', '*/', or null bytes; " - "remove them before calling comment()." + "QuerySet.comment() argument must contain only letters, numbers, " + "spaces, periods, underscores, and hyphens." ) clone = self._chain() clone.query.comments = (*clone.query.comments, message) diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index ec37dbdc246d..a15d7faaa3e1 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -1689,9 +1689,9 @@ def comments_sql(self): Return SQL comments for ``Query.comments`` as a single space-separated string of ``/* ... */`` blocks. - Validation in ``QuerySet.comment()`` already rejects strings containing - ``/*``, ``*/``, or null bytes, so the values here cannot break out of - the comment or be truncated by database adapters. + Validation in ``QuerySet.comment()`` restricts comments to word + characters, spaces, periods, underscores, and hyphens, so the values + here cannot contain SQL comment delimiters or control characters. """ return " ".join("/* %s */" % comment for comment in self.query.comments) diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index b5a374d0d9c6..6d1dbb762e1c 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -1958,9 +1958,10 @@ produces: UPDATE /* first */ /* second */ "blog_entry" SET "headline" = 'x' -``comment()`` raises :exc:`ValueError` if ``message`` contains ``/*``, -``*/``, or null bytes to prevent SQL comment injection and avoid database -adapter parsing issues, and :exc:`TypeError` if ``message`` is not a string. +``message`` must be a non-empty string containing only letters, numbers, +spaces, periods, underscores, and hyphens. ``comment()`` raises +:exc:`ValueError` if ``message`` contains any other characters, and +:exc:`TypeError` if ``message`` is not a string. ``comment()`` only affects ``SELECT`` and ``UPDATE`` statements; ``DELETE``, ``INSERT``, and statements issued by migrations are not annotated. When diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 72ca79af219e..6b6f8681e4c0 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -4646,8 +4646,8 @@ class QuerySetCommentTests(TestCase): def test_select_single_comment(self): with CaptureQueriesContext(connection) as ctx: - list(NamedCategory.objects.comment("blog/views.py:50")) - self.assertIn("SELECT /* blog/views.py:50 */ ", ctx.captured_queries[0]["sql"]) + list(NamedCategory.objects.comment("blog.views.py 50")) + self.assertIn("SELECT /* blog.views.py 50 */ ", ctx.captured_queries[0]["sql"]) def test_select_multiple_comments_preserve_order(self): with CaptureQueriesContext(connection) as ctx: @@ -4661,11 +4661,6 @@ def test_select_multiple_comments_preserve_order(self): ctx.captured_queries[0]["sql"], ) - def test_select_empty_comment_allowed(self): - with CaptureQueriesContext(connection) as ctx: - list(NamedCategory.objects.comment("").comment("non-empty")) - self.assertIn("SELECT /* */ /* non-empty */ ", ctx.captured_queries[0]["sql"]) - def test_select_with_distinct(self): with CaptureQueriesContext(connection) as ctx: list(NamedCategory.objects.distinct().comment("after distinct")) @@ -4677,8 +4672,8 @@ def test_select_with_distinct(self): def test_select_with_filter_preserves_comment(self): with CaptureQueriesContext(connection) as ctx: - list(NamedCategory.objects.comment("request-id=123").filter(name="x")) - self.assertIn("/* request-id=123 */", ctx.captured_queries[0]["sql"]) + list(NamedCategory.objects.comment("request-id 123").filter(name="x")) + self.assertIn("/* request-id 123 */", ctx.captured_queries[0]["sql"]) def test_select_subquery(self): with CaptureQueriesContext(connection) as ctx: @@ -4726,8 +4721,8 @@ def test_clone_preserves_comment(self): def test_unicode_comment(self): with CaptureQueriesContext(connection) as ctx: - list(NamedCategory.objects.comment("источник=cron")) - self.assertIn("/* источник=cron */", ctx.captured_queries[0]["sql"]) + list(NamedCategory.objects.comment("источник cron")) + self.assertIn("/* источник cron */", ctx.captured_queries[0]["sql"]) def test_comment_inside_union_leg(self): qs = NamedCategory.objects.comment("first leg").union( @@ -4754,28 +4749,29 @@ def test_rejects_non_string(self): with self.subTest(bad=bad), self.assertRaisesMessage(TypeError, msg): NamedCategory.objects.comment(bad) - def test_rejects_comment_delimiters(self): + def test_rejects_disallowed_comment_characters(self): msg = ( - "QuerySet.comment() cannot include '/*', '*/', or null bytes; " - "remove them before calling comment()." + "QuerySet.comment() argument must contain only letters, numbers, " + "spaces, periods, underscores, and hyphens." ) for bad in [ + "", + "blog/views.py:50", + "request-id=123", "foo /* bar", "foo */ bar", "*/-- DROP TABLE x;--/*", "/* nested */", + "quote '", + 'quote "', + "semi;colon", + "hash#tag", + "comma,value", + *(f"name{chr(c)}" for c in chain(range(32), range(0x7F, 0xA0))), ]: with self.subTest(bad=bad), self.assertRaisesMessage(ValueError, msg): NamedCategory.objects.comment(bad) - def test_rejects_null_byte(self): - msg = ( - "QuerySet.comment() cannot include '/*', '*/', or null bytes; " - "remove them before calling comment()." - ) - with self.assertRaisesMessage(ValueError, msg): - NamedCategory.objects.comment("\x00") - class QuerySetCloningTests(TestCase): @classmethod From 3d4e6106cf8aceaf2b8604d02b033c05403a64ee Mon Sep 17 00:00:00 2001 From: siddus Date: Thu, 14 May 2026 19:15:35 -0400 Subject: [PATCH 5/5] Refs #24638 -- Renamed SQL comment regex variable. --- django/db/models/query.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index 58bbc60f96e0..dfec9da6b3fe 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -48,7 +48,7 @@ REPR_OUTPUT_SIZE = 20 DEFAULT_FETCH_MODE = FETCH_ONE -SQL_COMMENT_RE = _lazy_re_compile(r"^[\w ._-]+$") +sql_comment_re = _lazy_re_compile(r"^[\w ._-]+$") class BaseIterable: @@ -1989,7 +1989,7 @@ def comment(self, message): """Add an SQL comment to the query.""" if not isinstance(message, str): raise TypeError("QuerySet.comment() argument must be a string.") - if not SQL_COMMENT_RE.fullmatch(message): + if not sql_comment_re.fullmatch(message): raise ValueError( "QuerySet.comment() argument must contain only letters, numbers, " "spaces, periods, underscores, and hyphens."