fix(dashboard): address Excel export review feedback

Addresses PR #41133 review comments:
- Blank/stringify non-finite and out-of-range Decimal cells instead of
  crashing xlsxwriter (shared _coerce_float_cell helper).
- Detect formula-injection prefixes behind leading whitespace via
  _quote_if_formula (lstrip before checking).
- Correct the module docstring to scope the constant-memory guarantee to
  the writer side (source rows may be materialized upstream).
- Append a "[Truncated: ...]" notice row when a sheet exceeds Excel's
  per-sheet row cap so dropped rows are visible.
- Note AWS S3's 7-day pre-signed URL cap on EXCEL_EXPORT_LINK_TTL_SECONDS.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: hughhhh

superset/config.py

@@ -1378,6 +1378,8 @@ def sync_theme_logo_href(
 # Key prefix for export objects: {prefix}{dashboard_id}/{job_id}.xlsx
 EXCEL_EXPORT_S3_KEY_PREFIX = "dashboard-exports/"
 # Lifetime (seconds) of the pre-signed download URL emailed to the user (24h).
+# Note: AWS S3 caps pre-signed URL lifetime at 7 days (604800 seconds); larger
+# values are rejected by S3, so keep this at or below that when using AWS.
 EXCEL_EXPORT_LINK_TTL_SECONDS = 86400
 # Extra kwargs passed to boto3.client("s3", ...) — e.g. region_name, or an
 # endpoint_url for S3-compatible stores (MinIO/LocalStack). Credentials

superset/utils/excel_streaming.py

@@ -15,12 +15,14 @@
 # specific language governing permissions and limitations
 # under the License.
 """
-Streaming, constant-memory XLSX writer for multi-sheet dashboard exports.
-
-Unlike :mod:`superset.utils.excel`, which materializes a whole DataFrame in
-memory, this writer streams rows one at a time into an ``xlsxwriter`` workbook
-opened in ``constant_memory`` mode, so a dashboard with many large charts never
-holds more than one row per sheet in memory at once.
+Streaming XLSX writer for multi-sheet dashboard exports.
+
+Unlike :mod:`superset.utils.excel`, which builds an in-memory DataFrame per
+sheet and hands the whole thing to ``xlsxwriter`` at once, this writer opens the
+workbook in ``constant_memory`` mode and writes rows one at a time, so
+``xlsxwriter`` keeps at most one row per sheet buffered on the writer side. The
+source records may still be materialized upstream (e.g. by the chart query
+response); this bounds only the writer's own footprint, not the caller's.
 """
 
 from __future__ import annotations
@@ -54,6 +56,37 @@
 _MAX_EXCEL_INT = 10**15
 
 
+def _quote_if_formula(text: str) -> str:
+    """
+    Prefix formula-like text with an apostrophe so spreadsheet apps treat it as
+    literal text (defense against formula injection).
+
+    Leading whitespace is ignored when detecting a formula, because spreadsheet
+    apps still evaluate a cell whose formula prefix is preceded by spaces or
+    tabs (e.g. ``" =cmd"`` or ``"\\t=cmd"``).
+    """
+    stripped = text.lstrip()
+    return f"'{text}" if stripped and stripped[0] in _FORMULA_PREFIXES else text
+
+
+def _coerce_float_cell(value: Any) -> Any:
+    """
+    Convert a ``Decimal``/real value to something ``xlsxwriter`` accepts.
+
+    ``float()`` on a non-finite ``Decimal`` ("NaN"/"Infinity") yields a value
+    xlsxwriter rejects, and an over-large value can raise ``OverflowError``;
+    blank the former and stringify the latter, and stringify magnitudes Excel
+    cannot represent precisely.
+    """
+    try:
+        number = float(value)
+    except (OverflowError, ValueError):
+        return str(value)
+    if not math.isfinite(number):
+        return ""
+    return str(number) if abs(number) > _MAX_EXCEL_INT else number
+
+
 def sanitize_sheet_name(raw: str, used: set[str]) -> str:
     """
     Produce a valid, unique Excel sheet name from ``raw``.
@@ -104,24 +137,19 @@ def _sanitize_cell(value: Any) -> Any:
     if isinstance(value, bool):
         return value
     if isinstance(value, str):
-        return f"'{value}" if value and value[0] in _FORMULA_PREFIXES else value
+        return _quote_if_formula(value)
     if isinstance(value, (datetime, date)):
         return value.isoformat()
     if isinstance(value, Decimal):
-        number = float(value)
-        return str(number) if abs(number) > _MAX_EXCEL_INT else number
+        return _coerce_float_cell(value)
     if isinstance(value, numbers.Integral):
         number = int(value)
         return str(number) if abs(number) > _MAX_EXCEL_INT else number
     if isinstance(value, numbers.Real):
-        number = float(value)
-        if not math.isfinite(number):
-            return ""
-        return str(number) if abs(number) > _MAX_EXCEL_INT else number
+        return _coerce_float_cell(value)
     # Anything else (lists, dicts, custom objects) is stringified, still guarding
     # against formula injection on the resulting text.
-    text = str(value)
-    return f"'{text}" if text and text[0] in _FORMULA_PREFIXES else text
+    return _quote_if_formula(str(value))
 
 
 class StreamingXlsxWriter:
@@ -153,20 +181,34 @@ def add_sheet(
         :param name: Desired sheet name (sanitized/de-duplicated automatically)
         :param columns: Column headers
         :param rows: Iterable of row sequences, streamed one at a time
-        :returns: The number of data rows actually written (capped at Excel's
-            per-sheet limit)
+        :returns: The number of data rows actually written (capped just below
+            Excel's per-sheet limit; when the data is larger a final notice row
+            is appended and the dropped rows are not counted)
         """
         sheet_name = sanitize_sheet_name(name, self._used_sheet_names)
         worksheet = self._workbook.add_worksheet(sheet_name)
         worksheet.write_row(0, 0, [_sanitize_cell(col) for col in columns])
 
+        # Reserve the final row for a truncation notice, so when the data
+        # exceeds the sheet's capacity the user can see rows were dropped
+        # instead of silently losing them.
+        row_cap = MAX_DATA_ROWS_PER_SHEET - 1
         written = 0
+        truncated = False
         for row in rows:
-            if written >= MAX_DATA_ROWS_PER_SHEET:
+            if written >= row_cap:
+                truncated = True
                 break
             worksheet.write_row(written + 1, 0, [_sanitize_cell(cell) for cell in row])
             written += 1
 
+        if truncated:
+            worksheet.write_string(
+                written + 1,
+                0,
+                f"[Truncated: only first {written:,} rows exported]",
+            )
+
         self.sheet_count += 1
         return written
 

tests/unit_tests/utils/excel_streaming_tests.py

@@ -99,6 +99,38 @@ def test_sanitize_cell_non_finite_floats_blanked() -> None:
     assert _sanitize_cell(float("inf")) == ""
 
 
+def test_sanitize_cell_non_finite_decimals_blanked() -> None:
+    # float(Decimal("NaN")) is nan and float(Decimal("Infinity")) is inf, both
+    # of which xlsxwriter rejects; they must be blanked rather than crash.
+    assert _sanitize_cell(Decimal("NaN")) == ""
+    assert _sanitize_cell(Decimal("Infinity")) == ""
+    assert _sanitize_cell(Decimal("-Infinity")) == ""
+
+
+def test_sanitize_cell_out_of_range_decimal_is_blanked() -> None:
+    # A Decimal too large for a float becomes inf (or, in edge cases, raises
+    # OverflowError); either way it must be neutralized rather than crash
+    # xlsxwriter or emit a bogus value.
+    assert _sanitize_cell(Decimal("1E10000")) == ""
+
+
+@pytest.mark.parametrize(
+    "value,expected",
+    [
+        (" =SUM(A1)", "' =SUM(A1)"),
+        ("\t=SUM(A1)", "'\t=SUM(A1)"),
+        ("  +1", "'  +1"),
+        ("\t@handle", "'\t@handle"),
+    ],
+)
+def test_sanitize_cell_quotes_formula_behind_whitespace(
+    value: str, expected: str
+) -> None:
+    # Spreadsheet apps evaluate formulas even when preceded by spaces/tabs, so
+    # the formula guard must look past leading whitespace.
+    assert _sanitize_cell(value) == expected
+
+
 # --- StreamingXlsxWriter (round-trip via openpyxl) ---
 
 
@@ -139,16 +171,34 @@ def test_writer_quotes_formula_cells(tmp_path: Path) -> None:
 def test_writer_caps_rows_per_sheet(
     tmp_path: Path, monkeypatch: pytest.MonkeyPatch
 ) -> None:
-    monkeypatch.setattr(excel_streaming, "MAX_DATA_ROWS_PER_SHEET", 2)
+    monkeypatch.setattr(excel_streaming, "MAX_DATA_ROWS_PER_SHEET", 3)
     path = str(tmp_path / "out.xlsx")
     writer = StreamingXlsxWriter(path)
     written = writer.add_sheet("data", ["col"], [[i] for i in range(5)])
     writer.close()
 
+    # One row is reserved for the truncation notice, so only 2 data rows fit.
+    assert written == 2
+    sheets = _read_workbook(path)
+    # header + 2 data rows + 1 truncation notice
+    assert len(sheets["data"]) == 4
+    assert sheets["data"][-1][0] == "[Truncated: only first 2 rows exported]"
+
+
+def test_writer_no_truncation_notice_when_data_fits(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    monkeypatch.setattr(excel_streaming, "MAX_DATA_ROWS_PER_SHEET", 3)
+    path = str(tmp_path / "out.xlsx")
+    writer = StreamingXlsxWriter(path)
+    # Exactly fills the reserved capacity (MAX - 1) with no leftover rows.
+    written = writer.add_sheet("data", ["col"], [[i] for i in range(2)])
+    writer.close()
+
     assert written == 2
     sheets = _read_workbook(path)
-    # header + 2 data rows
-    assert len(sheets["data"]) == 3
+    # header + 2 data rows, no notice
+    assert sheets["data"] == [["col"], [0], [1]]
 
 
 def test_writer_empty_workbook_is_valid(tmp_path: Path) -> None: