Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 0 additions & 4 deletions django/db/backends/base/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,10 +420,6 @@ class BaseDatabaseFeatures:
# Does the Round() database function round to even?
rounds_to_even = False

# Should dollar signs be prohibited in column aliases to prevent SQL
# injection?
prohibits_dollar_signs_in_column_aliases = False

# Should PatternLookup.process_rhs() use self.param_pattern? It's unneeded
# on databases that don't use LIKE for pattern matching.
pattern_lookup_needs_param_pattern = True
Expand Down
5 changes: 1 addition & 4 deletions django/db/backends/mysql/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ def as_sql(self):
# window functions as it doesn't allow for GROUP BY/HAVING clauses
# and the subquery wrapping (necessary to emulate QUALIFY).
return super().as_sql()
result = [
"DELETE %s FROM"
% self.quote_name_unless_alias(self.query.get_initial_alias())
]
result = ["DELETE %s FROM" % self.quote_name(self.query.get_initial_alias())]
from_sql, params = self.get_from_clause()
result.extend(from_sql)
try:
Expand Down
1 change: 0 additions & 1 deletion django/db/backends/postgresql/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
supports_nulls_distinct_unique_constraints = True
supports_no_precision_decimalfield = True
can_rename_index = True
prohibits_dollar_signs_in_column_aliases = True
test_collations = {
"deterministic": "C",
"non_default": "sv-x-icu",
Expand Down
2 changes: 1 addition & 1 deletion django/db/models/expressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1366,7 +1366,7 @@ def __repr__(self):
def as_sql(self, compiler, connection):
alias, column = self.alias, self.target.column
identifiers = (alias, column) if alias else (column,)
sql = ".".join(map(compiler.quote_name_unless_alias, identifiers))
sql = ".".join(map(compiler.quote_name, identifiers))
return sql, ()

def relabeled_clone(self, relabels):
Expand Down
65 changes: 28 additions & 37 deletions django/db/models/sql/compiler.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import collections
import json
import re
import warnings
from functools import partial
from itertools import chain

Expand All @@ -23,6 +24,7 @@
)
from django.db.models.sql.query import Query, get_order_dir
from django.db.transaction import TransactionManagementError
from django.utils.deprecation import RemovedInDjango70Warning, django_file_prefixes
from django.utils.functional import cached_property
from django.utils.hashable import make_hashable
from django.utils.regex_helper import _lazy_re_compile
Expand Down Expand Up @@ -439,9 +441,7 @@ def _order_by_pairs(self):
table, col = col.split(".", 1)
yield (
OrderBy(
RawSQL(
"%s.%s" % (self.quote_name_unless_alias(table), col), []
),
RawSQL("%s.%s" % (self.quote_name(table), col), []),
descending=descending,
),
False,
Expand Down Expand Up @@ -547,35 +547,28 @@ def get_extra_select(self, order_by, select):
extra_select.append((expr, (without_ordering, params), None))
return extra_select

def quote_name_unless_alias(self, name):
def quote_name(self, name):
"""
A wrapper around connection.ops.quote_name that doesn't quote aliases
for table names. This avoids problems with some SQL dialects that treat
quoted strings specially (e.g. PostgreSQL).
A wrapper around connection.ops.quote_name that memoizes quoted
name values.
"""
if (
self.connection.features.prohibits_dollar_signs_in_column_aliases
and "$" in name
):
raise ValueError(
"Dollar signs are not permitted in column aliases on "
f"{self.connection.display_name}."
)
if name in self.quote_cache:
return self.quote_cache[name]
if (
(name in self.query.alias_map and name not in self.query.table_map)
or name in self.query.extra_select
or (
self.query.external_aliases.get(name)
and name not in self.query.table_map
)
):
self.quote_cache[name] = name
return name
r = self.connection.ops.quote_name(name)
self.quote_cache[name] = r
return r
if (quoted := self.quote_cache.get(name)) is not None:
return quoted
quoted = self.connection.ops.quote_name(name)
self.quote_cache[name] = quoted
return quoted

# RemovedInDjango70Warning: When the deprecation ends, remove.
def quote_name_unless_alias(self, name):
warnings.warn(
(
"SQLCompiler.quote_name_unless_alias() is deprecated. "
"Use .quote_name() instead."
),
category=RemovedInDjango70Warning,
skip_file_prefixes=django_file_prefixes(),
)
return self.quote_name(name)

def compile(self, node):
vendor_impl = getattr(node, "as_" + self.connection.vendor, None)
Expand Down Expand Up @@ -1175,7 +1168,7 @@ def get_from_clause(self):
alias not in self.query.alias_map
or self.query.alias_refcount[alias] == 1
):
result.append(", %s" % self.quote_name_unless_alias(alias))
result.append(", %s" % self.quote_name(alias))
return result, params

