333 lines
13 KiB
Diff
333 lines
13 KiB
Diff
From 82b4dcdeb0505f2dfcece5f76045b28b0edda03d Mon Sep 17 00:00:00 2001
|
|
From: Mike Bayer <mike_mp@zzzcomputing.com>
|
|
Date: Mon, 08 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
|
|
---
|
|
|
|
diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
|
|
index 5a11ed1..4780bab 100644
|
|
--- a/lib/sqlalchemy/sql/compiler.py
|
|
+++ b/lib/sqlalchemy/sql/compiler.py
|
|
@@ -757,12 +757,11 @@
|
|
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),
|
|
+ elements._no_text_coercion(
|
|
+ element.element,
|
|
+ exc.CompileError,
|
|
+ "Can't resolve label reference for ORDER BY / GROUP BY.",
|
|
)
|
|
- return self.process(element._text_clause)
|
|
else:
|
|
kwargs["render_label_as_label"] = col
|
|
return self.process(
|
|
diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
|
|
index 299fcad..ff86deb 100644
|
|
--- a/lib/sqlalchemy/sql/elements.py
|
|
+++ b/lib/sqlalchemy/sql/elements.py
|
|
@@ -4432,6 +4432,17 @@
|
|
)
|
|
|
|
|
|
+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 abcb597..fc9531d 100644
|
|
--- a/test/orm/test_eager_relations.py
|
|
+++ b/test/orm/test_eager_relations.py
|
|
@@ -32,7 +32,6 @@
|
|
from sqlalchemy.testing import assert_raises
|
|
from sqlalchemy.testing import assert_raises_message
|
|
from sqlalchemy.testing import eq_
|
|
-from sqlalchemy.testing import expect_warnings
|
|
from sqlalchemy.testing import fixtures
|
|
from sqlalchemy.testing import in_
|
|
from sqlalchemy.testing import is_
|
|
@@ -343,16 +342,11 @@
|
|
.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 04ce8b5..0710507 100644
|
|
--- a/test/orm/test_query.py
|
|
+++ b/test/orm/test_query.py
|
|
@@ -51,7 +51,6 @@
|
|
from sqlalchemy.orm.util import with_parent
|
|
from sqlalchemy.sql import expression
|
|
from sqlalchemy.sql import operators
|
|
-from sqlalchemy.testing import assert_warnings
|
|
from sqlalchemy.testing import AssertsCompiledSQL
|
|
from sqlalchemy.testing import fixtures
|
|
from sqlalchemy.testing import is_
|
|
@@ -2139,18 +2138,10 @@
|
|
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):
|
|
@@ -4143,47 +4134,33 @@
|
|
# 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
|
|
@@ -4268,22 +4245,11 @@
|
|
.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(),
|
|
- )
|
|
+ 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 317fc61..75f1d6f 100644
|
|
--- a/test/sql/test_text.py
|
|
+++ b/test/sql/test_text.py
|
|
@@ -22,7 +22,6 @@
|
|
from sqlalchemy.sql import table
|
|
from sqlalchemy.sql import util as sql_util
|
|
from sqlalchemy.testing import assert_raises_message
|
|
-from sqlalchemy.testing import assert_warnings
|
|
from sqlalchemy.testing import AssertsCompiledSQL
|
|
from sqlalchemy.testing import eq_
|
|
from sqlalchemy.testing import expect_warnings
|
|
@@ -633,15 +632,13 @@
|
|
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,
|
|
+ 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,
|
|
)
|
|
|
|
@@ -691,9 +688,7 @@
|
|
|
|
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"
|
|
- )
|
|
+ self._test_exception(stmt, "foobar")
|
|
|
|
def test_group_by_label(self):
|
|
stmt = select([table1.c.myid.label("foo")]).group_by("foo")
|
|
@@ -709,9 +704,7 @@
|
|
|
|
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"
|
|
- )
|
|
+ self._test_exception(stmt, "foobar")
|
|
|
|
def test_asc(self):
|
|
stmt = select([table1.c.myid]).order_by(asc("name"), "description")
|
|
@@ -810,23 +803,13 @@
|
|
.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):
|