File 0001-Illustrate-fix-for-4481-in-terms-of-a-1.2-patch.patch of Package python-SQLAlchemy.16963
From 6d2d5eb63fcfba97efa86fc765bdf840b106e553 Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 8 Apr 2019 22:07:35 -0400
Subject: [PATCH] Illustrate fix for #4481 in terms of a 1.2 patch
Release 1.2 has decided (so far) not to backport 1.3's fix for #4481 as it is
backwards-incompatible with code that relied upon the feature of automatic text
coercion in SQL statements. However, for the specific case of order_by() and
group_by(), we present a patch that backports the specific change in compiler
to have 1.3's behavior for order_by/group_by specifically. This is much more
targeted than the 0.9 version of the patch as it takes advantage 1.0's
architecture which runs all order_by() / group_by() through a label lookup that
only warns if the label can't be matched.
For an example of an application that was actually impacted by 1.3's change
and how they had to change it, see:
https://github.com/ctxis/CAPE/commit/be0482294f5eb30026fe97a967ee5a768d032278
Basically, in the uncommon case an application is actually using the text
coercion feature which was generally little-known, within the order_by()
and group_by() an error is now raised instead of a warning; the application
must instead ensure the SQL fragment is passed within a text() construct.
The above application has also been seeing a warning about this since 1.0
which apparently remained unattended.
The patch includes adjustments to the tests that were testing for the
warning to now test that an exception is raised. Any distro that wants
to patch the specific CVE issue resolved in #4481 to SQLAlchemy 1.0, 1.1
or 1.2 can use this patch.
Change-Id: I3363b21428f1ad8797394b63197375a2e56a0bd7
References: #4481
---
lib/sqlalchemy/sql/compiler.py | 10 ++--
lib/sqlalchemy/sql/elements.py | 11 ++++
test/orm/test_eager_relations.py | 17 ++----
test/orm/test_query.py | 99 +++++++++++++-------------------
test/sql/test_text.py | 56 +++++++-----------
5 files changed, 81 insertions(+), 112 deletions(-)
diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index e27a736ef..467e89371 100644
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -637,12 +637,10 @@ class SQLCompiler(Compiled):
else:
col = with_cols[element.element]
except KeyError:
- # treat it like text()
- util.warn_limited(
- "Can't resolve label reference %r; converting to text()",
- util.ellipses_string(element.element))
- return self.process(
- element._text_clause
+ elements._no_text_coercion(
+ element.element,
+ exc.CompileError,
+ "Can't resolve label reference for ORDER BY / GROUP BY.",
)
else:
kwargs['render_label_as_label'] = col
diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index 7b827d130..03dda6376 100644
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -4306,6 +4306,17 @@ def _literal_as_text(element, warn=False):
)
+def _no_text_coercion(element, exc_cls=exc.ArgumentError, extra=None):
+ raise exc_cls(
+ "%(extra)sTextual SQL expression %(expr)r should be "
+ "explicitly declared as text(%(expr)r)"
+ % {
+ "expr": util.ellipses_string(element),
+ "extra": "%s " % extra if extra else "",
+ }
+ )
+
+
def _no_literals(element):
if hasattr(element, '__clause_element__'):
return element.__clause_element__()
diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index 3c669d90d..704e11313 100644
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -14,7 +14,7 @@ from sqlalchemy.orm import mapper, relationship, create_session, \
from sqlalchemy.sql import operators
from sqlalchemy.testing import assert_raises, assert_raises_message
from sqlalchemy.testing.assertsql import CompiledSQL
-from sqlalchemy.testing import fixtures, expect_warnings
+from sqlalchemy.testing import fixtures
from test.orm import _fixtures
from sqlalchemy.util import OrderedDict as odict
import datetime
@@ -246,16 +246,11 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
q = sess.query(User).options(joinedload("addresses")).\
order_by("email_address")
- with expect_warnings("Can't resolve label reference 'email_address'"):
- self.assert_compile(
- q,
- "SELECT users.id AS users_id, users.name AS users_name, "
- "addresses_1.id AS addresses_1_id, addresses_1.user_id AS "
- "addresses_1_user_id, addresses_1.email_address AS "
- "addresses_1_email_address FROM users LEFT OUTER JOIN "
- "addresses AS addresses_1 ON users.id = addresses_1.user_id "
- "ORDER BY email_address"
- )
+ assert_raises_message(
+ sa.exc.CompileError,
+ "Can't resolve label reference for ORDER BY / GROUP BY.",
+ q.all,
+ )
def test_deferred_fk_col(self):
users, Dingaling, User, dingalings, Address, addresses = (
diff --git a/test/orm/test_query.py b/test/orm/test_query.py
index 880497501..4f4935890 100644
--- a/test/orm/test_query.py
+++ b/test/orm/test_query.py
@@ -15,7 +15,7 @@ import sqlalchemy as sa
from sqlalchemy.testing.assertions import (
eq_, assert_raises, assert_raises_message, expect_warnings,
eq_ignore_whitespace)
-from sqlalchemy.testing import fixtures, AssertsCompiledSQL, assert_warnings
+from sqlalchemy.testing import fixtures, AssertsCompiledSQL
from test.orm import _fixtures
from sqlalchemy.orm.util import join, with_parent
import contextlib
@@ -1885,18 +1885,11 @@ class ColumnPropertyTest(_fixtures.FixtureTest, AssertsCompiledSQL):
ua = aliased(User)
q = s.query(ua).order_by("email_ad")
- def go():
- self.assert_compile(
- q,
- "SELECT (SELECT max(addresses.email_address) AS max_1 "
- "FROM addresses WHERE addresses.user_id = users_1.id) "
- "AS anon_1, users_1.id AS users_1_id, "
- "users_1.name AS users_1_name FROM users AS users_1 "
- "ORDER BY email_ad"
- )
- assert_warnings(
- go,
- ["Can't resolve label reference 'email_ad'"], regex=True)
+ assert_raises_message(
+ sa.exc.CompileError,
+ "Can't resolve label reference for ORDER BY / GROUP BY",
+ q.with_labels().statement.compile,
+ )
def test_order_by_column_labeled_prop_attr_aliased_one(self):
User = self.classes.User
@@ -3498,43 +3491,33 @@ class TextTest(QueryTest, AssertsCompiledSQL):
# the queries here are again "invalid" from a SQL perspective, as the
# "name" field isn't matched up to anything.
#
- with expect_warnings("Can't resolve label reference 'name';"):
- self.assert_compile(
- s.query(User).options(joinedload("addresses")).
- order_by(desc("name")).limit(1),
- "SELECT anon_1.users_id AS anon_1_users_id, "
- "anon_1.users_name AS anon_1_users_name, "
- "addresses_1.id AS addresses_1_id, "
- "addresses_1.user_id AS addresses_1_user_id, "
- "addresses_1.email_address AS addresses_1_email_address "
- "FROM (SELECT users.id AS users_id, users.name AS users_name "
- "FROM users ORDER BY users.name "
- "DESC LIMIT :param_1) AS anon_1 "
- "LEFT OUTER JOIN addresses AS addresses_1 "
- "ON anon_1.users_id = addresses_1.user_id "
- "ORDER BY name DESC, addresses_1.id"
- )
+ q = (
+ s.query(User)
+ .options(joinedload("addresses"))
+ .order_by(desc("name"))
+ .limit(1)
+ )
+ assert_raises_message(
+ sa_exc.CompileError,
+ "Can't resolve label reference for ORDER BY / GROUP BY.",
+ q.with_labels().statement.compile,
+ )
def test_order_by_w_eager_two(self):
User = self.classes.User
s = create_session()
- with expect_warnings("Can't resolve label reference 'name';"):
- self.assert_compile(
- s.query(User).options(joinedload("addresses")).
- order_by("name").limit(1),
- "SELECT anon_1.users_id AS anon_1_users_id, "
- "anon_1.users_name AS anon_1_users_name, "
- "addresses_1.id AS addresses_1_id, "
- "addresses_1.user_id AS addresses_1_user_id, "
- "addresses_1.email_address AS addresses_1_email_address "
- "FROM (SELECT users.id AS users_id, users.name AS users_name "
- "FROM users ORDER BY users.name "
- "LIMIT :param_1) AS anon_1 "
- "LEFT OUTER JOIN addresses AS addresses_1 "
- "ON anon_1.users_id = addresses_1.user_id "
- "ORDER BY name, addresses_1.id"
- )
+ q = (
+ s.query(User)
+ .options(joinedload("addresses"))
+ .order_by("name")
+ .limit(1)
+ )
+ assert_raises_message(
+ sa_exc.CompileError,
+ "Can't resolve label reference for ORDER BY / GROUP BY.",
+ q.with_labels().statement.compile,
+ )
def test_order_by_w_eager_three(self):
User = self.classes.User
@@ -3604,20 +3587,18 @@ class TextTest(QueryTest, AssertsCompiledSQL):
q = sess.query(User, Address.email_address.label('email_address'))
- result = q.join('addresses').options(joinedload(User.orders)).\
- order_by(
- "email_address desc").limit(1).offset(0)
- with expect_warnings(
- "Can't resolve label reference 'email_address desc'"):
- eq_(
- [
- (User(
- id=7,
- orders=[Order(id=1), Order(id=3), Order(id=5)],
- addresses=[Address(id=1)]
- ), 'jack@bean.com')
- ],
- result.all())
+ result = (
+ q.join("addresses")
+ .options(joinedload(User.orders))
+ .order_by("email_address desc")
+ .limit(1)
+ .offset(0)
+ )
+ assert_raises_message(
+ sa_exc.CompileError,
+ "Can't resolve label reference for ORDER BY / GROUP BY",
+ result.all,
+ )
class TextWarningTest(QueryTest, AssertsCompiledSQL):
diff --git a/test/sql/test_text.py b/test/sql/test_text.py
index c31c22853..519b300ff 100644
--- a/test/sql/test_text.py
+++ b/test/sql/test_text.py
@@ -1,7 +1,7 @@
"""Test the TextClause and related constructs."""
from sqlalchemy.testing import fixtures, AssertsCompiledSQL, eq_, \
- assert_raises_message, expect_warnings, assert_warnings
+ assert_raises_message, expect_warnings
from sqlalchemy import text, select, Integer, String, Float, \
bindparam, and_, func, literal_column, exc, MetaData, Table, Column,\
asc, func, desc, union, literal
@@ -580,18 +580,14 @@ class TextWarningsTest(fixtures.TestBase, AssertsCompiledSQL):
class OrderByLabelResolutionTest(fixtures.TestBase, AssertsCompiledSQL):
__dialect__ = 'default'
- def _test_warning(self, stmt, offending_clause, expected):
- with expect_warnings(
- "Can't resolve label reference %r;" % offending_clause):
- self.assert_compile(
- stmt,
- expected
- )
+ def _test_exception(self, stmt, offending_clause):
assert_raises_message(
- exc.SAWarning,
- "Can't resolve label reference %r; converting to text" %
- offending_clause,
- stmt.compile
+ exc.CompileError,
+ r"Can't resolve label reference for ORDER BY / GROUP BY. "
+ "Textual SQL "
+ "expression %r should be explicitly "
+ r"declared as text\(%r\)" % (offending_clause, offending_clause),
+ stmt.compile,
)
def test_order_by_label(self):
@@ -641,11 +637,8 @@ class OrderByLabelResolutionTest(fixtures.TestBase, AssertsCompiledSQL):
)
def test_unresolvable_warning_order_by(self):
- stmt = select([table1.c.myid]).order_by('foobar')
- self._test_warning(
- stmt, "foobar",
- "SELECT mytable.myid FROM mytable ORDER BY foobar"
- )
+ stmt = select([table1.c.myid]).order_by("foobar")
+ self._test_exception(stmt, "foobar")
def test_group_by_label(self):
stmt = select([table1.c.myid.label('foo')]).group_by('foo')
@@ -662,11 +655,8 @@ class OrderByLabelResolutionTest(fixtures.TestBase, AssertsCompiledSQL):
)
def test_unresolvable_warning_group_by(self):
- stmt = select([table1.c.myid]).group_by('foobar')
- self._test_warning(
- stmt, "foobar",
- "SELECT mytable.myid FROM mytable GROUP BY foobar"
- )
+ stmt = select([table1.c.myid]).group_by("foobar")
+ self._test_exception(stmt, "foobar")
def test_asc(self):
stmt = select([table1.c.myid]).order_by(asc('name'), 'description')
@@ -764,20 +754,14 @@ class OrderByLabelResolutionTest(fixtures.TestBase, AssertsCompiledSQL):
s1 = select([adapter.columns[expr] for expr in exprs]).\
apply_labels().order_by("myid", "t1name", "x")
- def go():
- # the labels here are anonymized, so label naming
- # can't catch these.
- self.assert_compile(
- s1,
- "SELECT mytable_1.myid AS mytable_1_myid, "
- "mytable_1.name AS name_1, foo(:foo_2) AS foo_1 "
- "FROM mytable AS mytable_1 ORDER BY mytable_1.myid, t1name, x"
- )
-
- assert_warnings(
- go,
- ["Can't resolve label reference 't1name'",
- "Can't resolve label reference 'x'"], regex=True)
+ assert_raises_message(
+ exc.CompileError,
+ r"Can't resolve label reference for ORDER BY / GROUP BY. "
+ "Textual SQL "
+ "expression 't1name' should be explicitly "
+ r"declared as text\('t1name'\)",
+ s1.compile,
+ )
def test_columnadapter_non_anonymized(self):
"""test issue #3148
--
2.22.0
