From 4e7181f17dabe0869b03cdf698a147f0b25c6081 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 2 Oct 2026 22:54:40 +0000 Subject: [PATCH 1/5] Validate metadata in the pure Python reader 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. Also check that each integer is in the range that libmaxminddb accepts, and raise InvalidDatabaseError for a metadata string that is not UTF-8. For a repeated key, this reader keeps the last value, but libmaxminddb uses the first. Handling that case needed a separate decode path for the metadata map, so this reader accepts the difference. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 9 +++ maxminddb/reader.py | 97 +++++++++++++++++++++++++---- tests/reader_test.py | 144 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 234 insertions(+), 16 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index a791dcc..2451817 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -30,6 +30,15 @@ History * Fixed a crash on free-threaded Python when two threads advanced the same iterator. +* Metadata: + + * The pure Python reader ignores unknown keys, which a new minor version of + the format can add. It raises ``InvalidDatabaseError`` for a missing key, a + value that is not of the expected Python type or is out of range, an + invalid ``ip_version`` or format version, a ``build_epoch`` of 0, or a + string that is not UTF-8. Previously, the reader opened most of these + files, and some raised ``TypeError`` or ``UnicodeDecodeError``. + 3.2.0 (2026-09-10) ++++++++++++++++++ diff --git a/maxminddb/reader.py b/maxminddb/reader.py index c0ca346..c27f22e 100644 --- a/maxminddb/reader.py +++ b/maxminddb/reader.py @@ -24,7 +24,7 @@ from typing_extensions import Self - from maxminddb.types import Record + from maxminddb.types import Record, RecordDict _IPV4_MAX_NUM = 2**32 _REOPENED = "Attempt to iterate over a reopened MaxMind DB. Create a new iterator." @@ -113,7 +113,15 @@ def _load( metadata_start += len(self._METADATA_START_MARKER) metadata_decoder = Decoder(self._buffer, metadata_start) - (metadata, _) = metadata_decoder.decode(metadata_start) + # For a repeated key, the decoder keeps the last value, but + # libmaxminddb uses the first. This reader accepts the difference. + try: + (metadata, _) = metadata_decoder.decode(metadata_start) + except (TypeError, UnicodeDecodeError) as e: + # For example, a map key that is a list, or a string that is + # not UTF-8. The C extension raises InvalidDatabaseError too. + msg = f"Error reading metadata in database file ({filename})." + raise InvalidDatabaseError(msg) from e if not isinstance(metadata, dict): msg = f"Error reading metadata in database file ({filename})." @@ -121,16 +129,8 @@ def _load( msg, ) - # The MaxMind DB spec fixes these keys and their value types. - fields: dict[str, Any] = metadata - self._metadata = Metadata(**fields) + self._metadata = Metadata(**_metadata_fields(metadata, filename)) self._record_size = self._metadata.record_size - if self._record_size not in (24, 28, 32): - msg = f"Unknown record size: {self._record_size}" - raise InvalidDatabaseError(msg) # noqa: TRY301 - if self._metadata.node_count < 0: - msg = f"Invalid node count: {self._metadata.node_count}" - raise InvalidDatabaseError(msg) # noqa: TRY301 # Traversal reads nodes below node_count. Once the tree fits, those # reads need no length checks of their own. @@ -376,6 +376,81 @@ def __enter__(self) -> Self: return self +# The type of each metadata value. libmaxminddb also rejects a database with a +# missing key or a value of another type. It also checks the width and sign of +# each integer, which the decoder does not report. +_METADATA_TYPES: dict[str, type] = { + "binary_format_major_version": int, + "binary_format_minor_version": int, + "build_epoch": int, + "database_type": str, + "description": dict, + "ip_version": int, + "languages": list, + "node_count": int, + "record_size": int, +} + + +# The size in bits of each unsigned integer metadata value in libmaxminddb that +# needs a range check. The other integers must have exact values. +_METADATA_UINT_BITS: dict[str, int] = { + "binary_format_minor_version": 16, + "build_epoch": 64, + "node_count": 32, +} + + +def _metadata_fields(metadata: RecordDict, filename: object) -> dict[str, Any]: + """Return the known metadata fields after a check of their types. + + A new minor version of the format can add keys. This ignores them. + """ + prefix = f"Error reading metadata in database file ({filename})." + fields: dict[str, Any] = {} + for key, value_type in _METADATA_TYPES.items(): + value = metadata.get(key) + # The exact type check rejects bool, a subclass of int. + valid = type(value) is value_type + if valid and isinstance(value, list): + valid = all(type(v) is str for v in value) + elif valid and isinstance(value, dict): + valid = all(type(k) is str and type(v) is str for k, v in value.items()) + if not valid: + msg = f"{prefix} The {key} value is missing or has the wrong type." + raise InvalidDatabaseError(msg) + fields[key] = value + + _check_metadata_ranges(fields, prefix) + return fields + + +def _check_metadata_ranges(fields: dict[str, Any], prefix: str) -> None: + """Raise InvalidDatabaseError for a value that libmaxminddb rejects.""" + # libmaxminddb stores each integer as an unsigned value. Check the size of + # those in _METADATA_UINT_BITS. The reader decodes only the version 2 format, + # ip_version drives the tree walk, and record_size picks the node layout. + # These exact values need no range check. libmaxminddb also rejects + # node_count 0, but this reader accepts an empty search tree. + if fields["record_size"] not in (24, 28, 32): + msg = f"{prefix} Unknown record size: {fields['record_size']}." + raise InvalidDatabaseError(msg) + for key, bits in _METADATA_UINT_BITS.items(): + if not 0 <= fields[key] < 1 << bits: + msg = f"{prefix} The {key} value {fields[key]} is out of range." + raise InvalidDatabaseError(msg) + if fields["binary_format_major_version"] != 2: + version = fields["binary_format_major_version"] + msg = f"{prefix} Unsupported binary format version {version}." + raise InvalidDatabaseError(msg) + if fields["ip_version"] not in (4, 6): + msg = f"{prefix} The ip_version is {fields['ip_version']}, not 4 or 6." + raise InvalidDatabaseError(msg) + if fields["build_epoch"] == 0: + msg = f"{prefix} The build_epoch is 0." + raise InvalidDatabaseError(msg) + + @dataclass(kw_only=True, frozen=True) class Metadata: """Metadata for the MaxMind DB reader.""" diff --git a/tests/reader_test.py b/tests/reader_test.py index 22a82c9..1e02751 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -1,6 +1,7 @@ from __future__ import annotations import contextlib +import dataclasses import gc import io import ipaddress @@ -8,6 +9,7 @@ import multiprocessing import os import pathlib +import struct import subprocess import sys import sysconfig @@ -35,9 +37,10 @@ MODE_MMAP, MODE_MMAP_EXT, ) +from maxminddb.decoder import Decoder if TYPE_CHECKING: - from collections.abc import Iterator + from collections.abc import Iterator, Sequence from typing import IO from maxminddb.reader import Reader @@ -114,6 +117,66 @@ def address_space_in_use() -> int: resource.setrlimit(resource.RLIMIT_AS, (soft, hard)) +_METADATA_START_MARKER = maxminddb.reader.Reader._METADATA_START_MARKER # noqa: SLF001 +# libmaxminddb requires these unsigned integer types for metadata values. +_METADATA_UINT_TYPES = {"build_epoch": 9, "node_count": 6} +_UINT16_TYPE = 5 + + +def _database_with_metadata( + extra: Sequence[tuple[object, object]] = (), + /, + **changes: object, +) -> bytes: + """Return the decoder test database with changed metadata. + + A value of None removes the key. extra adds entries, which can have keys + that are not strings. + """ + data = pathlib.Path(_DECODER_DB).read_bytes() + start = data.rfind(_METADATA_START_MARKER) + len(_METADATA_START_MARKER) + (metadata, _) = Decoder(data, start).decode(start) + merged = {**cast("dict[str, object]", metadata), **changes} + entries = [*((k, v) for k, v in merged.items() if v is not None), *extra] + items = b"".join(_encode_value(k) + _encode_value(v, str(k)) for k, v in entries) + return data[:start] + _encode_control(7, len(entries)) + items + + +def _encode_value(value: object, key: str = "") -> bytes: + if isinstance(value, str): + encoded = value.encode() + return _encode_control(2, len(encoded)) + encoded + if isinstance(value, bool): + return _encode_control(14, int(value)) + if isinstance(value, float): + return _encode_control(3, 8) + struct.pack(">d", value) + if isinstance(value, int): + encoded = value.to_bytes((value.bit_length() + 7) // 8, "big") + type_num = _METADATA_UINT_TYPES.get(key, _UINT16_TYPE) + return _encode_control(type_num, len(encoded)) + encoded + if isinstance(value, list): + items = b"".join(_encode_value(v) for v in value) + return _encode_control(11, len(value)) + items + if isinstance(value, dict): + items = b"".join( + _encode_value(k) + _encode_value(v, k) for k, v in value.items() + ) + return _encode_control(7, len(value)) + items + msg = f"cannot encode {value!r}" + raise TypeError(msg) + + +def _encode_control(type_num: int, size: int) -> bytes: + # Sizes from 29 to 284 use one extra size byte. These tests need no more. + extended = b"" + if type_num > 7: + extended = bytes([type_num - 7]) + type_num = 0 + if size < 29: + return bytes([type_num << 5 | size]) + extended + return bytes([type_num << 5 | 29]) + extended + bytes([size - 29]) + + def get_reader_from_file_descriptor(filepath: str, mode: int) -> Reader: """Patches open_database() for class TestFDReader().""" # There are a few cases where mode is statically defined in @@ -634,6 +697,57 @@ def test_search_tree_past_end_of_file(self) -> None: ): reader.get(self.ipf("1.1.1.1")) + def test_invalid_metadata_is_rejected(self) -> None: + cases: dict[str, dict[str, object]] = { + "missing languages": {"languages": None}, + "missing description": {"description": None}, + "string node_count": {"node_count": "1"}, + "double node_count": {"node_count": 1.5}, + "boolean record_size": {"record_size": True}, + "boolean build_epoch": {"build_epoch": True}, + "string languages": {"languages": "en"}, + "integer in languages": {"languages": [1]}, + "integer in description": {"description": {"en": 1}}, + "integer key in description": {"description": {1: "en"}}, + "integer database_type": {"database_type": 5}, + "ip_version 5": {"ip_version": 5}, + "binary_format_major_version 3": {"binary_format_major_version": 3}, + "build_epoch 0": {"build_epoch": 0}, + "binary_format_minor_version too large": { + "binary_format_minor_version": 2**16, + }, + "build_epoch too large": {"build_epoch": 2**64}, + } + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "invalid-metadata.mmdb" + for name, changes in cases.items(): + with self.subTest(name): + path.write_bytes(_database_with_metadata(**changes)) + with ( + self.assertRaises(InvalidDatabaseError), + open_database(str(path), self.mode), + ): + pass + + def test_metadata_that_does_not_decode_is_rejected(self) -> None: + cases = { + "list key": _database_with_metadata([([1], "value")]), + "string that is not UTF-8": _database_with_metadata( + database_type="NOT-UTF-8", + ).replace(b"NOT-UTF-8", b"\xff" * 9), + } + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "bad-metadata.mmdb" + for name, data in cases.items(): + with self.subTest(name): + path.write_bytes(data) + # The C reader raises some of these only in metadata(). + with ( + self.assertRaises(InvalidDatabaseError), + open_database(str(path), self.mode) as reader, + ): + reader.metadata() + def test_ip_validation(self) -> None: reader = open_database( "tests/data/test-data/MaxMind-DB-test-decoder.mmdb", @@ -1573,6 +1687,19 @@ def reopen_one_buffer() -> None: with self.assertRaisesRegex(ValueError, "reopened MaxMind DB"): next(iterator) + def test_metadata_types_match_metadata_fields(self) -> None: + self.assertEqual( + list(maxminddb.reader._METADATA_TYPES), # noqa: SLF001 + [field.name for field in dataclasses.fields(maxminddb.reader.Metadata)], + ) + + def test_unknown_metadata_key_is_ignored(self) -> None: + data = _database_with_metadata(unknown_key="value") + with maxminddb.reader.Reader(io.BytesIO(data), MODE_FD) as reader: + metadata = reader.metadata() + self.assertEqual(metadata.database_type, "MaxMind DB Decoder Test") + self.assertFalse(hasattr(metadata, "unknown_key")) + def test_empty_search_tree_is_accepted(self) -> None: data = pathlib.Path( f"{_TEST_DATA_DIR}/MaxMind-DB-test-ipv4-24.mmdb" @@ -1595,7 +1722,7 @@ def test_invalid_tree_metadata_is_rejected_on_open(self) -> None: ( b"node_count\xc1\xa3", b"node_count\x04\x01\xff\xff\xff\xff", - "Invalid node count: -1", + "The node_count value -1 is out of range", ), ) for original, replacement, message in cases: @@ -1610,11 +1737,18 @@ def test_invalid_tree_metadata_is_rejected_on_open(self) -> None: def test_failed_initialization_closes_buffer(self) -> None: reader_class = maxminddb.reader.Reader - marker = b"\xab\xcd\xefMaxMind.com" cases = ( (b"not a database", InvalidDatabaseError, "Is this a valid MaxMind DB"), - (marker + b"\x40", InvalidDatabaseError, "Error reading metadata"), - (marker + b"\xe0", TypeError, "required keyword-only arguments"), + ( + _METADATA_START_MARKER + b"\x40", + InvalidDatabaseError, + "Error reading metadata", + ), + ( + _METADATA_START_MARKER + b"\xe0", + InvalidDatabaseError, + "missing or has the wrong type", + ), ( pathlib.Path( f"{_TEST_DATA_DIR}/MaxMind-DB-test-metadata-payload-limit.mmdb" From 741eca3ac55b245edc3dbf4c715c0c4b8f4e7d9e Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 2 Oct 2026 22:55:00 +0000 Subject: [PATCH 2/5] Ignore unknown metadata keys in the C extension 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. Create the Metadata after the read lock is released, because creating it can run Python code. For a repeated key, the strings come from the last entry, but libmaxminddb uses the first. Handling that case needed a second copy of from_map, so this reader accepts the difference. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 2 + extension/maxminddb.c | 92 ++++++++++++++++++++++++++++++++++--------- tests/reader_test.py | 58 +++++++++++++++++++++++---- 3 files changed, 127 insertions(+), 25 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 2451817..0607d6f 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -38,6 +38,8 @@ History invalid ``ip_version`` or format version, a ``build_epoch`` of 0, or a string that is not UTF-8. Previously, the reader opened most of these files, and some raised ``TypeError`` or ``UnicodeDecodeError``. + * The C extension ignores unknown keys. Previously, ``Reader.metadata()`` + crashed on them. 3.2.0 (2026-09-10) ++++++++++++++++++ diff --git a/extension/maxminddb.c b/extension/maxminddb.c index 90cbeda..9b1ae2a 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -135,6 +135,7 @@ static inline maxminddb_state *get_maxminddb_state_from_self(PyObject *self) { static void reader_close_database(Reader_obj *reader); static bool can_read(const char *path); static int get_record(PyObject *self, PyObject *args, PyObject **record); +static PyObject *metadata_value(PyObject *map, const char *key); static PyObject *reader_iter_next(PyObject *self); static bool format_sockaddr(struct sockaddr *addr, char *dst); static PyObject *from_entry_data_list(maxminddb_state *state, @@ -680,43 +681,97 @@ static PyObject *Reader_metadata(PyObject *self, PyObject *UNUSED(args)) { return NULL; } - MMDB_entry_data_list_s *entry_data_list; - int status = + // libmaxminddb checked the metadata when it opened the database, so take + // the numbers from its copy. Its strings end at the first NUL, so take + // the strings from the decoded metadata map, which keeps their lengths. + // For a repeated key, the strings come from the last entry, but + // libmaxminddb uses the first. This reader accepts the difference. Keys + // that Metadata does not know are ignored. + const MMDB_metadata_s *m = &mmdb_obj->mmdb->metadata; + uint16_t const binary_format_major_version = m->binary_format_major_version; + uint16_t const binary_format_minor_version = m->binary_format_minor_version; + uint64_t const build_epoch = m->build_epoch; + uint16_t const ip_version = m->ip_version; + uint32_t const node_count = m->node_count; + uint16_t const record_size = m->record_size; + MMDB_entry_data_list_s *entry_data_list = NULL; + int const status = MMDB_get_metadata_as_entry_data_list(mmdb_obj->mmdb, &entry_data_list); if (status != MMDB_SUCCESS) { reader_release_read_lock(mmdb_obj); + MMDB_free_entry_data_list(entry_data_list); PyErr_Format(state->MaxMindDB_error, "Error decoding metadata. %s", MMDB_strerror(status)); return NULL; } MMDB_entry_data_list_s *original_entry_data_list = entry_data_list; - - PyObject *metadata_dict = from_entry_data_list(state, &entry_data_list); - MMDB_free_entry_data_list(original_entry_data_list); - if (metadata_dict == NULL || !PyDict_Check(metadata_dict)) { - reader_release_read_lock(mmdb_obj); + PyObject *map = NULL; + if (entry_data_list != NULL && + entry_data_list->entry_data.type == MMDB_DATA_TYPE_MAP) { + map = from_map(state, &entry_data_list); + } else { PyErr_SetString(state->MaxMindDB_error, "Error decoding metadata."); - Py_XDECREF(metadata_dict); - return NULL; } + MMDB_free_entry_data_list(original_entry_data_list); + // Creating a Metadata can run Python code, such as an __init__, and no + // Python code may run under the read lock. The values above are copies. reader_release_read_lock(mmdb_obj); - PyObject *args = PyTuple_New(0); - if (args == NULL) { - Py_DECREF(metadata_dict); - return NULL; + PyObject *metadata = NULL; + if (map != NULL) { + PyObject *description = NULL; + PyObject *languages = NULL; + PyObject *database_type = metadata_value(map, "database_type"); + if (database_type != NULL) { + description = metadata_value(map, "description"); + } + if (description != NULL) { + languages = metadata_value(map, "languages"); + } + if (languages == NULL) { + // MMDB_open requires these keys, so a missing key is a bug. + if (!PyErr_Occurred()) { + PyErr_SetString(state->MaxMindDB_error, + "Error decoding metadata."); + } + } else { + // The order of the values must match kwlist in Metadata_new. + metadata = PyObject_CallFunction(state->Metadata_Type, + "HHKOOHOIH", + binary_format_major_version, + binary_format_minor_version, + (unsigned long long)build_epoch, + database_type, + description, + ip_version, + languages, + (unsigned int)node_count, + record_size); + } + Py_DECREF(map); } - PyObject *metadata = - PyObject_Call(state->Metadata_Type, args, metadata_dict); - - Py_DECREF(metadata_dict); - Py_DECREF(args); + // libmaxminddb does not check that the metadata strings are UTF-8. + if (metadata == NULL && PyErr_ExceptionMatches(PyExc_UnicodeDecodeError)) { + PyErr_SetString(state->MaxMindDB_error, "Error decoding metadata."); + } return metadata; } +// Return a borrowed reference to the value of key in map. NULL with no +// exception set means that the key is missing. +static PyObject *metadata_value(PyObject *map, const char *key) { + PyObject *name = PyUnicode_FromString(key); + if (name == NULL) { + return NULL; + } + PyObject *value = PyDict_GetItemWithError(map, name); + Py_DECREF(name); + return value; +} + static PyObject *Reader_close(PyObject *self, PyObject *UNUSED(args)) { Reader_obj *mmdb_obj = (Reader_obj *)self; @@ -1040,6 +1095,7 @@ Metadata_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { *build_epoch, *database_type, *description, *ip_version, *languages, *node_count, *record_size; + // Reader_metadata passes the values in this order. static char *kwlist[] = {"binary_format_major_version", "binary_format_minor_version", "build_epoch", diff --git a/tests/reader_test.py b/tests/reader_test.py index 1e02751..8517304 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -697,6 +697,31 @@ def test_search_tree_past_end_of_file(self) -> None: ): reader.get(self.ipf("1.1.1.1")) + def test_unknown_metadata_key_is_ignored(self) -> None: + # A new minor version of the format can add metadata keys. + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "unknown-key.mmdb" + path.write_bytes(_database_with_metadata(unknown_key="value")) + with open_database(str(path), self.mode) as reader: + metadata = reader.metadata() + self.assertEqual(metadata.database_type, "MaxMind DB Decoder Test") + self.assertFalse(hasattr(metadata, "unknown_key")) + + def test_metadata_strings_keep_embedded_nuls(self) -> None: + changes: dict[str, object] = { + "database_type": "one\0two", + "description": {"en\0x": "first", "en\0y": "second\0value"}, + "languages": ["en\0x"], + } + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "nul.mmdb" + path.write_bytes(_database_with_metadata(**changes)) + with open_database(str(path), self.mode) as reader: + metadata = reader.metadata() + self.assertEqual(metadata.database_type, changes["database_type"]) + self.assertEqual(metadata.description, changes["description"]) + self.assertEqual(metadata.languages, changes["languages"]) + def test_invalid_metadata_is_rejected(self) -> None: cases: dict[str, dict[str, object]] = { "missing languages": {"languages": None}, @@ -1356,6 +1381,32 @@ def descriptors() -> int: reader.close() self.assertEqual(mappings(), 0) + def test_metadata_init_can_close_the_reader(self) -> None: + # metadata() must build Metadata after it releases the read lock, + # because Metadata can run Python code. Under the lock, a close() from + # that code would wait for the lock forever on free-threaded Python. + program = textwrap.dedent( + """ + import sys + + import maxminddb.extension + + reader = maxminddb.extension.Reader(sys.argv[1]) + + def close_reader(self, *args, **kwargs): + reader.close() + + maxminddb.extension.Metadata.__init__ = close_reader + metadata = reader.metadata() + if not isinstance(metadata, maxminddb.extension.Metadata): + sys.exit("metadata() did not return a Metadata") + if not reader.closed: + sys.exit("Metadata.__init__ did not close the reader") + print("ok") + """, + ) + self._run_program(program) + def test_initialize_after_close_on_uninitialized_reader(self) -> None: reader_class = maxminddb.extension.Reader reader = reader_class.__new__(reader_class) @@ -1693,13 +1744,6 @@ def test_metadata_types_match_metadata_fields(self) -> None: [field.name for field in dataclasses.fields(maxminddb.reader.Metadata)], ) - def test_unknown_metadata_key_is_ignored(self) -> None: - data = _database_with_metadata(unknown_key="value") - with maxminddb.reader.Reader(io.BytesIO(data), MODE_FD) as reader: - metadata = reader.metadata() - self.assertEqual(metadata.database_type, "MaxMind DB Decoder Test") - self.assertFalse(hasattr(metadata, "unknown_key")) - def test_empty_search_tree_is_accepted(self) -> None: data = pathlib.Path( f"{_TEST_DATA_DIR}/MaxMind-DB-test-ipv4-24.mmdb" From 7f71bb820c73a06d32d66fa03a9a269b346aa5dc Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 2 Oct 2026 22:40:44 +0000 Subject: [PATCH 3/5] Add node_byte_size and search_tree_size to the C Metadata 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 --- HISTORY.rst | 2 ++ extension/maxminddb.c | 39 +++++++++++++++++++++++++++++++++++++++ maxminddb/extension.pyi | 8 ++++++++ tests/reader_test.py | 5 +++++ 4 files changed, 54 insertions(+) diff --git a/HISTORY.rst b/HISTORY.rst index 0607d6f..d6e66c8 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -29,6 +29,8 @@ History during iteration, from another thread or from a signal handler. * Fixed a crash on free-threaded Python when two threads advanced the same iterator. + * Added the ``node_byte_size`` and ``search_tree_size`` properties to + ``Metadata``, as the pure Python ``Metadata`` has. * Metadata: diff --git a/extension/maxminddb.c b/extension/maxminddb.c index 9b1ae2a..63c5644 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -1406,6 +1406,44 @@ static PyMemberDef Metadata_members[] = { NULL}, {NULL, 0, 0, 0, NULL}}; +static PyObject *Metadata_node_byte_size(PyObject *self, + void *UNUSED(closure)) { + Metadata_obj *obj = (Metadata_obj *)self; + PyObject *four = PyLong_FromLong(4); + if (four == NULL) { + return NULL; + } + PyObject *node_byte_size = PyNumber_FloorDivide(obj->record_size, four); + Py_DECREF(four); + return node_byte_size; +} + +static PyObject *Metadata_search_tree_size(PyObject *self, + void *UNUSED(closure)) { + Metadata_obj *obj = (Metadata_obj *)self; + PyObject *node_byte_size = Metadata_node_byte_size(self, NULL); + if (node_byte_size == NULL) { + return NULL; + } + PyObject *search_tree_size = + PyNumber_Multiply(obj->node_count, node_byte_size); + Py_DECREF(node_byte_size); + return search_tree_size; +} + +// These match the properties of the pure Python Metadata class. +static PyGetSetDef Metadata_getset[] = {{"node_byte_size", + Metadata_node_byte_size, + NULL, + "The size of a node in bytes.", + NULL}, + {"search_tree_size", + Metadata_search_tree_size, + NULL, + "The size of the search tree.", + NULL}, + {NULL, NULL, NULL, NULL, NULL}}; + // ============================================================================= // Type specs for heap type conversion (PEP 489) // ============================================================================= @@ -1434,6 +1472,7 @@ static PyType_Slot Metadata_Type_slots[] = { {Py_tp_new, Metadata_new}, {Py_tp_methods, Metadata_methods}, {Py_tp_members, Metadata_members}, + {Py_tp_getset, Metadata_getset}, {0, NULL}, }; diff --git a/maxminddb/extension.pyi b/maxminddb/extension.pyi index a694ab4..39ae512 100644 --- a/maxminddb/extension.pyi +++ b/maxminddb/extension.pyi @@ -127,3 +127,11 @@ class Metadata: record_size: int, ) -> None: """Create new Metadata object from the metadata fields in the spec.""" + + @property + def node_byte_size(self) -> int: + """The size of a node in bytes.""" + + @property + def search_tree_size(self) -> int: + """The size of the search tree.""" diff --git a/tests/reader_test.py b/tests/reader_test.py index 8517304..a4906bd 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -1098,6 +1098,11 @@ def _check_metadata( self.assertGreater(metadata.node_count, 36) self.assertEqual(metadata.record_size, record_size) + self.assertEqual(metadata.node_byte_size, record_size // 4) + self.assertEqual( + metadata.search_tree_size, + metadata.node_count * record_size // 4, + ) def _check_ip_v4(self, reader: Reader, file_name: str) -> None: for i in range(6): From e1bb481fb95656b3a03301b86d2f322a4e48303d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 7 Oct 2026 17:38:36 +0000 Subject: [PATCH 4/5] Convert TypeError in the pure Python decoder A record map with a key that cannot be hashed, such as a list, made Decoder.decode raise a bare TypeError, so a pure Python get() raised TypeError where the C extension raises InvalidDatabaseError. decode() already converts IndexError and struct.error. Convert TypeError there too, so both readers raise InvalidDatabaseError for such a key. The extra clause costs nothing on a successful lookup. The metadata decode now needs to convert only UnicodeDecodeError, which lookups keep as it is. A key that can be hashed but is not a string, such as an int, still decodes in a pure Python lookup, while the C extension rejects it. Checking each key would slow every lookup. STF-1922 tracks it. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 3 +++ maxminddb/decoder.py | 5 +++-- maxminddb/reader.py | 6 +++--- tests/decoder_test.py | 6 ++++++ 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index d6e66c8..fd6e7cf 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -11,6 +11,9 @@ History C extension. Before, the iterator walked the new database with node numbers from the old one. A failed ``__init__`` keeps the old database. After ``close()``, an iterator raises ``ValueError`` in every mode. +* For a record map with a key that cannot be hashed, such as a list, the + pure Python reader raises ``InvalidDatabaseError`` instead of ``TypeError``, + as the C extension does. * C extension: * Fixed segmentation faults from invalid use of ``Metadata``, ``Reader`` and diff --git a/maxminddb/decoder.py b/maxminddb/decoder.py index 9ef985a..cd1269f 100644 --- a/maxminddb/decoder.py +++ b/maxminddb/decoder.py @@ -255,8 +255,9 @@ def decode(self, offset: int) -> tuple[Record, int]: ) except RecursionError as ex: raise InvalidDatabaseError(_TOO_DEEP) from ex - except (IndexError, struct.error) as ex: - # Convert failed buffer indexing and fixed-width unpacking. + except (IndexError, struct.error, TypeError) as ex: + # Convert failed buffer indexing, fixed-width unpacking, and a map + # key that cannot be hashed, such as a list. raise InvalidDatabaseError(_BAD_DATA) from ex # Keep type dispatch inline to avoid another call for every decoded value. diff --git a/maxminddb/reader.py b/maxminddb/reader.py index c27f22e..0393a19 100644 --- a/maxminddb/reader.py +++ b/maxminddb/reader.py @@ -117,9 +117,9 @@ def _load( # libmaxminddb uses the first. This reader accepts the difference. try: (metadata, _) = metadata_decoder.decode(metadata_start) - except (TypeError, UnicodeDecodeError) as e: - # For example, a map key that is a list, or a string that is - # not UTF-8. The C extension raises InvalidDatabaseError too. + except UnicodeDecodeError as e: + # A string that is not UTF-8. The C extension raises + # InvalidDatabaseError too. Lookups keep UnicodeDecodeError. msg = f"Error reading metadata in database file ({filename})." raise InvalidDatabaseError(msg) from e diff --git a/tests/decoder_test.py b/tests/decoder_test.py index f4760c3..ceb80e8 100644 --- a/tests/decoder_test.py +++ b/tests/decoder_test.py @@ -316,6 +316,12 @@ def test_value_limit_follows_the_flat_rule(self) -> None: with self.assertRaisesRegex(InvalidDatabaseError, _TOO_MANY_VALUES): Decoder(self._scalar_pointer_array(65_536), pointer_base=0).decode(1) + def test_map_key_that_cannot_be_hashed_is_rejected(self) -> None: + # A map with one entry, whose key is the array [1]. + decoder = Decoder(bytes.fromhex("e10104a1014178")) + with self.assertRaisesRegex(InvalidDatabaseError, "contains bad data"): + decoder.decode(0) + def test_pointer_to_pointer_is_rejected(self) -> None: # The root array shares a pointer chain that would bypass value counting. buf = b"\xa0" + self._pointer(0) + b"\x02\x04" + self._pointer(1) * 2 From c7c94c25e840a697af5e38e24b8a6b2d4b0d4182 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 7 Oct 2026 23:30:25 +0000 Subject: [PATCH 5/5] Name the file in pure Python metadata decode errors Only a UnicodeDecodeError from the metadata decode got the "Error reading metadata in database file ()." prefix. Other decode errors, such as a key that cannot be hashed or an exceeded decode limit, did not name the file. Add the prefix to those too, and keep the original message. Co-Authored-By: Claude Opus 5.5 --- maxminddb/reader.py | 9 +++++---- tests/reader_test.py | 21 +++++++++++++++++++-- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/maxminddb/reader.py b/maxminddb/reader.py index 0393a19..abd9c22 100644 --- a/maxminddb/reader.py +++ b/maxminddb/reader.py @@ -117,10 +117,11 @@ def _load( # libmaxminddb uses the first. This reader accepts the difference. try: (metadata, _) = metadata_decoder.decode(metadata_start) - except UnicodeDecodeError as e: - # A string that is not UTF-8. The C extension raises - # InvalidDatabaseError too. Lookups keep UnicodeDecodeError. - msg = f"Error reading metadata in database file ({filename})." + except (InvalidDatabaseError, UnicodeDecodeError) as e: + # Add the file name. For a string that is not UTF-8, the C + # extension raises InvalidDatabaseError too. Lookups keep + # UnicodeDecodeError. + msg = f"Error reading metadata in database file ({filename}). {e}" raise InvalidDatabaseError(msg) from e if not isinstance(metadata, dict): diff --git a/tests/reader_test.py b/tests/reader_test.py index a4906bd..70bad63 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -68,6 +68,10 @@ "^The MaxMind DB file's data section exceeds the maximum number of values$" ) _TOO_DEEP = "^The MaxMind DB file's data section exceeds the maximum depth$" +_METADATA_PAYLOAD_TOO_LARGE = ( + r"^Error reading metadata in database file \(.+\)\. " + "The MaxMind DB file's data section exceeds the maximum payload size$" +) _EXTENSION_LIMIT_MESSAGE = "exceeds the configured resource limits" @@ -201,7 +205,7 @@ class BaseTestReader(unittest.TestCase): use_ip_objects = False payload_error = _PAYLOAD_TOO_LARGE value_count_error = _TOO_MANY_VALUES - metadata_error = _PAYLOAD_TOO_LARGE + metadata_error = _METADATA_PAYLOAD_TOO_LARGE fan_out_error = f"{_TOO_MANY_VALUES}|{_TOO_DEEP}" # fork doesn't work on Windows and spawn would involve pickling the reader, @@ -1784,6 +1788,19 @@ def test_invalid_tree_metadata_is_rejected_on_open(self) -> None: ): pass + def test_metadata_decode_error_names_the_file(self) -> None: + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "bad-metadata.mmdb" + path.write_bytes(_database_with_metadata([([1], "value")])) + with self.assertRaises(InvalidDatabaseError) as cm: + maxminddb.reader.Reader(str(path), MODE_MEMORY) + self.assertEqual( + str(cm.exception), + f"Error reading metadata in database file ({path}). " + "The MaxMind DB file's data section contains bad data " + "(unknown data type or corrupt data)", + ) + def test_failed_initialization_closes_buffer(self) -> None: reader_class = maxminddb.reader.Reader cases = ( @@ -1803,7 +1820,7 @@ def test_failed_initialization_closes_buffer(self) -> None: f"{_TEST_DATA_DIR}/MaxMind-DB-test-metadata-payload-limit.mmdb" ).read_bytes(), InvalidDatabaseError, - _PAYLOAD_TOO_LARGE, + _METADATA_PAYLOAD_TOO_LARGE, ), ) with tempfile.TemporaryDirectory() as directory: