fix: close latent crash paths for malformed physical_index values

Review follow-ups. Removing the delete-based validators re-exposed two
crash paths that predate the sanitization PR and were masked by it (at
the cost of silently deleted sections):

- convert_physical_index_to_int crashed on malformed markers
  ("<physical_index_x>" -> int('x>') ValueError) and passed bare
  numeric strings through, which later raised TypeError at the
  validate_and_truncate comparison. Marker tails and bare numeric
  strings now parse to int; anything unparseable is left as-is for
  validation to nullify. Single-value mode now accepts bare "7"
  (previously None).

- validate_and_truncate_physical_indices only checked the upper bound.
  Below-start_index values (possible when process_large_node_recursively
  runs with start_index > 1) reached page_list[negative] in verify_toc
  and could raise IndexError. Non-int and out-of-range values are now
  nullified — this runs after the None-entry filter, so nullified
  entries survive as placeholders on the existing None-tolerant paths.

- _parse_physical_index accepted bools (True -> page 1), truncated
  non-integral floats (1.9 -> page 1), and raised uncaught
  OverflowError on infinite floats. It now rejects all three.

Author: rejojer

pageindex/page_index.py

@@ -12,14 +12,16 @@
 _PHYSICAL_INDEX_MARKER_RE = re.compile(r"^<physical_index_(\d+)>$")
 
 def _parse_physical_index(raw):
-    if raw is None:
+    if raw is None or isinstance(raw, bool):
         return None
     marker_match = _PHYSICAL_INDEX_MARKER_RE.match(str(raw).strip())
     if marker_match:
         return int(marker_match.group(1))
+    if isinstance(raw, float) and not raw.is_integer():
+        return None
     try:
         return int(raw)
-    except (TypeError, ValueError):
+    except (TypeError, ValueError, OverflowError):
         return None
 
 
@@ -1188,14 +1190,15 @@ def validate_and_truncate_physical_indices(toc_with_page_number, page_list_lengt
     for i, item in enumerate(toc_with_page_number):
         if item.get('physical_index') is not None:
             original_index = item['physical_index']
-            if original_index > max_allowed_page:
+            if (not isinstance(original_index, int) or isinstance(original_index, bool)
+                    or not (start_index <= original_index <= max_allowed_page)):
                 item['physical_index'] = None
                 truncated_items.append({
                     'title': item.get('title', 'Unknown'),
                     'original_index': original_index
                 })
                 if logger:
-                    logger.info(f"Removed physical_index for '{item.get('title', 'Unknown')}' (was {original_index}, too far beyond document)")
+                    logger.info(f"Removed physical_index for '{item.get('title', 'Unknown')}' (was {original_index}, outside the document range)")
 
     if truncated_items and logger:
         logger.info(f"Total removed items: {len(truncated_items)}")

pageindex/utils.py

@@ -580,19 +580,24 @@ def convert_physical_index_to_int(data):
             # Check if item is a dictionary and has 'physical_index' key
             if isinstance(data[i], dict) and 'physical_index' in data[i]:
                 if isinstance(data[i]['physical_index'], str):
-                    if data[i]['physical_index'].startswith('<physical_index_'):
-                        data[i]['physical_index'] = int(data[i]['physical_index'].split('_')[-1].rstrip('>').strip())
-                    elif data[i]['physical_index'].startswith('physical_index_'):
-                        data[i]['physical_index'] = int(data[i]['physical_index'].split('_')[-1].strip())
+                    value = data[i]['physical_index']
+                    if value.startswith('<physical_index_'):
+                        value = value.split('_')[-1].rstrip('>').strip()
+                    elif value.startswith('physical_index_'):
+                        value = value.split('_')[-1].strip()
+                    try:
+                        data[i]['physical_index'] = int(value)
+                    except ValueError:
+                        pass
     elif isinstance(data, str):
-        if data.startswith('<physical_index_'):
-            data = int(data.split('_')[-1].rstrip('>').strip())
-        elif data.startswith('physical_index_'):
-            data = int(data.split('_')[-1].strip())
-        # Check data is int
-        if isinstance(data, int):
-            return data
-        else:
+        value = data
+        if value.startswith('<physical_index_'):
+            value = value.split('_')[-1].rstrip('>').strip()
+        elif value.startswith('physical_index_'):
+            value = value.split('_')[-1].strip()
+        try:
+            return int(value)
+        except ValueError:
             return None
     return data
 

tests/test_page_index.py

@@ -1,7 +1,53 @@
 import unittest
 from unittest.mock import Mock, patch
 
-from pageindex.page_index import process_toc_no_page_numbers
+from pageindex.page_index import (
+    _parse_physical_index,
+    process_toc_no_page_numbers,
+    validate_and_truncate_physical_indices,
+)
+from pageindex.utils import convert_physical_index_to_int
+
+
+class PhysicalIndexGuardsTest(unittest.TestCase):
+    def test_parse_rejects_non_integral_values(self):
+        self.assertEqual(_parse_physical_index("<physical_index_7>"), 7)
+        self.assertEqual(_parse_physical_index("7"), 7)
+        self.assertEqual(_parse_physical_index(7.0), 7)
+        self.assertIsNone(_parse_physical_index(True))
+        self.assertIsNone(_parse_physical_index(1.9))
+        self.assertIsNone(_parse_physical_index(float("inf")))
+        self.assertIsNone(_parse_physical_index(float("nan")))
+
+    def test_convert_handles_bare_and_malformed_strings(self):
+        data = [
+            {"physical_index": "1"},
+            {"physical_index": "<physical_index_2>"},
+            {"physical_index": "<physical_index_x>"},
+            {"physical_index": "abc"},
+        ]
+        convert_physical_index_to_int(data)
+        self.assertEqual(data[0]["physical_index"], 1)
+        self.assertEqual(data[1]["physical_index"], 2)
+        self.assertEqual(data[2]["physical_index"], "<physical_index_x>")
+        self.assertEqual(data[3]["physical_index"], "abc")
+        self.assertEqual(convert_physical_index_to_int("7"), 7)
+        self.assertIsNone(convert_physical_index_to_int("<physical_index_x>"))
+
+    def test_truncate_nullifies_out_of_range_and_non_int(self):
+        toc = [
+            {"physical_index": 1},
+            {"physical_index": 5},
+            {"physical_index": 99},
+            {"physical_index": "abc"},
+            {"physical_index": 1.9},
+        ]
+        validate_and_truncate_physical_indices(toc, 10, start_index=5)
+        self.assertIsNone(toc[0]["physical_index"])
+        self.assertEqual(toc[1]["physical_index"], 5)
+        self.assertIsNone(toc[2]["physical_index"])
+        self.assertIsNone(toc[3]["physical_index"])
+        self.assertIsNone(toc[4]["physical_index"])
 
 
 class ProcessTocNoPageNumbersTest(unittest.TestCase):