Fix iteration over corrupt and unusual search trees by oschwald · Pull Request #469 · maxmind/MaxMind-DB-Reader-python

oschwald · 2026-10-03T15:52:51Z

Fixes iteration over corrupt and unusual search trees in both readers.

Corrupt trees. A tree with a cycle made the C extension set bits past its 16-byte address buffer and then past its heap allocation. The pure Python reader recursed until RecursionError, or returned part of the networks. Both now raise InvalidDatabaseError at a node at the full address depth or a record that points to the root.
Short IPv6 networks. Both readers turned any network whose first 96 bits were zero into IPv4, so ::/1 got a negative prefix length and raised ValueError. Only a /96 or longer network converts now. The pure Python reader also skipped data records equal to the IPv4 start node, mis-compared ::1:0:0/96, and treated a cycle inside the IPv4 subtree as an alias. It now follows the C iterator.
Iterator protocol. The C iterator raised ValueError instead of StopIteration when it was exhausted and its reader closed, and it continued after an error. It now stops after any error, including an error in one record's data, as the pure Python generator does.
Data separator. The pure Python reader returned {} for a record that points into the 16-byte separator before the data section. It now raises InvalidDatabaseError.

The C and pure Python iterators give the same networks on every test database. A pure Python lookup benchmark showed no measurable change.

STF-1959

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Corrected iteration over IPv6 networks with short prefixes, including networks whose addresses begin with zero bits.
- Invalid or corrupt databases now raise InvalidDatabaseError in applicable cases instead of returning incomplete results or causing recursion errors.
- Iterators remain exhausted after reaching the end or encountering an error, including after the reader closes.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

coderabbitai · 2026-10-03T15:52:58Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 157fe3f1-1fca-4f5f-b5d9-0a9e50935ef3

📥 Commits

Reviewing files that changed from the base of the PR and between 458321c and 2989384.

📒 Files selected for processing (4)

HISTORY.rst
extension/maxminddb.c
maxminddb/reader.py
tests/reader_test.py

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

Walkthrough

The pure Python reader and C extension update search-tree validation, IPv4 and IPv6 network iteration, and iterator exhaustion behavior. Tests cover malformed trees, pointer bounds, IPv6 network boundaries, and iterator errors.

Changes

Reader integrity and iteration

Layer / File(s)	Summary
Data-section bounds validation `maxminddb/reader.py`, `tests/reader_test.py`	The reader stores search-tree and data-section offsets. It rejects pointers before the data section or at and beyond the buffer end. A test covers a pointer into the tree/data separator.
Network traversal and corruption checks `maxminddb/reader.py`, `extension/maxminddb.c`, `tests/reader_test.py`, `HISTORY.rst`	The readers reject invalid tree traversal and apply explicit IPv4 and IPv6 network rules. Tests cover malformed trees, cycles, shallow IPv6 networks, and an IPv6 prefix boundary.
C-extension iterator lifecycle `extension/maxminddb.c`, `tests/reader_test.py`, `HISTORY.rst`	The C-extension iterator frees pending records and remains exhausted after exhaustion or an error. Tests check exhaustion before reader closure and stopping after a post-close error.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Suggested reviewers: horgh

Merge Risk: ⚪ Minimal · up to 29893