def get_related_selections(
Expand Down Expand Up @@ -1504,7 +1497,7 @@ def _get_field_choices():
if self.connection.features.select_for_update_of_column:
result.append(self.compile(col)[0])
else:
result.append(self.quote_name_unless_alias(col.alias))
result.append(self.quote_name(col.alias))
if invalid_names:
raise FieldError(
"Invalid field name(s) given in select_for_update(of=(...)): %s. "
Expand Down Expand Up @@ -1805,9 +1798,7 @@ def assemble_as_sql(self, fields, value_rows):
return placeholder_rows, param_rows

def as_sql(self):
# We don't need quote_name_unless_alias() here, since these are all
# going to be column names (so we can avoid the extra overhead).
qn = self.connection.ops.quote_name
qn = self.quote_name
opts = self.query.get_meta()
insert_statement = self.connection.ops.insert_statement(
on_conflict=self.query.on_conflict,
Expand Down Expand Up @@ -2006,7 +1997,7 @@ def contains_self_reference_subquery(self):
)

def _as_sql(self, query):
delete = "DELETE FROM %s" % self.quote_name_unless_alias(query.base_table)
delete = "DELETE FROM %s" % self.quote_name(query.base_table)
try:
where, params = self.compile(query.where)
except FullResultSet:
Expand Down Expand Up @@ -2050,7 +2041,7 @@ def as_sql(self):
self.pre_sql_setup()
if not self.query.values:
return "", ()
qn = self.quote_name_unless_alias
qn = self.quote_name
values, update_params = [], []
for field, model, val in self.query.values:
if hasattr(val, "resolve_expression"):
Expand Down
13 changes: 9 additions & 4 deletions django/db/models/sql/datastructures.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def as_sql(self, compiler, connection):
"""
join_conditions = []
params = []
qn = compiler.quote_name_unless_alias
qn = compiler.quote_name
# Add a join condition for each pair of joining columns.
for lhs, rhs in self.join_fields:
lhs, rhs = connection.ops.prepare_join_on_clause(
Expand Down Expand Up @@ -120,7 +120,9 @@ def as_sql(self, compiler, connection):
)
on_clause_sql = " AND ".join(join_conditions)
alias_str = (
"" if self.table_alias == self.table_name else (" %s" % self.table_alias)
""
if self.table_alias == self.table_name
else (" %s" % qn(self.table_alias))
)
sql = "%s %s%s ON (%s)" % (
self.join_type,
Expand Down Expand Up @@ -193,10 +195,13 @@ def __init__(self, table_name, alias):
self.table_alias = alias

def as_sql(self, compiler, connection):
qn = compiler.quote_name
alias_str = (
"" if self.table_alias == self.table_name else (" %s" % self.table_alias)
""
if self.table_alias == self.table_name
else (" %s" % qn(self.table_alias))
)
base_sql = compiler.quote_name_unless_alias(self.table_name)
base_sql = qn(self.table_name)
return base_sql + alias_str, []

def relabeled_clone(self, change_map):
Expand Down
8 changes: 8 additions & 0 deletions django/test/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from asgiref.local import Local

from django.apps import apps
from django.conf import settings
from django.core.exceptions import ImproperlyConfigured
from django.core.signals import setting_changed
from django.db import connections, router
Expand All @@ -13,6 +14,7 @@
from django.utils import timezone
from django.utils.formats import FORMAT_SETTINGS, reset_format_cache
from django.utils.functional import empty
from django.utils.log import configure_logging

template_rendered = Signal()

Expand Down Expand Up @@ -251,3 +253,9 @@ def user_model_swapped(*, setting, **kwargs):
from django.contrib.auth import views

views.UserModel = UserModel


@receiver(setting_changed)
def update_logging_config(*, setting, **kwargs):
if setting in {"LOGGING", "LOGGING_CONFIG"}:
configure_logging(settings.LOGGING_CONFIG, settings.LOGGING)
2 changes: 2 additions & 0 deletions docs/internals/deprecation.txt
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ details on these changes.
* The ``Field.get_placeholder_sql`` shim over the deprecated
``get_placeholder`` method will be removed.

* The ``SQLCompiler.quote_name_unless_alias()`` method will be removed.

.. _deprecation-removed-in-6.1:

6.1
Expand Down
5 changes: 3 additions & 2 deletions docs/ref/forms/fields.txt
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ field. This is used when the ``Field`` is displayed in a ``Form``.

As explained in :ref:`ref-forms-api-outputting-html`, the default label for a
``Field`` is generated from the field name by converting all underscores to
spaces and upper-casing the first letter. Specify ``label`` if that default
behavior doesn't result in an adequate label.
spaces and upper-casing the first letter. Specify a string for ``label`` if
that default behavior doesn't result in an adequate label. Use an empty string
(``""``) to hide the label.

Here's a full example ``Form`` that implements ``label`` for two of its fields.
We've specified ``auto_id=False`` to simplify the output:
Expand Down
18 changes: 18 additions & 0 deletions docs/releases/6.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,9 @@ backends.
* Set the new ``DatabaseFeatures.supports_inspectdb`` attribute to ``False``
if the management command isn't supported.

* The ``DatabaseFeatures.prohibits_dollar_signs_in_column_aliases`` feature
flag is removed.

* The ``DatabaseOperations.binary_placeholder_sql()`` method now expects a
query compiler as an extra positional argument and should return a
two-elements tuple composed of an SQL format string and a tuple of associated
Expand Down Expand Up @@ -434,6 +437,16 @@ backends.
instead of the JSON ``null`` primitive. This matches the behavior of a
standalone :class:`~django.db.models.JSONField` when storing ``None`` values.

Models
------

* SQL ``SELECT`` aliases originating from :meth:`.QuerySet.annotate`
calls as well as table and ``JOIN`` aliases are now systematically quoted to
prevent special character collisions. Because quoted aliases are
case-sensitive, *raw* SQL references to aliases mixing case, such as when
using :class:`.RawSQL`, might have to be adjusted to also make use of
quoting.

System checks
-------------

Expand Down Expand Up @@ -506,6 +519,11 @@ Miscellaneous
to be provided expressions meant to be compiled via the provided ``compiler``
argument.

* The ``quote_name_unless_alias()`` method of ``SQLCompiler``, the type of
object passed as the ``compiler`` argument to the ``as_sql()`` method of
:ref:`expressions <writing-your-own-query-expressions>`, is deprecated in
favor of the newly introduced ``quote_name()`` method.

Features removed in 6.1
=======================

Expand Down
14 changes: 0 additions & 14 deletions tests/annotations/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1574,20 +1574,6 @@ def test_alias_filtered_relation_sql_injection(self):
with self.assertRaisesMessage(ValueError, msg):
Book.objects.alias(**{crafted_alias: FilteredRelation("authors")})

def test_alias_filtered_relation_sql_injection_dollar_sign(self):
qs = Book.objects.alias(
**{"crafted_alia$": FilteredRelation("authors")}
).values("name", "crafted_alia$")
if connection.features.prohibits_dollar_signs_in_column_aliases:
msg = (
"Dollar signs are not permitted in column aliases on "
f"{connection.display_name}."
)
with self.assertRaisesMessage(ValueError, msg):
list(qs)
else:
self.assertEqual(qs.first()["name"], self.b1.name)

def test_values_wrong_alias(self):
expected_message = (
"Cannot resolve keyword 'alias_typo' into field. Choices are: %s"
Expand Down
5 changes: 3 additions & 2 deletions tests/filtered_relation/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,9 @@ def test_internal_queryset_alias_mapping(self):
),
).filter(book_alice__isnull=False)
self.assertIn(
"INNER JOIN {} book_alice ON".format(
connection.ops.quote_name("filtered_relation_book")
"INNER JOIN {} {} ON".format(
connection.ops.quote_name("filtered_relation_book"),
connection.ops.quote_name("book_alice"),
),
str(queryset.query),
)
Expand Down
2 changes: 1 addition & 1 deletion tests/foreign_object/models/article.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def __init__(self, alias, col, value):
self.alias, self.col, self.value = alias, col, value

def as_sql(self, compiler, connection):
qn = compiler.quote_name_unless_alias
qn = compiler.quote_name
return "%s.%s = %%s" % (qn(self.alias), qn(self.col)), [self.value]


Expand Down
42 changes: 42 additions & 0 deletions tests/logging_tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,10 @@ class SetupConfigureLogging(SimpleTestCase):
Calling django.setup() initializes the logging configuration.
"""

def tearDown(self):
super().tearDown()
dictConfig.called = False

def test_configure_initializes_logging(self):
from django import setup

Expand All @@ -586,6 +590,44 @@ def test_configure_initializes_logging(self):
setup()
self.assertTrue(dictConfig.called)

def test_logging_settings_changed(self):
"""
Logging is reconfigured when LOGGING or LOGGING_CONFIG changes.
"""
new_logging_info = {
"version": 1,
"disable_existing_loggers": False,
"loggers": {
"django.test_custom_logger": {
"level": "INFO",
}
},
}
new_logging_warning = {
"version": 1,
"disable_existing_loggers": False,
"loggers": {
"django.test_custom_logger": {
"level": "WARNING",
}
},
}
logger = logging.getLogger("django.test_custom_logger")

with override_settings(LOGGING=new_logging_info):
self.assertEqual(logger.level, logging.INFO)

# Repeating the operation works.
with override_settings(LOGGING=new_logging_warning):
self.assertEqual(logger.level, logging.WARNING)

# The default unconfigured level is NOTSET.
self.assertEqual(logger.level, logging.NOTSET)

self.assertIs(dictConfig.called, False)
with override_settings(LOGGING_CONFIG="logging_tests.tests.dictConfig"):
self.assertIs(dictConfig.called, True)


@override_settings(DEBUG=True, ROOT_URLCONF="logging_tests.urls")
class SecurityLoggerTest(LoggingAssertionMixin, SimpleTestCase):
Expand Down
Loading
Loading