You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/06/15 16:59:47 UTC

(superset) branch master updated: fix(reports): escape LIKE wildcards in text filter and preserve typed screenshot width (#40980)

This is an automated email from the ASF dual-hosted git repository.

sadpandajoe pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new b656b1d4770 fix(reports): escape LIKE wildcards in text filter and preserve typed screenshot width (#40980)
b656b1d4770 is described below

commit b656b1d4770d474345aaad13d6560a97ae4a9e3a
Author: Joe Li <jo...@preset.io>
AuthorDate: Mon Jun 15 09:59:40 2026 -0700

    fix(reports): escape LIKE wildcards in text filter and preserve typed screenshot width (#40980)
    
    Co-authored-by: Claude Opus 4.8 (1M context) <no...@anthropic.com>
---
 .../reports/ReportModal/ReportModal.test.tsx       | 25 ++++++++++++
 .../src/features/reports/ReportModal/index.tsx     |  5 ++-
 superset/reports/filters.py                        | 11 ++++--
 tests/unit_tests/commands/report/update_test.py    | 37 ++++++++++++++++++
 tests/unit_tests/daos/test_report_dao.py           | 37 ++++++++++++++++++
 tests/unit_tests/reports/filters_test.py           | 44 ++++++++++++++++++++++
 6 files changed, 153 insertions(+), 6 deletions(-)

diff --git a/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx b/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx
index 80cecb18c56..c7c370a6449 100644
--- a/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx
+++ b/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx
@@ -168,6 +168,31 @@ test('non-text chart shows screenshot width and message content', () => {
   expect(screen.getByText('Screenshot width')).toBeInTheDocument();
 });
 
+test('screenshot width input preserves a typed zero instead of dropping it', () => {
+  const lineChartProps = {
+    ...defaultProps,
+    dashboardId: undefined,
+    chart: { id: 1, sliceFormData: { viz_type: VizType.Line } },
+    chartName: 'My Line Chart',
+    creationMethod: 'charts' as const,
+  };
+  render(<ReportModal {...lineChartProps} />, { useRedux: true });
+
+  const widthInput = screen.getByPlaceholderText(
+    'Input custom width in pixels',
+  );
+
+  // The old `|| null` / `|| ''` logic silently coerced a typed 0 to null, so the
+  // invalid width was swallowed instead of being submitted and surfaced by the
+  // server's min-width validation. The field must preserve the literal value.
+  userEvent.type(widthInput, '0');
+  expect(widthInput).toHaveDisplayValue('0');
+
+  // Clearing the field still yields an empty value (parsed NaN → null).
+  userEvent.clear(widthInput);
+  expect(widthInput).toHaveDisplayValue('');
+});
+
 test('dashboard report hides message content section', () => {
   const dashboardProps = {
     ...defaultProps,
diff --git a/superset-frontend/src/features/reports/ReportModal/index.tsx b/superset-frontend/src/features/reports/ReportModal/index.tsx
index 05582574fab..078e2a8e35c 100644
--- a/superset-frontend/src/features/reports/ReportModal/index.tsx
+++ b/superset-frontend/src/features/reports/ReportModal/index.tsx
@@ -296,11 +296,12 @@ function ReportModal({
         <Input
           type="number"
           name="custom_width"
-          value={currentReport?.custom_width || ''}
+          value={currentReport?.custom_width ?? ''}
           placeholder={t('Input custom width in pixels')}
           onChange={(event: ChangeEvent<HTMLInputElement>) => {
+            const parsedWidth = parseInt(event.target.value, 10);
             setCurrentReport({
-              custom_width: parseInt(event.target.value, 10) || null,
+              custom_width: Number.isNaN(parsedWidth) ? null : parsedWidth,
             });
           }}
         />
diff --git a/superset/reports/filters.py b/superset/reports/filters.py
index a03238b6408..c064c9fa056 100644
--- a/superset/reports/filters.py
+++ b/superset/reports/filters.py
@@ -21,6 +21,7 @@ from sqlalchemy import or_
 from sqlalchemy.orm.query import Query
 
 from superset import db, security_manager
+from superset.daos.base import _escape_like
 from superset.reports.models import ReportSchedule
 from superset.views.base import BaseFilter
 
@@ -47,11 +48,13 @@ class ReportScheduleAllTextFilter(BaseFilter):  # pylint: disable=too-few-public
     def apply(self, query: Query, value: Any) -> Query:
         if not value:
             return query
-        ilike_value = f"%{value}%"
+        # ``value`` may arrive as a non-string (e.g. an int in the API ``filters``
+        # array); coerce it so escaping never raises on ``.replace``.
+        ilike_value = f"%{_escape_like(str(value))}%"
         return query.filter(
             or_(
-                ReportSchedule.name.ilike(ilike_value),
-                ReportSchedule.description.ilike(ilike_value),
-                ReportSchedule.sql.ilike(ilike_value),
+                ReportSchedule.name.ilike(ilike_value, escape="\\"),
+                ReportSchedule.description.ilike(ilike_value, escape="\\"),
+                ReportSchedule.sql.ilike(ilike_value, escape="\\"),
             )
         )
diff --git a/tests/unit_tests/commands/report/update_test.py b/tests/unit_tests/commands/report/update_test.py
index 19612347dda..3d0ca80ac2d 100644
--- a/tests/unit_tests/commands/report/update_test.py
+++ b/tests/unit_tests/commands/report/update_test.py
@@ -447,6 +447,43 @@ def test_ownership_check_raises_forbidden(mocker: MockerFixture) -> None:
         cmd.validate()
 
 
+# --- Dashboard extra (activeTabs) validation on update ---
+
+
+def test_update_rejects_invalid_active_tab_ids(mocker: MockerFixture) -> None:
+    """On PUT, activeTabs must be validated against the model's dashboard layout.
+
+    The dashboard is not in the payload, so validation must fall back to the
+    existing model's dashboard; tab ids absent from position_json are rejected.
+    """
+    model = _make_model(mocker, model_type=ReportScheduleType.REPORT, database_id=None)
+    model.dashboard.position_json = '{"TAB-valid": {}}'
+    _setup_mocks(mocker, model)
+
+    cmd = UpdateReportScheduleCommand(
+        model_id=1,
+        data={"extra": {"dashboard": {"activeTabs": ["TAB-missing"]}}},
+    )
+    with pytest.raises(ReportScheduleInvalidError) as exc_info:
+        cmd.validate()
+    messages = _get_validation_messages(exc_info)
+    assert "extra" in messages
+    assert "invalid tab ids" in messages["extra"].lower()
+
+
+def test_update_accepts_valid_active_tab_ids(mocker: MockerFixture) -> None:
+    """A tab id present in the model dashboard's position_json passes validation."""
+    model = _make_model(mocker, model_type=ReportScheduleType.REPORT, database_id=None)
+    model.dashboard.position_json = '{"TAB-valid": {}}'
+    _setup_mocks(mocker, model)
+
+    cmd = UpdateReportScheduleCommand(
+        model_id=1,
+        data={"extra": {"dashboard": {"activeTabs": ["TAB-valid"]}}},
+    )
+    cmd.validate()  # should not raise
+
+
 # --- Database not found for alert ---
 
 
diff --git a/tests/unit_tests/daos/test_report_dao.py b/tests/unit_tests/daos/test_report_dao.py
index 55d089d2226..3fa883ce00e 100644
--- a/tests/unit_tests/daos/test_report_dao.py
+++ b/tests/unit_tests/daos/test_report_dao.py
@@ -90,3 +90,40 @@ def test_find_by_extra_metadata_escapes_underscore_wildcard(
 
     assert len(results) == 1
     assert results[0].name == "with-underscore"
+
+
+def test_find_by_native_filter_id_returns_matching_reports(
+    session: Session,
+) -> None:
+    extra = json.dumps({"dashboard": {"nativeFilters": "NATIVE_FILTER-abc123"}})
+    _create_report(session, "match", extra_json=extra)
+    _create_report(session, "no-match", extra_json="{}")
+
+    results = ReportScheduleDAO.find_by_native_filter_id("NATIVE_FILTER-abc123")
+
+    assert len(results) == 1
+    assert results[0].name == "match"
+
+
+def test_find_by_native_filter_id_escapes_percent_wildcard(
+    session: Session,
+) -> None:
+    _create_report(session, "with-percent", extra_json='{"id": "FILTER-100%x"}')
+    _create_report(session, "other", extra_json='{"id": "FILTER-100yx"}')
+
+    results = ReportScheduleDAO.find_by_native_filter_id("FILTER-100%x")
+
+    assert len(results) == 1
+    assert results[0].name == "with-percent"
+
+
+def test_find_by_native_filter_id_escapes_underscore_wildcard(
+    session: Session,
+) -> None:
+    _create_report(session, "with-underscore", extra_json='{"id": "FILTER-a_b"}')
+    _create_report(session, "other", extra_json='{"id": "FILTER-axb"}')
+
+    results = ReportScheduleDAO.find_by_native_filter_id("FILTER-a_b")
+
+    assert len(results) == 1
+    assert results[0].name == "with-underscore"
diff --git a/tests/unit_tests/reports/filters_test.py b/tests/unit_tests/reports/filters_test.py
index 308eb439136..bdcb2c181fb 100644
--- a/tests/unit_tests/reports/filters_test.py
+++ b/tests/unit_tests/reports/filters_test.py
@@ -62,3 +62,47 @@ def test_report_schedule_all_text_filter_applies_ilike() -> None:
     f = ReportScheduleAllTextFilter("name", MagicMock())
     f.apply(query, "test")
     query.filter.assert_called_once()
+
+
+@patch("superset.reports.filters.or_")
+@patch("superset.reports.filters.ReportSchedule")
+def test_report_schedule_all_text_filter_escapes_wildcards(
+    mock_report_schedule: MagicMock, mock_or: MagicMock
+) -> None:
+    """User-supplied wildcards must be escaped so they match literally."""
+    from superset.reports.filters import ReportScheduleAllTextFilter
+
+    query = MagicMock()
+    f = ReportScheduleAllTextFilter("name", MagicMock())
+    # raw input contains every LIKE special character plus a backslash
+    f.apply(query, "50%_off\\promo")
+
+    # %, _ and \ are all escaped, and the literal is wrapped for a "contains" match
+    expected = "%50\\%\\_off\\\\promo%"
+    for column in (
+        mock_report_schedule.name,
+        mock_report_schedule.description,
+        mock_report_schedule.sql,
+    ):
+        column.ilike.assert_called_once_with(expected, escape="\\")
+
+
+@patch("superset.reports.filters.or_")
+@patch("superset.reports.filters.ReportSchedule")
+def test_report_schedule_all_text_filter_coerces_non_string(
+    mock_report_schedule: MagicMock, mock_or: MagicMock
+) -> None:
+    """A non-string value (e.g. an int) must not raise when escaping."""
+    from superset.reports.filters import ReportScheduleAllTextFilter
+
+    query = MagicMock()
+    f = ReportScheduleAllTextFilter("name", MagicMock())
+    f.apply(query, 50)
+
+    expected = "%50%"
+    for column in (
+        mock_report_schedule.name,
+        mock_report_schedule.description,
+        mock_report_schedule.sql,
+    ):
+        column.ilike.assert_called_once_with(expected, escape="\\")