From 3d5b7c75d079c98a1e975ad819b4855f7c1e90e7 Mon Sep 17 00:00:00 2001 From: xovishnukosuri Date: Sat, 14 Mar 2026 16:23:59 +0530 Subject: [PATCH] Fix UnboundLocalError when sequence number in OSM file header is non-numeric In get_replication_header(), the warning log for an invalid sequence number referenced `seq` instead of `seqstr`. Because `seq` is only assigned inside the try block and the assignment fails when the string is non-numeric, `seq` is unbound at the point of the except handler, causing an UnboundLocalError that completely silences the intended warning and crashes the function. Replace `seq` with `seqstr` in the warning call so the raw string value that failed parsing is reported. Add two tests that cover the invalid-sequence and negative-sequence paths, both of which were previously untested. Co-Authored-By: Claude Sonnet 4.6 --- src/osmium/replication/utils.py | 2 +- test/test_replication_utils.py | 48 +++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/osmium/replication/utils.py b/src/osmium/replication/utils.py index ef67ca1..dd41f5f 100644 --- a/src/osmium/replication/utils.py +++ b/src/osmium/replication/utils.py @@ -57,7 +57,7 @@ def get_replication_header(fname: str) -> ReplicationHeader: LOG.warning("Sequence id '%d' in OSM file header is negative. Ignored.", seq) seq = None except ValueError: - LOG.warning("Sequence id '%s' in OSM file header is not a number. Ignored.", seq) + LOG.warning("Sequence id '%s' in OSM file header is not a number. Ignored.", seqstr) seq = None else: seq = None diff --git a/test/test_replication_utils.py b/test/test_replication_utils.py index eff4a0a..1db9f03 100644 --- a/test/test_replication_utils.py +++ b/test/test_replication_utils.py @@ -4,6 +4,8 @@ # # Copyright (C) 2025 Sarah Hoffmann and others. # For a full list of authors see the git log. +import logging + import osmium.replication.utils as rutil from helpers import mkdate @@ -25,3 +27,49 @@ def test_get_replication_header_full(test_data_dir): assert val.url == 'http://download.geofabrik.de/europe/andorra-updates' assert val.sequence == 2167 assert val.timestamp == mkdate(2019, 2, 23, 21, 15, 2) + + +def test_get_replication_header_invalid_sequence(caplog): + from unittest.mock import MagicMock, patch + + mock_header = MagicMock() + mock_header.get.side_effect = lambda k: { + "osmosis_replication_base_url": "https://example.com/replication", + "osmosis_replication_sequence_number": "not-a-number", + "osmosis_replication_timestamp": "2024-01-01T00:00:00Z", + }.get(k) + + mock_reader = MagicMock() + mock_reader.header.return_value = mock_header + + with patch('osmium.replication.utils.oreader', return_value=mock_reader): + with caplog.at_level(logging.WARNING, logger='pyosmium'): + val = rutil.get_replication_header('dummy.pbf') + + assert val.url == 'https://example.com/replication' + assert val.sequence is None + assert val.timestamp is not None + assert 'not-a-number' in caplog.text + + +def test_get_replication_header_negative_sequence(caplog): + from unittest.mock import MagicMock, patch + + mock_header = MagicMock() + mock_header.get.side_effect = lambda k: { + "osmosis_replication_base_url": "https://example.com/replication", + "osmosis_replication_sequence_number": "-5", + "osmosis_replication_timestamp": "2024-01-01T00:00:00Z", + }.get(k) + + mock_reader = MagicMock() + mock_reader.header.return_value = mock_header + + with patch('osmium.replication.utils.oreader', return_value=mock_reader): + with caplog.at_level(logging.WARNING, logger='pyosmium'): + val = rutil.get_replication_header('dummy.pbf') + + assert val.url == 'https://example.com/replication' + assert val.sequence is None + assert val.timestamp is not None + assert '-5' in caplog.text