diff --git a/HISTORY.rst b/HISTORY.rst index ed913b10..a612a188 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -27,6 +27,17 @@ 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: + + * 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 of the wrong type or out of range, an invalid ``ip_version`` or + format version, or a ``build_epoch`` of 0. + * 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 94854280..29035773 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -659,40 +659,62 @@ 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. + // Keys that Metadata does not know are ignored. + const MMDB_metadata_s *m = &mmdb_obj->mmdb->metadata; + 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); + PyObject *map = 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); - PyErr_SetString(state->MaxMindDB_error, "Error decoding metadata."); - Py_XDECREF(metadata_dict); - return NULL; + + PyObject *metadata = NULL; + if (map != NULL) { + // MMDB_open requires these keys, so a missing key is a bug. + PyObject *database_type = NULL; + PyObject *description = NULL; + PyObject *languages = NULL; + if (PyDict_Check(map)) { + database_type = PyDict_GetItemString(map, "database_type"); + description = PyDict_GetItemString(map, "description"); + languages = PyDict_GetItemString(map, "languages"); + } + if (database_type == NULL || description == NULL || + languages == NULL) { + PyErr_SetString(state->MaxMindDB_error, + "Error decoding metadata."); + } else { + metadata = PyObject_CallFunction(state->Metadata_Type, + "HHKOOHOIH", + m->binary_format_major_version, + m->binary_format_minor_version, + (unsigned long long)m->build_epoch, + database_type, + description, + m->ip_version, + languages, + (unsigned int)m->node_count, + m->record_size); + } + Py_DECREF(map); } reader_release_read_lock(mmdb_obj); - - PyObject *args = PyTuple_New(0); - if (args == NULL) { - Py_DECREF(metadata_dict); - return NULL; + // 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."); } - - PyObject *metadata = - PyObject_Call(state->Metadata_Type, args, metadata_dict); - - Py_DECREF(metadata_dict); - Py_DECREF(args); return metadata; } @@ -1323,6 +1345,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) // ============================================================================= @@ -1351,6 +1411,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 a694ab40..39ae512f 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/maxminddb/reader.py b/maxminddb/reader.py index cb814c2a..dadf750b 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." @@ -95,7 +95,13 @@ def __init__( metadata_start += len(self._METADATA_START_MARKER) metadata_decoder = Decoder(self._buffer, metadata_start) - (metadata, _) = metadata_decoder.decode(metadata_start) + 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})." @@ -103,16 +109,8 @@ def __init__( 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. @@ -356,6 +354,86 @@ 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. +_METADATA_UINT_BITS: dict[str, int] = { + "binary_format_major_version": 16, + "binary_format_minor_version": 16, + "build_epoch": 64, + "ip_version": 16, + "node_count": 32, + "record_size": 16, +} + + +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 of the size 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. + # 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) + if fields["node_count"] < 0: + msg = f"{prefix} Invalid node count: {fields['node_count']}." + 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 f5ac1557..d4cee5ab 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -1,12 +1,14 @@ from __future__ import annotations import contextlib +import dataclasses import gc import io import ipaddress import multiprocessing import os import pathlib +import struct import subprocess import sys import sysconfig @@ -34,6 +36,7 @@ MODE_MMAP, MODE_MMAP_EXT, ) +from maxminddb.decoder import Decoder if TYPE_CHECKING: from collections.abc import Iterator @@ -112,6 +115,60 @@ def address_space_in_use() -> int: resource.setrlimit(resource.RLIMIT_AS, (soft, hard)) +_METADATA_START_MARKER = b"\xab\xcd\xefMaxMind.com" +# 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(**changes: object) -> bytes: + """Return the decoder test database with changed metadata. + + A value of None removes the key. + """ + data = pathlib.Path(f"{_TEST_DATA_DIR}/MaxMind-DB-test-decoder.mmdb").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} + edited = {k: v for k, v in merged.items() if v is not None} + return data[:start] + _encode_value(edited) + + +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().""" if mode == MODE_FD: @@ -626,6 +683,62 @@ 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}, + "missing description": {"description": None}, + "string node_count": {"node_count": "1"}, + "double node_count": {"node_count": 1.5}, + "boolean record_size": {"record_size": 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_ip_validation(self) -> None: reader = open_database( "tests/data/test-data/MaxMind-DB-test-decoder.mmdb", @@ -925,6 +1038,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): @@ -1304,6 +1422,12 @@ 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_empty_search_tree_is_accepted(self) -> None: data = pathlib.Path( f"{_TEST_DATA_DIR}/MaxMind-DB-test-ipv4-24.mmdb" @@ -1341,11 +1465,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"