Add bad test databases for libmaxminddb metadata and search-tree corruption#236
Add bad test databases for libmaxminddb metadata and search-tree corruption#236
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a minimal corrupt test database file. The review feedback points out that several other test fixtures mentioned in the PR description are missing from the current commit. Additionally, it is recommended to generate these test cases programmatically within the existing writer package rather than checking in binary blobs to maintain consistency with the project's current practices and improve maintainability.
| @@ -0,0 +1 @@ | |||
| ���MaxMind.com No newline at end of file | |||
There was a problem hiding this comment.
The pull request title and description indicate that 4 minimal corrupt test databases are being added, including several variants for search-tree corruption (libmaxminddb-separator-record-*.mmdb). However, only libmaxminddb-metadata-marker-only.mmdb is included in the current diff. Please ensure all intended test fixtures are included in the commit.
Additionally, there is a discrepancy in the description: it mentions "three variants" of the separator record databases, but the pattern {min,max}-{left,right} typically expands to four files. Please clarify the intended set of fixtures.
| @@ -0,0 +1 @@ | |||
| ���MaxMind.com No newline at end of file | |||
There was a problem hiding this comment.
Most corrupt databases in this repository (as seen in pkg/writer/baddata.go) are generated programmatically. To maintain consistency and allow for easier inspection or future modification, consider adding these new test cases to the WriteBadDataDBs function in pkg/writer/baddata.go instead of checking in binary blobs. For example, libmaxminddb-metadata-marker-only.mmdb is a simple 14-byte file that could be represented as a byte slice in code.
A minimal 14-byte file containing only the \xab\xcd\xefMaxMind.com metadata marker, with no metadata following. Readers should reject this as invalid metadata. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three minimal variants of MaxMind-DB-test-ipv4-24.mmdb where node 0 has a record value in the half-open range [node_count+1, node_count+16) — i.e., pointing into the 16-byte separator between the search tree and data section. Readers should reject these as corrupt search trees rather than exposing data entries with underflowed offsets. -min-left: left record = node_count + 1 (first separator byte) -min-right: right record = node_count + 1 (verifies right-side check) -max-left: left record = node_count + 15 (last separator byte) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 participants
Summary
Adds 4 minimal corrupt test databases under
bad-data/libmaxminddb/,exercising metadata-section and search-tree-record corruption that
libmaxminddb now rejects:
libmaxminddb-metadata-marker-only.mmdb(14 bytes) — file containingonly the
\xab\xcd\xefMaxMind.commetadata marker. Readers shouldreject as invalid metadata.
libmaxminddb-separator-record-{min,max}-{left,right}.mmdb— threevariants of
MaxMind-DB-test-ipv4-24.mmdbwith a node-0 recordpointing into the 16-byte separator between the search tree and data
section. Readers should reject as a corrupt search tree.
Pulled out of runtime file mutation in maxmind/libmaxminddb#431 so all
readers can share the same corruption fixtures.
Linear: STF-326
Test plan
🤖 Generated with Claude Code