rusackas commented on code in PR #33924:
URL: https://github.com/apache/superset/pull/33924#discussion_r3717370439
##########
superset/jinja_context.py:
##########
@@ -349,22 +423,32 @@ def filter_values(
def _escape_value(self, val: Any) -> Any:
"""Return a dialect-quoted form of ``val`` suitable for direct SQL
interpolation. When no dialect is configured the value is returned
- unchanged so callers see the raw value as before. Strings are
- passed through SQLAlchemy's ``String`` literal processor (with the
- surrounding quotes stripped, mirroring ``url_param``). Lists are
- processed element-wise; non-string members are left as-is.
+ unchanged so callers see the raw value as before.
+
+ Strings are rendered through the dialect compiler's
+ ``render_literal_value`` (with the surrounding quotes stripped),
+ which applies dialect-specific escaping beyond quote doubling; in
+ particular, MySQL/MariaDB treat the backslash as an escape
+ character, so backslashes are doubled there to prevent a trailing
+ ``\\'`` from re-opening the string literal. Dialects whose escaping
+ mode cannot be introspected without a live connection err on the
+ side of over-escaping, which can distort a backslash-containing
+ value but can never widen the query.
+
+ Lists are processed element-wise and dict values recursively, so
+ strings nested inside JSON structures are also escaped; dict keys
+ are left untouched since they are used for member lookups, not
+ interpolation. Non-string leaf values are left as-is.
"""
if not self.dialect:
return val
if isinstance(val, str):
- return
String().literal_processor(dialect=self.dialect)(value=val)[1:-1]
+ compiler = self.dialect.statement_compiler(self.dialect, None)
+ return compiler.render_literal_value(val, String())[1:-1]
Review Comment:
I don't think this reproduces in practice. render_literal_value here always
compiles against a plain String() type, and I checked SQLAlchemy's bundled
mysql/mssql/postgres/oracle/sqlite dialects directly, none of them add a
literal prefix for that type. Happy to look again if you've got a dialect where
it actually shows up.
##########
tests/integration_tests/security/api_tests.py:
##########
@@ -195,6 +195,174 @@ def test_post_guest_token_authorized(self):
assert user == decoded_token["user"]
assert resource == decoded_token["resources"][0]
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_post_guest_token_with_attributes(self) -> None:
Review Comment:
The new test methods here already have -> None per our type-hint rule. The
rest of this file predates that standard though, so retrofitting a bunch more
methods feels like a separate cleanup PR rather than something to fold into
this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
