From 6b5d82749c57a1aae8c9e81d2b85b2cadb9ef933 Mon Sep 17 00:00:00 2001 From: Thomas Chaumeny Date: Sat, 27 Sep 2014 12:41:54 +0200 Subject: [PATCH] Fixed #16731 -- Made pattern lookups work properly with F() expressions --- django/db/backends/mysql/base.py | 18 +++++ django/db/backends/oracle/base.py | 26 +++++++ .../db/backends/postgresql_psycopg2/base.py | 17 ++++- django/db/backends/sqlite3/base.py | 17 ++++- django/db/models/lookups.py | 28 ++++---- docs/releases/1.8.txt | 6 ++ tests/custom_lookups/tests.py | 3 + tests/expressions/tests.py | 67 ++++++++++++++++++- 8 files changed, 163 insertions(+), 19 deletions(-) diff --git a/django/db/backends/mysql/base.py b/django/db/backends/mysql/base.py index 6a1989715a..566ce6c0a7 100644 --- a/django/db/backends/mysql/base.py +++ b/django/db/backends/mysql/base.py @@ -425,6 +425,24 @@ class DatabaseWrapper(BaseDatabaseWrapper): 'iendswith': 'LIKE %s', } + # The patterns below are used to generate SQL pattern lookup clauses when + # the right-hand side of the lookup isn't a raw string (it might be an expression + # or the result of a bilateral transformation). + # In those cases, special characters for LIKE operators (e.g. \, *, _) should be + # escaped on database side. + # + # Note: we use str.format() here for readability as '%' is used as a wildcard for + # the LIKE operator. + pattern_esc = r"REPLACE(REPLACE(REPLACE({}, '\\', '\\\\'), '%%', '\%%'), '_', '\_')" + pattern_ops = { + 'contains': "LIKE BINARY CONCAT('%%', {}, '%%')", + 'icontains': "LIKE CONCAT('%%', {}, '%%')", + 'startswith': "LIKE BINARY CONCAT({}, '%%')", + 'istartswith': "LIKE CONCAT({}, '%%')", + 'endswith': "LIKE BINARY CONCAT('%%', {})", + 'iendswith': "LIKE CONCAT('%%', {})", + } + Database = Database SchemaEditorClass = DatabaseSchemaEditor diff --git a/django/db/backends/oracle/base.py b/django/db/backends/oracle/base.py index b41386d4f0..aa97022931 100644 --- a/django/db/backends/oracle/base.py +++ b/django/db/backends/oracle/base.py @@ -607,6 +607,30 @@ class DatabaseWrapper(BaseDatabaseWrapper): 'iendswith': "LIKEC UPPER(%s) ESCAPE '\\'", }) + # The patterns below are used to generate SQL pattern lookup clauses when + # the right-hand side of the lookup isn't a raw string (it might be an expression + # or the result of a bilateral transformation). + # In those cases, special characters for LIKE operators (e.g. \, *, _) should be + # escaped on database side. + # + # Note: we use str.format() here for readability as '%' is used as a wildcard for + # the LIKE operator. + pattern_esc = r"REPLACE(REPLACE(REPLACE({}, '\', '\\'), '%%', '\%%'), '_', '\_')" + _pattern_ops = { + 'contains': "'%%' || {} || '%%'", + 'icontains': "'%%' || UPPER({}) || '%%'", + 'startswith': "{} || '%%'", + 'istartswith': "UPPER({}) || '%%'", + 'endswith': "'%%' || {}", + 'iendswith': "'%%' || UPPER({})", + } + + _standard_pattern_ops = {k: "LIKE TRANSLATE( " + v + " USING NCHAR_CS)" + " ESCAPE TRANSLATE('\\' USING NCHAR_CS)" + for k, v in _pattern_ops.items()} + _likec_pattern_ops = {k: "LIKEC " + v + " ESCAPE '\\'" + for k, v in _pattern_ops.items()} + Database = Database SchemaEditorClass = DatabaseSchemaEditor @@ -674,8 +698,10 @@ class DatabaseWrapper(BaseDatabaseWrapper): ['X']) except DatabaseError: self.operators = self._likec_operators + self.pattern_ops = self._likec_pattern_ops else: self.operators = self._standard_operators + self.pattern_ops = self._standard_pattern_ops cursor.close() try: diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py index 45fb7c77f0..1530e20eb7 100644 --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -86,9 +86,22 @@ class DatabaseWrapper(BaseDatabaseWrapper): 'iendswith': 'LIKE UPPER(%s)', } + # The patterns below are used to generate SQL pattern lookup clauses when + # the right-hand side of the lookup isn't a raw string (it might be an expression + # or the result of a bilateral transformation). + # In those cases, special characters for LIKE operators (e.g. \, *, _) should be + # escaped on database side. + # + # Note: we use str.format() here for readability as '%' is used as a wildcard for + # the LIKE operator. + pattern_esc = r"REPLACE(REPLACE(REPLACE({}, '\', '\\'), '%%', '\%%'), '_', '\_')" pattern_ops = { - 'startswith': "LIKE %s || '%%%%'", - 'istartswith': "LIKE UPPER(%s) || '%%%%'", + 'contains': "LIKE '%%' || {} || '%%'", + 'icontains': "LIKE '%%' || UPPER({}) || '%%'", + 'startswith': "LIKE {} || '%%'", + 'istartswith': "LIKE UPPER({}) || '%%'", + 'endswith': "LIKE '%%' || {}", + 'iendswith': "LIKE '%%' || UPPER({})", } Database = Database diff --git a/django/db/backends/sqlite3/base.py b/django/db/backends/sqlite3/base.py index 4799576ba9..e85bfaf031 100644 --- a/django/db/backends/sqlite3/base.py +++ b/django/db/backends/sqlite3/base.py @@ -343,9 +343,22 @@ class DatabaseWrapper(BaseDatabaseWrapper): 'iendswith': "LIKE %s ESCAPE '\\'", } + # The patterns below are used to generate SQL pattern lookup clauses when + # the right-hand side of the lookup isn't a raw string (it might be an expression + # or the result of a bilateral transformation). + # In those cases, special characters for LIKE operators (e.g. \, *, _) should be + # escaped on database side. + # + # Note: we use str.format() here for readability as '%' is used as a wildcard for + # the LIKE operator. + pattern_esc = r"REPLACE(REPLACE(REPLACE({}, '\', '\\'), '%%', '\%%'), '_', '\_')" pattern_ops = { - 'startswith': "LIKE %s || '%%%%'", - 'istartswith': "LIKE UPPER(%s) || '%%%%'", + 'contains': r"LIKE '%%' || {} || '%%' ESCAPE '\'", + 'icontains': r"LIKE '%%' || UPPER({}) || '%%' ESCAPE '\'", + 'startswith': r"LIKE {} || '%%' ESCAPE '\'", + 'istartswith': r"LIKE UPPER({}) || '%%' ESCAPE '\'", + 'endswith': r"LIKE '%%' || {} ESCAPE '\'", + 'iendswith': r"LIKE '%%' || UPPER({}) ESCAPE '\'", } Database = Database diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py index 062a9a10ed..9ba5791a15 100644 --- a/django/db/models/lookups.py +++ b/django/db/models/lookups.py @@ -225,16 +225,6 @@ class IExact(BuiltinLookup): default_lookups['iexact'] = IExact -class Contains(BuiltinLookup): - lookup_name = 'contains' -default_lookups['contains'] = Contains - - -class IContains(BuiltinLookup): - lookup_name = 'icontains' -default_lookups['icontains'] = IContains - - class GreaterThan(BuiltinLookup): lookup_name = 'gt' default_lookups['gt'] = GreaterThan @@ -306,6 +296,7 @@ default_lookups['in'] = In class PatternLookup(BuiltinLookup): + def get_rhs_op(self, connection, rhs): # Assume we are in startswith. We need to produce SQL like: # col LIKE %s, ['thevalue%'] @@ -318,11 +309,22 @@ class PatternLookup(BuiltinLookup): # pattern added. if (hasattr(self.rhs, 'get_compiler') or hasattr(self.rhs, 'as_sql') or hasattr(self.rhs, '_as_sql') or self.bilateral_transforms): - return connection.pattern_ops[self.lookup_name] % rhs + pattern = connection.pattern_ops[self.lookup_name].format(connection.pattern_esc) + return pattern.format(rhs) else: return super(PatternLookup, self).get_rhs_op(connection, rhs) +class Contains(PatternLookup): + lookup_name = 'contains' +default_lookups['contains'] = Contains + + +class IContains(PatternLookup): + lookup_name = 'icontains' +default_lookups['icontains'] = IContains + + class StartsWith(PatternLookup): lookup_name = 'startswith' default_lookups['startswith'] = StartsWith @@ -333,12 +335,12 @@ class IStartsWith(PatternLookup): default_lookups['istartswith'] = IStartsWith -class EndsWith(BuiltinLookup): +class EndsWith(PatternLookup): lookup_name = 'endswith' default_lookups['endswith'] = EndsWith -class IEndsWith(BuiltinLookup): +class IEndsWith(PatternLookup): lookup_name = 'iendswith' default_lookups['iendswith'] = IEndsWith diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index b06c32ce5f..ffdd33c12f 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -382,6 +382,12 @@ Models are applied to both ``lhs`` and ``rhs`` when used in a lookup expression, providing opportunities for more sophisticated lookups. +* SQL special characters (\, %, _) are now escaped properly when a pattern + lookup (e.g. ``contains``, ``startswith``, etc.) is used with an ``F()`` + expression as the right-hand side. In those cases, the escaping is performed + by the database, which can lead to somewhat complex queries involving nested + ``REPLACE`` function calls. + Signals ^^^^^^^ diff --git a/tests/custom_lookups/tests.py b/tests/custom_lookups/tests.py index 620db613a1..e9a205be5b 100644 --- a/tests/custom_lookups/tests.py +++ b/tests/custom_lookups/tests.py @@ -286,6 +286,9 @@ class BilateralTransformTests(TestCase): self.assertQuerysetEqual( Author.objects.filter(name__upper='doe'), ["", ""], ordered=False) + self.assertQuerysetEqual( + Author.objects.filter(name__upper__contains='f'), + [""], ordered=False) finally: models.CharField._unregister_lookup(UpperBilateralTransform) diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py index 4c4f9dd407..344b7846fb 100644 --- a/tests/expressions/tests.py +++ b/tests/expressions/tests.py @@ -4,9 +4,8 @@ from copy import deepcopy import datetime from django.core.exceptions import FieldError -from django.db import connection +from django.db import connection, transaction from django.db.models import F -from django.db import transaction from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature from django.test.utils import Approximate from django.utils import six @@ -311,6 +310,70 @@ class ExpressionsTests(TestCase): # The original query still works correctly self.assertEqual(c_qs.get(), c) + def test_patterns_escape(self): + """ + Test that special characters (e.g. %, _ and \) stored in database are + properly escaped when using a pattern lookup with an expression + refs #16731 + """ + Employee.objects.bulk_create([ + Employee(firstname="%Joh\\nny", lastname="%Joh\\n"), + Employee(firstname="Johnny", lastname="%John"), + Employee(firstname="Jean-Claude", lastname="Claud_"), + Employee(firstname="Jean-Claude", lastname="Claude"), + Employee(firstname="Jean-Claude", lastname="Claude%"), + Employee(firstname="Johnny", lastname="Joh\\n"), + Employee(firstname="Johnny", lastname="John"), + Employee(firstname="Johnny", lastname="_ohn"), + ]) + + self.assertQuerysetEqual( + Employee.objects.filter(firstname__contains=F('lastname')), + ["", "", ""], + ordered=False) + + self.assertQuerysetEqual( + Employee.objects.filter(firstname__startswith=F('lastname')), + ["", ""], + ordered=False) + + self.assertQuerysetEqual( + Employee.objects.filter(firstname__endswith=F('lastname')), + [""], + ordered=False) + + def test_insensitive_patterns_escape(self): + """ + Test that special characters (e.g. %, _ and \) stored in database are + properly escaped when using a case insensitive pattern lookup with an + expression -- refs #16731 + """ + Employee.objects.bulk_create([ + Employee(firstname="%Joh\\nny", lastname="%joh\\n"), + Employee(firstname="Johnny", lastname="%john"), + Employee(firstname="Jean-Claude", lastname="claud_"), + Employee(firstname="Jean-Claude", lastname="claude"), + Employee(firstname="Jean-Claude", lastname="claude%"), + Employee(firstname="Johnny", lastname="joh\\n"), + Employee(firstname="Johnny", lastname="john"), + Employee(firstname="Johnny", lastname="_ohn"), + ]) + + self.assertQuerysetEqual( + Employee.objects.filter(firstname__icontains=F('lastname')), + ["", "", ""], + ordered=False) + + self.assertQuerysetEqual( + Employee.objects.filter(firstname__istartswith=F('lastname')), + ["", ""], + ordered=False) + + self.assertQuerysetEqual( + Employee.objects.filter(firstname__iendswith=F('lastname')), + [""], + ordered=False) + class ExpressionsNumericTests(TestCase):