Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
oschwald
force-pushed
the
greg/stf-1960
branch
from
October 3, 2026 16:17
8d06e7c to
c0d7169
Compare
oschwald
force-pushed
the
greg/stf-1959
branch
from
October 3, 2026 16:17
3ab7b3c to
432eaed
Compare
oschwald
force-pushed
the
greg/stf-1960
branch
from
October 3, 2026 16:38
c0d7169 to
a77adfe
Compare
oschwald
force-pushed
the
greg/stf-1959
branch
from
October 3, 2026 16:38
432eaed to
107c356
Compare
oschwald
force-pushed
the
greg/stf-1960
branch
from
October 3, 2026 16:55
a77adfe to
f371d1c
Compare
oschwald
force-pushed
the
greg/stf-1959
branch
from
October 3, 2026 16:55
107c356 to
f8fd2ab
Compare
oschwald
force-pushed
the
greg/stf-1959
branch
from
October 3, 2026 17:18
f8fd2ab to
84e08a9
Compare
oschwald
force-pushed
the
greg/stf-1960
branch
from
October 3, 2026 17:18
f371d1c to
203ffb4
Compare
oschwald
force-pushed
the
greg/stf-1960
branch
from
October 3, 2026 17:36
203ffb4 to
d1b06c4
Compare
oschwald
force-pushed
the
greg/stf-1960
branch
from
October 5, 2026 15:32
d1b06c4 to
55c777f
Compare
oschwald
force-pushed
the
greg/stf-1959
branch
from
October 5, 2026 15:32
84e08a9 to
0c2b173
Compare
oschwald
force-pushed
the
greg/stf-1959
branch
from
October 5, 2026 16:29
0c2b173 to
a816889
Compare
oschwald
force-pushed
the
greg/stf-1960
branch
from
October 6, 2026 16:59
5e7d02d to
f150ecf
Compare
oschwald
force-pushed
the
greg/stf-1959
branch
from
October 6, 2026 16:59
e56d8aa to
d21dd5d
Compare
The reader passed every decoded metadata key to Metadata, with no check. An unknown key or a missing key raised a bare TypeError. The spec says that a new key is a minor version change, so a reader must accept keys that it does not know. A value of the wrong type passed, so Metadata held values that did not match its annotations. For example, a string node_count failed later with TypeError, and a string languages value opened with no error. libmaxminddb rejects a missing key or a wrong type with InvalidDatabaseError. Pass only the known keys to Metadata, after a check that each one is present and has the expected type. This also removes the annotated local that widened the unchecked metadata to dict[str, Any], and its comment, which said that the spec fixes the metadata keys. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The spec says that a new metadata key is a minor version change, so a reader must accept keys that it does not know. Reader.metadata() decoded the whole metadata map again and passed every key to the Metadata constructor, which accepts only the nine known keys. A database with an unknown key made metadata() raise TypeError. Before the segmentation fault fix, it crashed the process. Pass only the nine known fields to Metadata. Take the numbers from the metadata that libmaxminddb parsed and checked when it opened the database, which are the values that libmaxminddb uses for lookups. Take database_type, description and languages from the decoded metadata map. libmaxminddb stores its copies of these strings as C strings, which end at the first NUL, so they would truncate a value and merge description keys that differ only after a NUL. Invalid UTF-8 in a metadata string still raises InvalidDatabaseError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
open_database() declares the pure Python Reader as its return type, even when it returns the extension Reader. Type checkers therefore accepted metadata().node_byte_size and metadata().search_tree_size, but the extension Metadata did not have them. In MODE_AUTO with the extension, the code raised AttributeError. Add both properties to the C type and to the stub. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
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>
maxminddb.decoder does not export InvalidDatabaseError. It only imports it. mypy --strict reports the import as an implicit re-export. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since 3.0.0, the README has said that the modes are available from maxminddb.Mode. maxminddb did not import Mode, so that attribute raised AttributeError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub #464 replaced the AnyStr TypeVar in Primitive with str | bytes. Primitive and Record are no longer generic aliases, so a subscript such as Record[str] now raises TypeError. HISTORY.rst did not mention the change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The C extension decodes the MaxMind DB bytes type to bytearray. The pure Python reader decodes it to bytes. MODE_AUTO uses the extension when it is available, so most callers get bytearray. mypy does not treat bytearray as bytes, so it reported an isinstance(value, bytearray) check as unreachable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
__iter__ and _generate_children returned a bare Iterator, so strict type checkers inferred Unknown for the network and record of each item. The extension stub already declares the item type. Use the same type. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both __exit__ methods left their arguments untyped and suppressed the ruff warning. Strict type checkers report the missing annotation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hasattr() never causes an attr-defined error. mypy --strict reports the ignore as unused. The ignore on the os.pread() call stays because Windows has no os.pread. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
open_database(), Reader.__init__, Reader._load and Reader._load_buffer each spelled out the database argument union. A change to the accepted types had to edit every copy, and nothing caught a missed one. Define StrOrBytesPath and DatabaseSource in maxminddb.types and use them. The extension stub accepts different types, so it keeps its own annotation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The extension parses the database argument with PyUnicode_FSConverter, so it accepts only str, bytes and os.PathLike. The stub also accepted int and IO[bytes], which raise TypeError. The extension does not support MODE_FD, so the docstring was wrong too. open_database() passes any database argument to the extension in MODE_AUTO and MODE_MMAP_EXT. The narrower stub shows that mismatch. Keep the runtime behavior and explain the type: ignore. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MODE_FD calls only database.read() and reads database.name when it exists. IO[bytes] requires much more, so a GzipFile and other binary readers did not type-check, although they work at runtime. Accept any object with a read() method that returns bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The release notes point users to the type aliases in maxminddb.types, such as Record, DatabaseSource and SupportsRead, but the API docs did not include that module. Add it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No test checked the public types, and non-strict mypy accepts a bare generic alias with no error. A TypeVar back in Primitive, a bare Iterator, or a wider stub would pass CI. mypy already checks tests/ in the lint environment. Add assert_type checks under TYPE_CHECKING, so the file costs nothing at runtime. The file enables warn-unused-ignores, so each type: ignore asserts that a call fails the type check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
oschwald
force-pushed
the
greg/stf-1959
branch
from
October 6, 2026 22:47
d21dd5d to
34161ac
Compare
oschwald
force-pushed
the
greg/stf-1960
branch
from
October 6, 2026 22:47
f150ecf to
8ff0a38
Compare
oschwald
force-pushed
the
greg/stf-1959
branch
2 times, most recently
from
October 8, 2026 21:27
458321c to
2989384
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improves the public type hints. Stacked on #469; review only the commits in this PR.
Primitiveincludesbytearray, which the C extension returns for the bytes type.Reader.__iter__declares its item type, and both__exit__methods annotate their arguments.maxminddb.Modeis exported, as the README has said since 3.0.0.maxminddb.typesaddsStrOrBytesPath,DatabaseSourceandSupportsRead, which replace repeated unions.MODE_FDaccepts any object whoseread()returnsbytes, such as aGzipFile.Readerstub accepts only a path, as the extension does.tests/typing_test.pyaddsassert_typechecks, which mypy runs in the lint environment.maxminddb.types, and HISTORY.rst notes thatRecordandPrimitiveare no longer generic (Make Record and database parameter types concrete #464).Overloads that tie each argument type to a mode are not included. geoip2 passes
mode: intthrough toopen_database(), so they would break its type checks.STF-1960
🤖 Generated with Claude Code