diff --git a/superset/views/base_api.py b/superset/views/base_api.py index 5ddacb93682c..d800ea48dba4 100644 --- a/superset/views/base_api.py +++ b/superset/views/base_api.py @@ -29,6 +29,7 @@ rison as parse_rison, safe, ) +from flask_appbuilder.const import API_FILTERS_RIS_KEY from flask_appbuilder.models.filters import BaseFilter, Filters from flask_appbuilder.models.sqla.filters import FilterStartsWith from flask_appbuilder.models.sqla.interface import SQLAInterface @@ -377,6 +378,35 @@ def _init_properties(self) -> None: self.add_columns = [model_id] super()._init_properties() + def _handle_filters_args(self, rison_args: dict[str, Any]) -> Filters: + """ + Build a request-scoped ``Filters`` instance from Rison-encoded args. + + Overrides :meth:`flask_appbuilder.api.ModelRestApi._handle_filters_args`, + which mutates ``self._filters`` (a single instance shared across + requests on the same API view). Under concurrent traffic that shared + state can leak filters from one request into another — e.g. two + parallel ``GET /api/v1//`` calls filtering by different + values can return mixed results. + + Returning a fresh ``Filters`` per call keeps each request isolated. + Applies to every subclass of ``BaseSupersetModelRestApi`` + (datasets, charts, dashboards, saved queries, queries, databases, + etc.) — see issue #33828 for the original report on the dataset + endpoint. + + :param rison_args: Arguments parsed from the API request's + Rison-encoded ``q`` parameter. + :returns: A request-scoped ``Filters`` instance joined with the + API's base filters. + """ + filters = self.datamodel.get_filters( + search_columns=self.search_columns, + search_filters=self.search_filters, + ) + filters.rest_add_filters(rison_args.get(API_FILTERS_RIS_KEY, [])) + return filters.get_joined_filters(self._base_filters) + def _get_related_filter( self, datamodel: SQLAInterface, column_name: str, value: str ) -> Filters: diff --git a/tests/unit_tests/datasets/api_tests.py b/tests/unit_tests/datasets/api_tests.py index 82e8453c8784..c536fad9ea54 100644 --- a/tests/unit_tests/datasets/api_tests.py +++ b/tests/unit_tests/datasets/api_tests.py @@ -120,3 +120,46 @@ def test_get_dataset_include_rendered_sql_passes_table_to_template_processor( assert response.status_code == 200 mock_get_processor.assert_called_once_with(database=database, table=dataset) + + +def test_handle_filters_args_returns_request_scoped_filters( + session: Session, + client: Any, + full_api_access: None, +) -> None: + """ + ``_handle_filters_args`` must return a fresh ``Filters`` instance per + call so concurrent requests don't share filter state. + + Regression test for #33828: under concurrent traffic the FAB default + implementation mutates ``self._filters`` (a single shared instance), + causing filters from one request to leak into another. + + The fix lives on ``BaseSupersetModelRestApi`` so every superset REST + API subclass (datasets, charts, dashboards, saved queries, etc.) + inherits the request-scoped behavior. This test exercises it via + ``DatasetRestApi`` as a concrete subclass. + """ + from flask_appbuilder.const import API_FILTERS_RIS_KEY + + from superset.datasets.api import DatasetRestApi + + api = DatasetRestApi() + api.datamodel = MagicMock() + api.search_columns = ["table_name"] + api.search_filters = {} + api._base_filters = MagicMock() # noqa: SLF001 + + # Each call should construct a fresh Filters instance via datamodel.get_filters + rison_args = { + API_FILTERS_RIS_KEY: [{"col": "table_name", "opr": "eq", "value": "a"}], + } + api._handle_filters_args(rison_args) # noqa: SLF001 + api._handle_filters_args(rison_args) # noqa: SLF001 + + assert api.datamodel.get_filters.call_count == 2 + # Returned object must be the joined-filters result of the *fresh* Filters, + # not the shared self._filters attribute. + fresh_filters = api.datamodel.get_filters.return_value + assert fresh_filters.rest_add_filters.call_count == 2 + assert fresh_filters.get_joined_filters.call_count == 2