mirror of
https://github.com/apache/superset.git
synced 2026-07-10 08:45:41 +00:00
fix(db_engine_specs): escape schema name in regex; document safe filter pattern (#40642)
Co-authored-by: Claude Code <noreply@anthropic.com>
This commit is contained in:
@@ -1746,7 +1746,8 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
|
||||
raise cls.get_dbapi_mapped_exception(ex) from ex
|
||||
|
||||
if schema and cls.try_remove_schema_from_table_name:
|
||||
tables = {re.sub(f"^{schema}\\.", "", table) for table in tables}
|
||||
escaped_schema = re.escape(schema)
|
||||
tables = {re.sub(f"^{escaped_schema}\\.", "", table) for table in tables}
|
||||
return tables
|
||||
|
||||
@classmethod
|
||||
@@ -1774,7 +1775,8 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
|
||||
raise cls.get_dbapi_mapped_exception(ex) from ex
|
||||
|
||||
if schema and cls.try_remove_schema_from_table_name:
|
||||
views = {re.sub(f"^{schema}\\.", "", view) for view in views}
|
||||
escaped_schema = re.escape(schema)
|
||||
views = {re.sub(f"^{escaped_schema}\\.", "", view) for view in views}
|
||||
return views
|
||||
|
||||
@classmethod
|
||||
|
||||
@@ -356,6 +356,18 @@ class ExtraCache:
|
||||
- you want to have the ability for filter inside the main query for speed
|
||||
purposes
|
||||
|
||||
Always use the ``where_in`` filter for list membership rather than
|
||||
building SQL by hand. The filter renders values with dialect-safe quoting
|
||||
(via SQLAlchemy's ``literal_binds`` compilation) instead of interpolating
|
||||
them directly into the SQL string.
|
||||
|
||||
.. warning::
|
||||
|
||||
Do not manually escape filter values (for example, with
|
||||
``replace("'", "''")``). Hand-rolled escaping is error-prone and easy
|
||||
to get wrong across dialects. Rely on the ``where_in`` filter so values
|
||||
are quoted safely by the engine.
|
||||
|
||||
Usage example::
|
||||
|
||||
|
||||
@@ -377,10 +389,6 @@ class ExtraCache:
|
||||
AND
|
||||
full_name IN {{ filter.get('val')|where_in }}
|
||||
{%- endif -%}
|
||||
{%- if filter.get('op') == 'LIKE' -%}
|
||||
AND
|
||||
full_name LIKE '{{ filter.get('val') | replace("'", "''") }}'
|
||||
{%- endif -%}
|
||||
{%- endfor -%}
|
||||
UNION ALL
|
||||
SELECT
|
||||
|
||||
@@ -1283,3 +1283,59 @@ def test_start_oauth2_dance_falls_back_to_url_for(mocker: MockerFixture) -> None
|
||||
error = exc_info.value.error
|
||||
|
||||
assert error.extra["redirect_uri"] == fallback_uri
|
||||
|
||||
|
||||
def test_get_table_names_strips_schema_with_regex_metacharacters(
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
Test that get_table_names strips a schema prefix containing regex
|
||||
metacharacters without raising and without mangling unrelated names.
|
||||
"""
|
||||
schema = "a.b(c)"
|
||||
|
||||
inspector = mocker.MagicMock()
|
||||
inspector.get_table_names.return_value = [
|
||||
f"{schema}.orders",
|
||||
"axbc.other",
|
||||
]
|
||||
|
||||
database = mocker.MagicMock()
|
||||
|
||||
spec = BaseEngineSpec
|
||||
mocker.patch.object(spec, "try_remove_schema_from_table_name", True)
|
||||
|
||||
tables = spec.get_table_names(database, inspector, schema)
|
||||
|
||||
# The real schema prefix is stripped; the look-alike name is left intact
|
||||
# because the metacharacters are escaped before being used as a regex.
|
||||
# "axbc.other" would match the old unescaped pattern ^a.b(c)\. and be
|
||||
# incorrectly stripped — the escaped version correctly preserves it.
|
||||
assert tables == {"orders", "axbc.other"}
|
||||
|
||||
|
||||
def test_get_view_names_strips_schema_with_regex_metacharacters(
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
Test that get_view_names strips a schema prefix containing regex
|
||||
metacharacters without raising and without mangling unrelated names.
|
||||
"""
|
||||
schema = "a.b(c)"
|
||||
|
||||
inspector = mocker.MagicMock()
|
||||
inspector.get_view_names.return_value = [
|
||||
f"{schema}.report",
|
||||
"axbc.other",
|
||||
]
|
||||
|
||||
database = mocker.MagicMock()
|
||||
|
||||
spec = BaseEngineSpec
|
||||
mocker.patch.object(spec, "try_remove_schema_from_table_name", True)
|
||||
|
||||
views = spec.get_view_names(database, inspector, schema)
|
||||
|
||||
# "axbc.other" would match the old unescaped pattern ^a.b(c)\. and be
|
||||
# incorrectly stripped — the escaped version correctly preserves it.
|
||||
assert views == {"report", "axbc.other"}
|
||||
|
||||
Reference in New Issue
Block a user