sadpandajoe commented on code in PR #33924:
URL: https://github.com/apache/superset/pull/33924#discussion_r3693512209
##########
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:
This doubles every backslash on PostgreSQL because `Database.get_dialect()`
returns an uninitialized dialect whose `_backslash_escapes` remains true; with
modern `standard_conforming_strings=on`, an existing `url_param` value like
`C:\Users` becomes the different value `C:\\Users` and silently matches the
wrong rows. Could we preserve PostgreSQL backslashes (while retaining the
MySQL/MariaDB hardening) and cover both this helper and `url_param` with
regression tests?
--
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]
