Skip to content

BUG: Fix PageRange.__add__ crashing with TypeError when start or stop is None#3898

Open
gaoflow wants to merge 2 commits into
py-pdf:mainfrom
gaoflow:fix-pagerange-add-none-bounds
Open

BUG: Fix PageRange.__add__ crashing with TypeError when start or stop is None#3898
gaoflow wants to merge 2 commits into
py-pdf:mainfrom
gaoflow:fix-pagerange-add-none-bounds

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Problem

PageRange.__add__ raises TypeError: '>' not supported between instances of 'int' and 'NoneType' (or NoneType and int, or NoneType and NoneType) whenever either operand has a None start or None stop, which is perfectly normal for ranges like "5:" or ":3".

>>> from pypdf.pagerange import PageRange
>>> PageRange("0:5") + PageRange("3:")   # stop is None
TypeError: '>' not supported between instances of 'NoneType' and 'int'

>>> PageRange(":5") + PageRange(":3")    # both starts are None
TypeError: '>' not supported between instances of 'NoneType' and 'NoneType'

The problem is that __add__ compares slice.start and slice.stop with > directly. None bounds are not comparable to integers, but they are semantically well-defined: None start = "beginning of sequence" and None stop = "end of sequence".

Fix

Add two small helper functions inside __add__ that map None to float sentinels before any comparison:

  • _start_key(None)-inf (beginning sorts before any page index)
  • _stop_key(None)+inf (end sorts after any page index)

Replace the direct comparisons and max() call with these key functions. No public API changes.

Behaviour after fix

>>> PageRange("0:5") + PageRange("3:")   # => PageRange("0:")
>>> PageRange(":5")  + PageRange(":3")   # => PageRange(":5")
>>> PageRange("5:")  + PageRange(":3")   # => ValueError: gap (correct)
>>> PageRange(":")   + PageRange("2:7")  # => PageRange(":")

All 23 existing tests continue to pass.


This pull request was prepared with the assistance of AI, under my direction and review.

… is None

PageRange.__add__ compared slice bounds with `>` directly, which raises
TypeError when either bound is None. Slices with None start (":5") or
None stop ("5:") are perfectly valid and common, so adding two such
ranges could crash instead of returning the correct union.

Fix: introduce two small key functions that map None start to -inf and
None stop to +inf before all comparisons and the max-stop selection.
All existing tests continue to pass; new cases like

    PageRange("0:5") + PageRange("3:")  # => PageRange("0:")
    PageRange(":5")  + PageRange(":3")  # => PageRange(":5")

now work correctly.
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.86%. Comparing base (75e339c) to head (3a1e152).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3898      +/-   ##
==========================================
+ Coverage   97.85%   97.86%   +0.01%     
==========================================
  Files          57       57              
  Lines       10702    10743      +41     
  Branches     2001     2006       +5     
==========================================
+ Hits        10472    10514      +42     
  Misses        127      127              
+ Partials      103      102       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread pypdf/pagerange.py Outdated
def parse_filename_page_ranges(
args: list[Union[str, PageRange, None]]
) -> list[tuple[str, PageRange]]:
def parse_filename_page_ranges(args: list[Union[str, PageRange, None]]) -> list[tuple[str, PageRange]]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid unnecessary changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restored in 3a1e152a. Verification passed:

python -m pytest tests/test_pagerange.py -q
python -m ruff check pypdf/pagerange.py tests/test_pagerange.py

Comment thread pypdf/pagerange.py Outdated
raise ValueError(
"The first argument must be a filename, not a page range."
)
raise ValueError("The first argument must be a filename, not a page range.")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid unnecessary changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restored in 3a1e152a. Verification passed:

python -m pytest tests/test_pagerange.py -q
python -m ruff check pypdf/pagerange.py tests/test_pagerange.py

@gaoflow

gaoflow commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Reverted both unrelated reformatting changes (the parse_filename_page_ranges signature and the raise ValueError line) in 3a1e152 — the diff is now limited to the __add__ None-bounds fix. Ready for another look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants