You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/03/24 14:59:44 UTC
(superset) branch 6.1 updated (4041a46b6eb -> 1d170f0da17)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 6.1
in repository https://gitbox.apache.org/repos/asf/superset.git
discard 4041a46b6eb fix(keys): Unsafe dict access in get_native_filters_params() crashes execution (#38272)
new 1d170f0da17 fix(keys): Unsafe dict access in get_native_filters_params() crashes execution (#38272)
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (4041a46b6eb)
\
N -- N -- N refs/heads/6.1 (1d170f0da17)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../src/components/Chart/DrillBy/DrillBySubmenu.test.tsx | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
(superset) 01/01: fix(keys): Unsafe dict access in get_native_filters_params() crashes execution (#38272)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 6.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 1d170f0da173e9a210a55e3cd80870b8e844f711
Author: Alexandru Soare <37...@users.noreply.github.com>
AuthorDate: Tue Mar 24 15:43:27 2026 +0200
fix(keys): Unsafe dict access in get_native_filters_params() crashes execution (#38272)
(cherry picked from commit 89d1b80ce7d22c9c0a90fbc4db0455c16e896f72)
---
.../Chart/DrillBy/DrillBySubmenu.test.tsx | 6 +-
superset/commands/report/execute.py | 23 +++-
superset/reports/models.py | 39 +++++--
tests/unit_tests/commands/report/execute_test.py | 8 ++
tests/unit_tests/reports/model_test.py | 121 +++++++++++++++++----
5 files changed, 162 insertions(+), 35 deletions(-)
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx
index b723c13d570..02c2d11a22b 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx
@@ -274,7 +274,11 @@ test('When menu item is clicked, call onSelection with clicked column and drill
test('matrixify_mode_rows enabled should not render component', () => {
const { container } = renderSubmenu({
- formData: { ...defaultFormData, matrixify_enable: true, matrixify_mode_rows: 'metrics' },
+ formData: {
+ ...defaultFormData,
+ matrixify_enable: true,
+ matrixify_mode_rows: 'metrics',
+ },
});
expect(container).toBeEmptyDOMElement();
});
diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py
index 90f574e27cb..a59e13afaa5 100644
--- a/superset/commands/report/execute.py
+++ b/superset/commands/report/execute.py
@@ -99,6 +99,7 @@ class BaseReportState:
self._scheduled_dttm = scheduled_dttm
self._start_dttm = datetime.utcnow()
self._execution_id = execution_id
+ self._filter_warnings: list[str] = []
def update_report_schedule_and_log(
self,
@@ -265,7 +266,11 @@ class BaseReportState:
if (
dashboard_state := self._report_schedule.extra.get("dashboard")
) and feature_flag_manager.is_feature_enabled("ALERT_REPORT_TABS"):
- native_filter_params = self._report_schedule.get_native_filters_params()
+ native_filter_params, filter_warnings = (
+ self._report_schedule.get_native_filters_params()
+ )
+ if filter_warnings:
+ self._filter_warnings.extend(filter_warnings)
if anchor := dashboard_state.get("anchor"):
try:
anchor_list = json.loads(anchor)
@@ -835,7 +840,13 @@ class ReportNotTriggeredErrorState(BaseReportState):
)
return
self.send()
- self.update_report_schedule_and_log(ReportState.SUCCESS)
+ # Include filter warnings in the log if any were collected
+ warning_message = (
+ ";".join(self._filter_warnings) if self._filter_warnings else None
+ )
+ self.update_report_schedule_and_log(
+ ReportState.SUCCESS, error_message=warning_message
+ )
except (SupersetErrorsException, Exception) as first_ex:
error_message = str(first_ex)
if isinstance(first_ex, SupersetErrorsException):
@@ -980,7 +991,13 @@ class ReportSuccessState(BaseReportState):
try:
self.send()
- self.update_report_schedule_and_log(ReportState.SUCCESS)
+ # Include filter warnings in the log if any were collected
+ warning_message = (
+ ";".join(self._filter_warnings) if self._filter_warnings else None
+ )
+ self.update_report_schedule_and_log(
+ ReportState.SUCCESS, error_message=warning_message
+ )
except Exception as ex: # pylint: disable=broad-except
try:
self.update_report_schedule_and_log(
diff --git a/superset/reports/models.py b/superset/reports/models.py
index 4fa573d6f1e..e26c3fbdbf9 100644
--- a/superset/reports/models.py
+++ b/superset/reports/models.py
@@ -16,6 +16,7 @@
# under the License.
"""A collection of ORM sqlalchemy models for Superset"""
+import logging
from typing import Any, Optional
import prison
@@ -47,6 +48,8 @@ from superset.reports.types import ReportScheduleExtra
from superset.utils.backports import StrEnum
from superset.utils.core import MediumText
+logger = logging.getLogger(__name__)
+
metadata = Model.metadata # pylint: disable=no-member
@@ -187,24 +190,46 @@ class ReportSchedule(AuditMixinNullable, ExtraJSONMixin, Model):
def crontab_humanized(self) -> str:
return get_description(self.crontab)
- def get_native_filters_params(self) -> str:
+ def get_native_filters_params(self) -> tuple[str, list[str]]:
+ """
+ Generate native filter params for dashboard URL.
+
+ Returns:
+ A tuple of (rison_encoded_params, list_of_warning_messages).
+ Warnings are returned so they can be surfaced to users in the
+ execution log.
+ """
params: dict[str, Any] = {}
+ warnings: list[str] = []
dashboard = self.extra.get("dashboard")
if dashboard and dashboard.get("nativeFilters"):
- for filter in dashboard.get("nativeFilters") or []: # type: ignore
+ native_filters = dashboard.get("nativeFilters") or []
+ for native_filter in native_filters: # type: ignore
+ native_filter_id = native_filter.get("nativeFilterId")
+ filter_type = native_filter.get("filterType")
+
+ if native_filter_id is None or filter_type is None:
+ warning_msg = (
+ f"Skipping malformed native filter missing required "
+ f"fields: {native_filter}"
+ )
+ warnings.append(warning_msg)
+ logger.warning(warning_msg)
+ continue
+
params = {
**params,
**self._generate_native_filter(
- filter["nativeFilterId"],
- filter["filterType"],
- filter["columnName"],
- filter["filterValues"],
+ native_filter_id,
+ filter_type,
+ native_filter.get("columnName") or "",
+ native_filter.get("filterValues") or [],
),
}
# hack(hughhh): workaround for escaping prison not handling quotes right
rison = prison.dumps(params)
rison = rison.replace("'", "%27")
- return rison
+ return rison, warnings
def _generate_native_filter(
self,
diff --git a/tests/unit_tests/commands/report/execute_test.py b/tests/unit_tests/commands/report/execute_test.py
index e40a45d9bbe..f1f73bcf4bc 100644
--- a/tests/unit_tests/commands/report/execute_test.py
+++ b/tests/unit_tests/commands/report/execute_test.py
@@ -290,6 +290,10 @@ def test_get_dashboard_urls_with_multiple_tabs(
"urlParams": None,
}
}
+ mock_report_schedule.get_native_filters_params.return_value = ( # type: ignore
+ "()",
+ [],
+ )
class_instance: BaseReportState = BaseReportState(
mock_report_schedule, "January 1, 2021", "execution_id_example"
@@ -333,6 +337,10 @@ def test_get_dashboard_urls_with_exporting_dashboard_only(
"urlParams": None,
}
}
+ mock_report_schedule.get_native_filters_params.return_value = ( # type: ignore
+ "()",
+ [],
+ )
mock_run.return_value = "url1"
class_instance: BaseReportState = BaseReportState(
diff --git a/tests/unit_tests/reports/model_test.py b/tests/unit_tests/reports/model_test.py
index 19e12e14072..a741a06bf70 100644
--- a/tests/unit_tests/reports/model_test.py
+++ b/tests/unit_tests/reports/model_test.py
@@ -14,7 +14,6 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-import pytest
from superset.reports.models import ReportSchedule
@@ -37,9 +36,14 @@ def test_get_native_filters_params():
}
}
- assert report_schedule.get_native_filters_params() == (
- "(filter_id:(extraFormData:(filters:!((col:column_name,op:IN,val:!(value1,value2)))),filterState:(label:column_name,validateStatus:!f,value:!(value1,value2)),id:filter_id,ownState:()))"
+ result, warnings = report_schedule.get_native_filters_params()
+ expected = (
+ "(filter_id:(extraFormData:(filters:!((col:column_name,op:IN,"
+ "val:!(value1,value2)))),filterState:(label:column_name,"
+ "validateStatus:!f,value:!(value1,value2)),id:filter_id,ownState:()))"
)
+ assert result == expected
+ assert warnings == []
def test_get_native_filters_params_multiple_filters():
@@ -66,9 +70,17 @@ def test_get_native_filters_params_multiple_filters():
}
}
- assert report_schedule.get_native_filters_params() == (
- "(filter_id_1:(extraFormData:(filters:!((col:column_name_1,op:IN,val:!(value1,value2)))),filterState:(label:column_name_1,validateStatus:!f,value:!(value1,value2)),id:filter_id_1,ownState:()),filter_id_2:(extraFormData:(filters:!((col:column_name_2,op:IN,val:!(value3,value4)))),filterState:(label:column_name_2,validateStatus:!f,value:!(value3,value4)),id:filter_id_2,ownState:()))"
+ result, warnings = report_schedule.get_native_filters_params()
+ expected = (
+ "(filter_id_1:(extraFormData:(filters:!((col:column_name_1,op:IN,"
+ "val:!(value1,value2)))),filterState:(label:column_name_1,"
+ "validateStatus:!f,value:!(value1,value2)),id:filter_id_1,ownState:()),"
+ "filter_id_2:(extraFormData:(filters:!((col:column_name_2,op:IN,"
+ "val:!(value3,value4)))),filterState:(label:column_name_2,"
+ "validateStatus:!f,value:!(value3,value4)),id:filter_id_2,ownState:()))"
)
+ assert result == expected
+ assert warnings == []
def test_report_generate_native_filter_no_values():
@@ -99,9 +111,10 @@ def test_report_generate_native_filter_no_values():
}
-def test_get_native_filters_params_invalid_structure():
+def test_get_native_filters_params_missing_filter_values():
"""
- Test the ``get_native_filters_params`` method with invalid structure.
+ Test the ``get_native_filters_params`` method with missing filterValues.
+ Should handle gracefully by using empty list as default.
"""
report_schedule = ReportSchedule()
report_schedule.extra = {
@@ -111,29 +124,85 @@ def test_get_native_filters_params_invalid_structure():
"nativeFilterId": "filter_id",
"columnName": "column_name",
"filterType": "filter_select",
- # Missing "filterValues" key
+ # Missing "filterValues" key - should default to []
}
]
}
}
- with pytest.raises(KeyError, match="'filterValues'"):
- report_schedule.get_native_filters_params()
+ # Should not raise, should handle gracefully with empty filterValues
+ result, warnings = report_schedule.get_native_filters_params()
+ assert "filter_id" in result
+ assert "column_name" in result
+ assert warnings == []
-# todo(hugh): how do we want to handle this case?
-# def test_report_generate_native_filter_invalid_filter_id():
-# """
-# Test the ``_generate_native_filter`` method with invalid filter id.
-# """
-# report_schedule = ReportSchedule()
-# native_filter_id = None
-# column_name = "column_name"
-# values = ["value1", "value2"]
+def test_get_native_filters_params_explicit_none_values():
+ """
+ Test the ``get_native_filters_params`` method with explicit None values.
+ Should handle gracefully by coercing None to empty string/list.
+ """
+ report_schedule = ReportSchedule()
+ report_schedule.extra = {
+ "dashboard": {
+ "nativeFilters": [
+ {
+ "nativeFilterId": "filter_id",
+ "columnName": None, # Explicit None
+ "filterType": "filter_select",
+ "filterValues": None, # Explicit None
+ }
+ ]
+ }
+ }
+
+ # Should not raise TypeError, should handle gracefully
+ result, warnings = report_schedule.get_native_filters_params()
+ assert "filter_id" in result
+ assert warnings == []
+
+
+def test_get_native_filters_params_missing_required_fields():
+ """
+ Test the ``get_native_filters_params`` method with missing required fields.
+ Filters missing nativeFilterId or filterType should be skipped.
+ """
+ report_schedule = ReportSchedule()
+ report_schedule.extra = {
+ "dashboard": {
+ "nativeFilters": [
+ {
+ # Missing nativeFilterId - should be skipped
+ "filterType": "filter_select",
+ "columnName": "column_name",
+ "filterValues": ["value1"],
+ },
+ {
+ # Missing filterType - should be skipped
+ "nativeFilterId": "filter_2",
+ "columnName": "column_name",
+ "filterValues": ["value2"],
+ },
+ {
+ # Valid filter - should be processed
+ "nativeFilterId": "filter_3",
+ "filterType": "filter_select",
+ "columnName": "column_name",
+ "filterValues": ["value3"],
+ },
+ ]
+ }
+ }
-# assert report_schedule._generate_native_filter(
-# native_filter_id, column_name, values
-# ) == {}
+ result, warnings = report_schedule.get_native_filters_params()
+ # Only the valid filter should be in the result
+ assert "filter_3" in result
+ assert "filter_2" not in result
+ assert "value1" not in result
+ assert "value3" in result
+ # Two malformed filters should generate two warnings
+ assert len(warnings) == 2
+ assert all("Skipping malformed native filter" in w for w in warnings)
def test_report_generate_native_filter():
@@ -173,7 +242,9 @@ def test_get_native_filters_params_empty():
report_schedule = ReportSchedule()
report_schedule.extra = {}
- assert report_schedule.get_native_filters_params() == "()"
+ result, warnings = report_schedule.get_native_filters_params()
+ assert result == "()"
+ assert warnings == []
def test_get_native_filters_params_no_native_filters():
@@ -183,7 +254,9 @@ def test_get_native_filters_params_no_native_filters():
report_schedule = ReportSchedule()
report_schedule.extra = {"dashboard": {"nativeFilters": []}}
- assert report_schedule.get_native_filters_params() == "()"
+ result, warnings = report_schedule.get_native_filters_params()
+ assert result == "()"
+ assert warnings == []
def test_report_generate_native_filter_empty_values():