inserting subnets in non-strict mode fail if more-specific subnet is inserted first

[zajdee]

Hi Cloudflare,

first please let me thank you for your code. It is really amazing.

We at Showmax are using it and have found an issue (a corner case, if you wish) which results in some prefixes not being stored into the generated file. The issue can be demonstrated on the following piece of code:

#!/usr/bin/env python3
"""Geoexceptions file generator."""
import argparse
import logging
import logging.handlers
import sys

import geoip2.database
import mmdbencoder
from geoip2.errors import AddressNotFoundError

def exception_data(country_code):
    """
    Format the data as needed by the MaxMind Country database format.

    Do not modify unless really sure what are you doing!

    :param country_code: ISO 2-character country code, e.g. ZA
    :return: MaxMind Country DB structure needed for an IP network record.
    """
    return {
        'continent': {},
        'registered_country': {},
        'represented_country': {},
        'traits': {},
        'country': {
            'iso_code': country_code,
        },
    }

def tryip(reader, ip):
    """Try to retrieve country code from the MaxMind database."""
    try:
        return reader.country(ip).country.iso_code
    except AddressNotFoundError:
        # Address is not present in the database
        return None

def main():
    """Main."""
    logging.basicConfig(stream=sys.stdout)
    logging.getLogger()
    logging.getLogger().setLevel(logging.DEBUG)

    logging.debug('Test case #1')

    test_pfxs = ['2001:db8:807:1::1', '2001:db8:807:102::1', '2001:db8:807:111::1']

    fname = 'test.mmdb'
    cc_za = exception_data('ZA')
    cc_ke = exception_data('KE')
    enc = mmdbencoder.Encoder(
            6,          # IP version
            32,         # Size of the pointers
            'Country',  # Name of the table
            ['en'],     # Languages
            {'en': 'Geoexceptions Country Database'},  # Description
            compat=True)
    data_za = enc.insert_data(cc_za)
    data_ke = enc.insert_data(cc_ke)
    enc.insert_network('2001:db8:807::/48', data_za, False)
    enc.insert_network('2001:db8:807:102::/64', data_ke, False)
    enc.write_file(fname)

    reader = geoip2.database.Reader(fname)
    for pfx in test_pfxs:
        logging.debug('prefix %s, present? %s', pfx, tryip(reader, pfx))

    logging.debug('Test case #2')
    fname = 'test.mmdb'
    cc_za = exception_data('ZA')
    cc_ke = exception_data('KE')
    enc = mmdbencoder.Encoder(
            6,          # IP version
            32,         # Size of the pointers
            'Country',  # Name of the table
            ['en'],     # Languages
            {'en': 'Geoexceptions Country Database'},  # Description
            compat=True)
    data_za = enc.insert_data(cc_za)
    data_ke = enc.insert_data(cc_ke)
    enc.insert_network('2001:db8:807:102::/64', data_ke, False)
    enc.insert_network('2001:db8:807::/48', data_za, False)
    enc.write_file(fname)

    reader = geoip2.database.Reader(fname)
    for pfx in test_pfxs:
        logging.debug('prefix %s, present? %s', pfx, tryip(reader, pfx))

    logging.debug('Tests completed.')

if __name__ == '__main__':
    main()

The resulting output is:

$ python test_subnet.py
DEBUG:root:Test case #1
DEBUG:root:prefix 2001:db8:807:1::1, present? ZA
DEBUG:root:prefix 2001:db8:807:102::1, present? KE
DEBUG:root:prefix 2001:db8:807:111::1, present? ZA
DEBUG:root:Test case #2
DEBUG:root:prefix 2001:db8:807:1::1, present? ZA
DEBUG:root:prefix 2001:db8:807:102::1, present? KE
DEBUG:root:prefix 2001:db8:807:111::1, present? None
DEBUG:root:Tests completed.

If the more-specific prefix is inserted first, the less-specific data for 2001:db8:807:100::/63, 2001:db8:807:103::..2001:db8:807:1ff:ffff:ffff:ffff:ffff are missing from the resulting file.

I believe this should not be happening.
(If strict mode is disabled, both cases end up with error Exception: Encoder: add_to_trie: try setting data on a non-final: (...128bit-int-number) already has child. Not updating in strict mode.)

[zajdee]

Correction: the last sentence should read:
If strict mode is enabled, both cases end up with error Exception: Encoder: add_to_trie: try setting data on a non-final: (...128bit-int-number) already has child. Not updating in strict mode.

[jovimon]

Hello @zajdee,
I had the same problem, and solved it adding strict=False to the insert_network function:
enc.insert_network(cidr, data, strict=False)
Bear in mind that in the source code the strict parameter is in the fourth place (https://github.com/cloudflare/py-mmdb-encoder/blob/master/mmdbencoder/__init__.py#L213), not the third, so you have to explicitly set it for it to work.

[zajdee]

Hi @jovimon,
I believe you might be referring to the following error message:

Exception: Encoder: add_to_trie: try setting data on a non-final: (...128bit-int-number) already has child. Not updating in strict mode.

This issue report is however not about that error message (there's False argument set in the demo code). It's about the library not generating the data file correctly in the edge case described.