Strip Excel-illegal control characters from inventory report cells (PP-5246)

src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py

@@ -14,6 +14,7 @@
 from celery import shared_task
 from openpyxl import Workbook
 from openpyxl.cell import WriteOnlyCell
+from openpyxl.cell.cell import ILLEGAL_CHARACTERS_RE
 from openpyxl.styles.numbers import FORMAT_TEXT
 from sqlalchemy import (
     bindparam,
@@ -268,21 +269,40 @@ def _stringify_cell_value(value: Any) -> str:
     return str(value)
 
 
+def _sanitize_cell_value(value: str) -> str:
+    """Strip characters that Excel does not permit in worksheet cells.
+
+    Some imported metadata contains stray control characters (e.g. a U+001F unit
+    separator embedded in a contributor name). openpyxl rejects these with an
+    ``IllegalCharacterError``, which would fail the whole report, so we remove
+    them. The same regex openpyxl validates with is used here, so anything that
+    survives is guaranteed to be writable. Tab, newline and carriage return are
+    not matched by that regex and are therefore preserved.
+    """
+    return ILLEGAL_CHARACTERS_RE.sub("", value)
+
+
 def _cell_value(key: str, value: Any, stringify_cols: frozenset[str]) -> Any:
     """Convert a cell value for report output (shared by CSV and Excel writers).
 
     For columns in stringify_cols, forces the value to a plain string.
     Enum values are converted using their .value attribute.
     Timezone-aware datetimes are formatted as strings for Excel compatibility.
+    String values from the database are stripped of characters that Excel
+    disallows, so that the CSV and Excel outputs stay identical.
     """
     if key in stringify_cols:
-        return _stringify_cell_value(value)
+        return _sanitize_cell_value(_stringify_cell_value(value))
     if value is None:
         return ""
     if isinstance(value, enum.Enum):
+        # Enum values are declared in our own source, so they can't carry
+        # illegal characters and don't need sanitizing.
         return str(value.value)
     if isinstance(value, datetime) and value.tzinfo is not None:
         return value.strftime("%Y-%m-%d %H:%M:%S.%f")
+    if isinstance(value, str):
+        return _sanitize_cell_value(value)
     return value
 
 

tests/manager/celery/tasks/test_generate_inventory_and_hold_reports.py

@@ -277,6 +277,79 @@ def test_generate_csv_report_quotes_target_age(
     assert '"5-8"' in csv_content
 
 
+def test_generate_excel_report_strips_illegal_characters(
+    db: DatabaseTransactionFixture,
+):
+    """Control characters in the data don't blow up Excel generation.
+
+    Imported metadata sometimes carries stray control characters (e.g. a U+001F
+    unit separator inside a contributor name). openpyxl refuses to write them,
+    so they are stripped rather than failing the whole report.
+    """
+    query = select(
+        literal("Ja\x1fne Doe").label("author"),
+        literal("Ti\x0btle").label("title"),
+        literal("line1\nline2\ttabbed").label("notes"),
+        literal("97803064\x1f06157").label("identifier"),
+    )
+
+    excel_file = io.BytesIO()
+    excel_file.name = "test_report.xlsx"
+
+    generate_excel_report(
+        db=db.session,
+        excel_file=excel_file,
+        sql_params={},
+        query=query,
+        columns_to_stringify={"identifier"},
+    )
+
+    excel_file.seek(0)
+    wb = load_workbook(io.BytesIO(excel_file.getvalue()))
+    ws = wb.active
+    assert ws is not None
+    headers = [ws.cell(row=1, column=c).value for c in range(1, ws.max_column + 1)]
+    values = {
+        header: ws.cell(row=2, column=index + 1).value
+        for index, header in enumerate(headers)
+    }
+    assert values["author"] == "Jane Doe"
+    # Stringified columns go through a separate branch of the sanitizer.
+    assert values["identifier"] == "9780306406157"
+    assert values["title"] == "Title"
+    # Tab, newline and carriage return are legal and must be preserved.
+    assert values["notes"] == "line1\nline2\ttabbed"
+
+
+def test_generate_csv_report_strips_illegal_characters(
+    db: DatabaseTransactionFixture,
+):
+    """The CSV output is sanitized identically to the Excel output.
+
+    ``identifier`` is a stringified column, so it exercises the
+    ``_stringify_cell_value`` branch of the sanitizer as well.
+    """
+    query = select(
+        literal("Ja\x1fne Doe").label("author"),
+        literal("97803064\x1f06157").label("identifier"),
+    )
+
+    csv_file = io.StringIO()
+    csv_file.name = "test_report.csv"
+
+    generate_csv_report(
+        db=db.session,
+        csv_file=csv_file,
+        sql_params={},
+        query=query,
+        columns_to_stringify={"identifier"},
+    )
+
+    csv_file.seek(0)
+    rows = list(csv.reader(io.StringIO(csv_file.getvalue())))
+    assert rows[1] == ["Jane Doe", "9780306406157"]
+
+
 def test_only_active_collections_are_included(
     db: DatabaseTransactionFixture, services_fixture: ServicesFixture
 ):