From 991bda5dd22d3c2dd501152c58a52dd46b4045d6 Mon Sep 17 00:00:00 2001 From: Rudy Mizrahi Celekli <47457359+rudycelekli@users.noreply.github.com> Date: Sun, 18 Oct 2026 01:19:24 -0400 Subject: [PATCH] fix(salary): reject non-finite workbook numbers (#572) * fix(salary): reject non-finite workbook numbers * docs(changelog): record fix for #572 --- CHANGELOG.md | 2 ++ tests/test_salary_nonfinite_cli.py | 53 ++++++++++++++++++++++++++++++ tools/convert_salary_excel.py | 12 +++++-- 3 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 tests/test_salary_nonfinite_cli.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b7948527..1847804d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -423,6 +423,8 @@ per-file diff commands. harmless, and valid UTF-8 rules decide as before. Nine new tests; the seven that pin the fail-opens fail on master. +- **Salary workbook conversion rejects non-finite text cells** (#572) - hand-typed `Infinity` or `NaN` text cells now produce validation errors instead of numeric-conversion tracebacks. + - **`verify_layout.py` finds an orphaned entry header by where the text starts** (`tools/verify_layout.py`, `tests/test_verify_layout.py`) - the orphan rule compared line left edges against the document margin, and a list marker moves a line's left edge without diff --git a/tests/test_salary_nonfinite_cli.py b/tests/test_salary_nonfinite_cli.py new file mode 100644 index 00000000..34c6a011 --- /dev/null +++ b/tests/test_salary_nonfinite_cli.py @@ -0,0 +1,53 @@ +"""Non-finite workbook cells cannot crash conversion or escape as JSON numbers.""" +import json +import subprocess +import sys +import unittest +from pathlib import Path +from tempfile import TemporaryDirectory + +try: + import openpyxl +except ImportError: + openpyxl = None + +TOOL = Path(__file__).resolve().parent.parent / "tools" / "convert_salary_excel.py" + + +@unittest.skipUnless(openpyxl, "requires optional openpyxl (installed in CI)") +class NonFiniteWorkbookCLI(unittest.TestCase): + def convert(self, headers, rows): + with TemporaryDirectory() as directory: + root = Path(directory) + workbook, output = root / "salary.xlsx", root / "salary.json" + book = openpyxl.Workbook() + try: + book.active.append(headers) + for row in rows: + book.active.append(row) + book.save(workbook) + finally: + book.close() + result = subprocess.run( + [sys.executable, str(TOOL), str(workbook), "--output", str(output)], + capture_output=True, text=True, encoding="utf-8", + ) + self.assertEqual(result.returncode, 0, result.stderr) + def reject_constant(value): + self.fail(f"converter emitted non-JSON numeric constant {value}") + return json.loads(output.read_text(encoding="utf-8"), parse_constant=reject_constant)["companies"] + + def test_infinite_paired_count_does_not_abort_the_workbook(self): + companies = self.convert(["Company", "Count", "Index"], [ + ["Invalid Count", "Infinity", 108.5], ["Valid Control", 12, 102.5] + ]) + self.assertEqual(companies[0]["categories"], {"all_employees": {"count": None, "index": 108.5}}) + self.assertEqual(companies[1]["categories"], {"all_employees": {"count": 12, "index": 102.5}}) + + def test_nonfinite_paired_index_is_skipped_instead_of_emitted_as_nan(self): + companies = self.convert(["Company", "Count", "Index"], [["Acme", 12, "NaN"]]) + self.assertEqual(companies[0]["categories"], {"all_employees": {"count": 12, "index": None}}) + + def test_nonfinite_standalone_count_is_skipped(self): + companies = self.convert(["Company", "Count"], [["Acme", "NaN"]]) + self.assertEqual(companies[0]["categories"], {}) diff --git a/tools/convert_salary_excel.py b/tools/convert_salary_excel.py index fa01414b..3d4822cf 100644 --- a/tools/convert_salary_excel.py +++ b/tools/convert_salary_excel.py @@ -26,6 +26,7 @@ with paired count/index columns per category, it groups them automatically. """ import json +import math import sys import argparse import re @@ -58,10 +59,17 @@ ID_PATTERNS = {"id", "personnummer"} DEFAULT_CATEGORY = "all_employees" +def _finite_float(value): + number = float(value) + if not math.isfinite(number): + raise ValueError("salary values must be finite") + return number + + def parse_numeric_cell(value): """Parse numeric Excel values, including localized string cells.""" if isinstance(value, (int, float)): - return float(value) + return _finite_float(value) if not isinstance(value, str): raise ValueError("not numeric") @@ -84,7 +92,7 @@ def parse_numeric_cell(value): elif "." in text: if re.fullmatch(r"[+-]?\d+\.\d{3}", text): raise ValueError("ambiguous dot separator") - return float(text) + return _finite_float(text) def header_matches(header, patterns):