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-1958
branch
from
October 3, 2026 16:17
19c4250 to
788c36c
Compare
oschwald
force-pushed
the
greg/stf-1957
branch
from
October 3, 2026 16:17
43f3dfb to
690e7f7
Compare
oschwald
force-pushed
the
greg/stf-1958
branch
from
October 3, 2026 16:38
788c36c to
4b038aa
Compare
oschwald
force-pushed
the
greg/stf-1957
branch
from
October 3, 2026 16:38
690e7f7 to
4146d9c
Compare
oschwald
force-pushed
the
greg/stf-1958
branch
from
October 3, 2026 16:55
4b038aa to
ca0e3e2
Compare
oschwald
force-pushed
the
greg/stf-1958
branch
from
October 5, 2026 15:32
ca0e3e2 to
8bc9b74
Compare
oschwald
force-pushed
the
greg/stf-1957
branch
from
October 5, 2026 15:32
4146d9c to
cd376ca
Compare
oschwald
force-pushed
the
greg/stf-1957
branch
from
October 5, 2026 16:29
cd376ca to
8f9492c
Compare
oschwald
force-pushed
the
greg/stf-1958
branch
from
October 5, 2026 16:29
8bc9b74 to
ed0485d
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
oschwald
force-pushed
the
greg/stf-1957
branch
from
October 5, 2026 17:00
8f9492c to
f3c264c
Compare
oschwald
force-pushed
the
greg/stf-1958
branch
from
October 5, 2026 17:00
ed0485d to
a07fe62
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.
Makes both readers handle database metadata the same way. Stacked on #467; review only the commits in this PR.
Metadatawith no checks. It now ignores unknown keys, which a minor version of the format can add, and raisesInvalidDatabaseErrorfor a missing key, a value of the wrong type or out of range, anip_versionother than 4 or 6, a major format version other than 2, abuild_epochof 0, or metadata that does not decode, such as a string that is not UTF-8. These match libmaxminddb. It still accepts an empty search tree (node_count0).Reader.metadata()decoded the metadata map again and raised on an unknown key. It now passes only the nine known fields toMetadata. The numbers come from the metadata that libmaxminddb checked when it opened the database.database_type,descriptionandlanguagescome from the decoded map, because libmaxminddb's copies of these strings end at the first NUL.Metadata. It gains thenode_byte_sizeandsearch_tree_sizeproperties that the pure PythonMetadataand the type hints already have.The new checks can reject a database that the pure Python reader opened before. libmaxminddb already rejected those databases. The reader cannot check the width of an integer, because the decoder does not report it. ENG-5541 tracks the optional
languagesanddescriptionkeys, which both libmaxminddb and this reader require although the spec does not.STF-1958
🤖 Generated with Claude Code