No concrete merge-blocking issue remains in the reviewed reader and iterator changes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 4 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: fixing iteration behavior for corrupt and unusual search trees in both readers.

Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 4 files. (1 skipped: 1 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{A rabbit checks the branching tree
And bounds each pointer carefully
IPv6 paths now show their range
A stopped iterator stays the same
The moonlit tests hop into view
Then carrots mark the work as through}

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

reader_iter_next checked for a closed reader before it checked whether the iterator had ended. After close(), an exhausted iterator raised ValueError instead of StopIteration, which breaks the iterator protocol. The pure Python iterator, a generator, stops as expected. Mark the iterator as done when next() raises StopIteration, and check that flag first. It needs no lock, because the flag belongs to the iterator. The list of pending records cannot replace the flag: it is already empty when the last record is returned, and the next call must still report a closed or reopened reader, as the pure Python iterator does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Neither reader limited the depth of the tree walk during iteration. A corrupt tree, such as one where a node points back to itself, made the C extension set bits past the end of the 16-byte ip_packed array, and then past its heap allocation. The process aborted with heap corruption. The pure Python reader recursed until it raised RecursionError, which a caller that catches InvalidDatabaseError does not catch. A node at the full address depth has no valid children, so both readers now raise InvalidDatabaseError when they reach one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

After an error, such as a corrupt search tree, the C iterator kept its pending records, and the next call continued from them. With a cycle in the tree, a caller that skipped bad records could get many errors before StopIteration. The pure Python iterator is a generator, so it stops after its first error. Free the pending records and mark the iterator done when next() fails, so the C iterator stops too. Reuse the loop from ReaderIter_dealloc as free_records. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Both iterators turned any network whose first 96 bits were zero into an IPv4 network and subtracted 96 from its prefix length. For a network shorter than /96, such as ::/1, the prefix length became negative, and iteration raised ValueError. An IPv4 network in an IPv6 tree is at least /96, so convert only those. The pure Python iterator had two more bugs here. It compared the address with 2**32 using <=, so ::1:0:0/96 got a prefix length of 0 and raised ValueError. It also skipped every data record equal to the IPv4 start node, although only a search node can be the IPv4 subtree. Use <, and skip only a search node, as the C iterator does. Build the network with IPv4Network or IPv6Network, because ip_network() picks IPv4 for any small integer. Its alias rule also differed from the C iterator. It skipped the IPv4 start node in an IPv4 tree, where a record that points back to the root is a cycle, and inside the IPv4 subtree of an IPv6 tree. Both hid a corrupt tree behind partial results. Skip the subtree only when an address with a set bit in its first 96 bits leads to it, as the C iterator does, and raise InvalidDatabaseError for a record that points to the root, which libmaxminddb treats as invalid. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

_resolve_data_pointer checked only that a data pointer was inside the buffer. A search tree record that pointed into the 16-byte separator between the search tree and the data section decoded the zero bytes there, so get() returned {} and iteration yielded the network with {}. libmaxminddb rejects such a record as a corrupt search tree. Reject a pointer before the start of the data section too. A lookup benchmark showed no measurable cost. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI balanced review requested due to automatic review settings October 3, 2026 15:52

oschwald mentioned this pull request Oct 3, 2026

Improve the public type hints #470

Open

Copilot AI reviewed Oct 3, 2026

oschwald force-pushed the greg/stf-1958 branch from 19c4250 to 788c36c Compare October 3, 2026 16:17

Copilot AI balanced review requested due to automatic review settings October 3, 2026 16:17

oschwald force-pushed the greg/stf-1959 branch from 3ab7b3c to 432eaed Compare October 3, 2026 16:17

Copilot AI reviewed Oct 3, 2026

Copilot AI balanced review requested due to automatic review settings October 3, 2026 16:38

oschwald force-pushed the greg/stf-1958 branch from 788c36c to 4b038aa Compare October 3, 2026 16:38

oschwald force-pushed the greg/stf-1959 branch from 432eaed to 107c356 Compare October 3, 2026 16:38

Copilot AI reviewed Oct 3, 2026

Copilot AI balanced review requested due to automatic review settings October 3, 2026 16:55

oschwald force-pushed the greg/stf-1959 branch from 107c356 to f8fd2ab Compare October 3, 2026 16:55

oschwald force-pushed the greg/stf-1958 branch from 4b038aa to ca0e3e2 Compare October 3, 2026 16:55

Copilot AI reviewed Oct 3, 2026

oschwald force-pushed the greg/stf-1959 branch from f8fd2ab to 84e08a9 Compare October 3, 2026 17:18

Copilot AI balanced review requested due to automatic review settings October 3, 2026 17:18

Copilot AI reviewed Oct 3, 2026

oschwald mentioned this pull request Oct 3, 2026

Improve types and fix C extension crashes #465

Closed

oschwald force-pushed the greg/stf-1958 branch from ca0e3e2 to 8bc9b74 Compare October 5, 2026 15:32

Copilot AI balanced review requested due to automatic review settings October 5, 2026 15:32

oschwald force-pushed the greg/stf-1959 branch from 84e08a9 to 0c2b173 Compare October 5, 2026 15:32

Copilot AI reviewed Oct 5, 2026

oschwald force-pushed the greg/stf-1958 branch from 8bc9b74 to ed0485d Compare October 5, 2026 16:29

Copilot AI balanced review requested due to automatic review settings October 5, 2026 16:29

oschwald force-pushed the greg/stf-1959 branch from 0c2b173 to a816889 Compare October 5, 2026 16:29

Copilot AI reviewed Oct 5, 2026

Copilot AI balanced review requested due to automatic review settings October 5, 2026 17:00

oschwald force-pushed the greg/stf-1959 branch from a816889 to 36af132 Compare October 5, 2026 17:00

oschwald force-pushed the greg/stf-1959 branch from da21457 to e56d8aa Compare October 6, 2026 16:32

oschwald force-pushed the greg/stf-1958 branch from fda9ed0 to 5642dcb Compare October 6, 2026 16:32

Copilot AI reviewed Oct 6, 2026

oschwald force-pushed the greg/stf-1959 branch from e56d8aa to d21dd5d Compare October 6, 2026 16:59

Copilot AI balanced review requested due to automatic review settings October 6, 2026 16:59

oschwald force-pushed the greg/stf-1958 branch from 5642dcb to 1a2020d Compare October 6, 2026 16:59

Copilot AI reviewed Oct 6, 2026

oschwald force-pushed the greg/stf-1958 branch from 1a2020d to 28a2e5f Compare October 6, 2026 22:47

Copilot AI balanced review requested due to automatic review settings October 6, 2026 22:47

oschwald force-pushed the greg/stf-1959 branch from d21dd5d to 34161ac Compare October 6, 2026 22:47

Copilot AI reviewed Oct 6, 2026

oschwald force-pushed the greg/stf-1958 branch 7 times, most recently from a5282b1 to 47376c5 Compare October 8, 2026 14:52

Base automatically changed from greg/stf-1958 to main October 8, 2026 18:13

Copilot AI balanced review requested due to automatic review settings October 8, 2026 18:40

oschwald force-pushed the greg/stf-1959 branch from 34161ac to 458321c Compare October 8, 2026 18:40

Copilot AI reviewed Oct 8, 2026

oschwald and others added 4 commits October 8, 2026 20:57

oschwald force-pushed the greg/stf-1959 branch from 458321c to 2989384 Compare October 8, 2026 21:27

Copilot AI balanced review requested due to automatic review settings October 8, 2026 21:27

Copilot AI reviewed Oct 8, 2026

Conversation

oschwald commented Oct 3, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

oschwald commented Oct 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 3, 2026 •

edited

Loading