From a067ff519a4fb04c8a35ca727799cdda4f61deca Mon Sep 17 00:00:00 2001 From: Silvano Cerza Date: Wed, 25 Feb 2026 13:53:09 +0100 Subject: [PATCH] Fix batch requests failing when using prefixed IPs --- ipinfo/handler.py | 61 ++++++------ ipinfo/handler_async.py | 81 +++++++++------- ipinfo/handler_utils.py | 10 ++ tests/handler_async_test.py | 59 +++++++++++- tests/handler_test.py | 186 ++++++++++++++++++++++++++++++++---- 5 files changed, 312 insertions(+), 85 deletions(-) diff --git a/ipinfo/handler.py b/ipinfo/handler.py index 65bf88a..86063ac 100644 --- a/ipinfo/handler.py +++ b/ipinfo/handler.py @@ -2,33 +2,34 @@ Main API client handler for fetching data from the IPinfo service. """ -from ipaddress import IPv4Address, IPv6Address import time +from ipaddress import IPv4Address, IPv6Address import requests -from .error import APIError +from . import handler_utils +from .bogon import is_bogon from .cache.default import DefaultCache +from .data import ( + continents, + countries, + countries_currencies, + countries_flags, + eu_countries, +) from .details import Details +from .error import APIError from .exceptions import RequestQuotaExceededError, TimeoutExceededError from .handler_utils import ( API_URL, - RESPROXY_API_URL, BATCH_MAX_SIZE, + BATCH_REQ_TIMEOUT_DEFAULT, CACHE_MAXSIZE, CACHE_TTL, REQUEST_TIMEOUT_DEFAULT, - BATCH_REQ_TIMEOUT_DEFAULT, + RESPROXY_API_URL, cache_key, -) -from . import handler_utils -from .bogon import is_bogon -from .data import ( - continents, - countries, - countries_currencies, - eu_countries, - countries_flags, + is_prefixed_lookup, ) @@ -91,9 +92,7 @@ def getDetails(self, ip_address=None, timeout=None): # If the supplied IP address uses the objects defined in the built-in # module ipaddress extract the appropriate string notation before # formatting the URL. - if isinstance(ip_address, IPv4Address) or isinstance( - ip_address, IPv6Address - ): + if isinstance(ip_address, IPv4Address) or isinstance(ip_address, IPv6Address): ip_address = ip_address.exploded # check if bogon. @@ -125,11 +124,11 @@ def getDetails(self, ip_address=None, timeout=None): raise RequestQuotaExceededError() if response.status_code >= 400: error_code = response.status_code - content_type = response.headers.get('Content-Type') - if content_type == 'application/json': + content_type = response.headers.get("Content-Type") + if content_type == "application/json": error_response = response.json() else: - error_response = {'error': response.text} + error_response = {"error": response.text} raise APIError(error_code, error_response) details = response.json() @@ -196,7 +195,6 @@ def getResproxy(self, ip_address, timeout=None): return Details(details) - def getBatchDetails( self, ip_addresses, @@ -251,7 +249,11 @@ def getBatchDetails( ): ip_address = ip_address.exploded - if ip_address and is_bogon(ip_address): + if ( + ip_address + and not is_prefixed_lookup(ip_address) + and is_bogon(ip_address) + ): details = {} details["ip"] = ip_address details["bogon"] = True @@ -280,10 +282,7 @@ def getBatchDetails( headers["content-type"] = "application/json" for i in range(0, len(lookup_addresses), batch_size): # quit if total timeout is reached. - if ( - timeout_total is not None - and time.time() - start_time > timeout_total - ): + if timeout_total is not None and time.time() - start_time > timeout_total: return handler_utils.return_or_fail( raise_on_fail, TimeoutExceededError(), result ) @@ -292,9 +291,7 @@ def getBatchDetails( # lookup try: - response = requests.post( - url, json=chunk, headers=headers, **req_opts - ) + response = requests.post(url, json=chunk, headers=headers, **req_opts) except Exception as e: return handler_utils.return_or_fail(raise_on_fail, e, result) @@ -347,9 +344,7 @@ def getMap(self, ips): url = f"{API_URL}/map?cli=1" headers = handler_utils.get_headers(None, self.headers) headers["content-type"] = "application/json" - response = requests.post( - url, json=ip_strs, headers=headers, **req_opts - ) + response = requests.post(url, json=ip_strs, headers=headers, **req_opts) response.raise_for_status() return response.json()["reportUrl"] @@ -370,7 +365,9 @@ def getBatchDetailsIter( ): ip_address = ip_address.exploded - if ip_address and is_bogon(ip_address): + if is_prefixed_lookup(ip_address): + lookup_addresses.append(ip_address) + elif ip_address and is_bogon(ip_address): details = {} details["ip"] = ip_address details["bogon"] = True diff --git a/ipinfo/handler_async.py b/ipinfo/handler_async.py index af6a486..4c5fd47 100644 --- a/ipinfo/handler_async.py +++ b/ipinfo/handler_async.py @@ -2,35 +2,36 @@ Main API client asynchronous handler for fetching data from the IPinfo service. """ -from ipaddress import IPv4Address, IPv6Address import asyncio import json import time +from ipaddress import IPv4Address, IPv6Address import aiohttp -from .error import APIError +from . import handler_utils +from .bogon import is_bogon from .cache.default import DefaultCache +from .data import ( + continents, + countries, + countries_currencies, + countries_flags, + eu_countries, +) from .details import Details +from .error import APIError from .exceptions import RequestQuotaExceededError, TimeoutExceededError from .handler_utils import ( API_URL, - RESPROXY_API_URL, BATCH_MAX_SIZE, + BATCH_REQ_TIMEOUT_DEFAULT, CACHE_MAXSIZE, CACHE_TTL, REQUEST_TIMEOUT_DEFAULT, - BATCH_REQ_TIMEOUT_DEFAULT, + RESPROXY_API_URL, cache_key, -) -from . import handler_utils -from .bogon import is_bogon -from .data import ( - continents, - countries, - countries_currencies, - eu_countries, - countries_flags, + is_prefixed_lookup, ) @@ -117,9 +118,7 @@ async def getDetails(self, ip_address=None, timeout=None): # If the supplied IP address uses the objects defined in the built-in # module ipaddress, extract the appropriate string notation before # formatting the URL. - if isinstance(ip_address, IPv4Address) or isinstance( - ip_address, IPv6Address - ): + if isinstance(ip_address, IPv4Address) or isinstance(ip_address, IPv6Address): ip_address = ip_address.exploded # check if bogon. @@ -147,11 +146,11 @@ async def getDetails(self, ip_address=None, timeout=None): raise RequestQuotaExceededError() if resp.status >= 400: error_code = resp.status - content_type = resp.headers.get('Content-Type') - if content_type == 'application/json': + content_type = resp.headers.get("Content-Type") + if content_type == "application/json": error_response = await resp.json() else: - error_response = {'error': resp.text()} + error_response = {"error": resp.text()} raise APIError(error_code, error_response) details = await resp.json() @@ -277,11 +276,19 @@ async def getBatchDetails( ): ip_address = ip_address.exploded - try: - cached_ipaddr = self.cache[cache_key(ip_address)] - result[ip_address] = cached_ipaddr - except KeyError: - lookup_addresses.append(ip_address) + if ( + ip_address + and not is_prefixed_lookup(ip_address) + and is_bogon(ip_address) + ): + details = {"ip": ip_address, "bogon": True} + result[ip_address] = Details(details) + else: + try: + cached_ipaddr = self.cache[cache_key(ip_address)] + result[ip_address] = cached_ipaddr + except KeyError: + lookup_addresses.append(ip_address) # all in cache - return early. if not lookup_addresses: @@ -296,22 +303,24 @@ async def getBatchDetails( headers = handler_utils.get_headers(self.access_token, self.headers) headers["content-type"] = "application/json" - # prepare coroutines that will make reqs and update results. + # prepare tasks that will make reqs and update results. reqs = [ - self._do_batch_req( - lookup_addresses[i : i + batch_size], - url, - headers, - timeout_per_batch, - raise_on_fail, - result, + asyncio.ensure_future( + self._do_batch_req( + lookup_addresses[i : i + batch_size], + url, + headers, + timeout_per_batch, + raise_on_fail, + result, + ) ) for i in range(0, len(lookup_addresses), batch_size) ] try: _, pending = await asyncio.wait( - {*reqs}, + reqs, timeout=timeout_total, return_when=asyncio.FIRST_EXCEPTION, ) @@ -404,7 +413,11 @@ async def getBatchDetailsIter( ): ip_address = ip_address.exploded - if ip_address and is_bogon(ip_address): + if ( + ip_address + and not is_prefixed_lookup(ip_address) + and is_bogon(ip_address) + ): details = {"ip": ip_address, "bogon": True} yield Details(details) else: diff --git a/ipinfo/handler_utils.py b/ipinfo/handler_utils.py index f37bbfb..b891f6b 100644 --- a/ipinfo/handler_utils.py +++ b/ipinfo/handler_utils.py @@ -136,3 +136,13 @@ def cache_key(k): Transforms a user-input key into a versioned cache key. """ return f"{k}:{CACHE_KEY_VSN}" + + +def is_prefixed_lookup(ip_address): + """ + Check if the address is a prefixed batch lookup (e.g., "resproxy/1.2.3.4", + "lookup/8.8.8.8", "domains/google.com"). + + Prefixed lookups skip bogon checking as they are not plain IP addresses. + """ + return isinstance(ip_address, str) and "/" in ip_address diff --git a/tests/handler_async_test.py b/tests/handler_async_test.py index 0e1a903..00a15fa 100644 --- a/tests/handler_async_test.py +++ b/tests/handler_async_test.py @@ -3,8 +3,9 @@ import sys import aiohttp -import ipinfo import pytest + +import ipinfo from ipinfo import handler_utils from ipinfo.cache.default import DefaultCache from ipinfo.details import Details @@ -367,3 +368,59 @@ def mock_get(*args, **kwargs): # Verify only one API call was made (second was cached) assert call_count == 1 await handler.deinit() + + +class MockBatchResponse(MockResponse): + """MockResponse with raise_for_status for batch endpoint mocking.""" + + def raise_for_status(self): + if self.status >= 400: + raise Exception(f"HTTP {self.status}") + + +@pytest.mark.asyncio +async def test_get_batch_details_with_resproxy(monkeypatch): + """Prefixed lookups like 'resproxy/IP' should not crash in async getBatchDetails.""" + mock_api_response = { + "resproxy/1.2.3.4": {"ip": "1.2.3.4", "service": "example"}, + "8.8.8.8": {"ip": "8.8.8.8", "country": "US"}, + } + + async def mock_post(*args, **kwargs): + return MockBatchResponse( + json.dumps(mock_api_response), + 200, + {"Content-Type": "application/json"}, + ) + + handler = AsyncHandler("test_token") + handler._ensure_aiohttp_ready() + monkeypatch.setattr(handler.httpsess, "post", mock_post) + result = await handler.getBatchDetails(["resproxy/1.2.3.4", "8.8.8.8"]) + assert "resproxy/1.2.3.4" in result + assert "8.8.8.8" in result + await handler.deinit() + + +@pytest.mark.asyncio +async def test_get_batch_details_mixed_resproxy_and_bogon(monkeypatch): + """Async getBatchDetails: mixing prefixed, plain, and bogon IPs.""" + mock_api_response = { + "resproxy/1.2.3.4": {"ip": "1.2.3.4", "service": "ex"}, + "8.8.8.8": {"ip": "8.8.8.8", "country": "US"}, + } + + async def mock_post(*args, **kwargs): + return MockBatchResponse( + json.dumps(mock_api_response), + 200, + {"Content-Type": "application/json"}, + ) + + handler = AsyncHandler("test_token") + handler._ensure_aiohttp_ready() + monkeypatch.setattr(handler.httpsess, "post", mock_post) + result = await handler.getBatchDetails(["resproxy/1.2.3.4", "8.8.8.8", "127.0.0.1"]) + assert "resproxy/1.2.3.4" in result + assert "8.8.8.8" in result + await handler.deinit() diff --git a/tests/handler_test.py b/tests/handler_test.py index 8fe21ae..f8db3fe 100644 --- a/tests/handler_test.py +++ b/tests/handler_test.py @@ -1,14 +1,16 @@ +import json import os +import pytest +import requests + +import ipinfo +from ipinfo import handler_utils from ipinfo.cache.default import DefaultCache from ipinfo.details import Details -from ipinfo.handler import Handler -from ipinfo import handler_utils from ipinfo.error import APIError from ipinfo.exceptions import RequestQuotaExceededError -import ipinfo -import pytest -import requests +from ipinfo.handler import Handler def test_init(): @@ -47,8 +49,7 @@ def test_get_details(): assert country_flag["unicode"] == "U+1F1FA U+1F1F8" country_flag_url = details.country_flag_url assert ( - country_flag_url - == "https://cdn.ipinfo.io/static/images/countries-flags/US.svg" + country_flag_url == "https://cdn.ipinfo.io/static/images/countries-flags/US.svg" ) country_currency = details.country_currency assert country_currency["code"] == "USD" @@ -99,15 +100,45 @@ def test_get_details(): assert "total" in domains assert len(domains["domains"]) == 5 + @pytest.mark.parametrize( - ("mock_resp_status_code", "mock_resp_headers", "mock_resp_error_msg", "expected_error_json"), + ( + "mock_resp_status_code", + "mock_resp_headers", + "mock_resp_error_msg", + "expected_error_json", + ), [ - pytest.param(503, {"Content-Type": "text/plain"}, b"Service Unavailable", {"error": "Service Unavailable"}, id="5xx_not_json"), - pytest.param(403, {"Content-Type": "application/json"}, b'{"message": "missing token"}', {"message": "missing token"}, id="4xx_json"), - pytest.param(400, {"Content-Type": "application/json"}, b'{"message": "missing field"}', {"message": "missing field"}, id="400"), - ] + pytest.param( + 503, + {"Content-Type": "text/plain"}, + b"Service Unavailable", + {"error": "Service Unavailable"}, + id="5xx_not_json", + ), + pytest.param( + 403, + {"Content-Type": "application/json"}, + b'{"message": "missing token"}', + {"message": "missing token"}, + id="4xx_json", + ), + pytest.param( + 400, + {"Content-Type": "application/json"}, + b'{"message": "missing field"}', + {"message": "missing field"}, + id="400", + ), + ], ) -def test_get_details_error(monkeypatch, mock_resp_status_code, mock_resp_headers, mock_resp_error_msg, expected_error_json): +def test_get_details_error( + monkeypatch, + mock_resp_status_code, + mock_resp_headers, + mock_resp_error_msg, + expected_error_json, +): def mock_get(*args, **kwargs): response = requests.Response() response.status_code = mock_resp_status_code @@ -115,7 +146,7 @@ def mock_get(*args, **kwargs): response._content = mock_resp_error_msg return response - monkeypatch.setattr(requests, 'get', mock_get) + monkeypatch.setattr(requests, "get", mock_get) token = os.environ.get("IPINFO_TOKEN", "") handler = Handler(token) @@ -124,19 +155,21 @@ def mock_get(*args, **kwargs): assert exc_info.value.error_code == mock_resp_status_code assert exc_info.value.error_json == expected_error_json + def test_get_details_quota_error(monkeypatch): def mock_get(*args, **kwargs): response = requests.Response() response.status_code = 429 return response - monkeypatch.setattr(requests, 'get', mock_get) + monkeypatch.setattr(requests, "get", mock_get) token = os.environ.get("IPINFO_TOKEN", "") handler = Handler(token) with pytest.raises(RequestQuotaExceededError): handler.getDetails("8.8.8.8") + ############# # BATCH TESTS ############# @@ -193,9 +226,7 @@ def test_get_batch_details(batch_size): def test_get_batch_details_total_timeout(batch_size): handler, token, ips = _prepare_batch_test() with pytest.raises(ipinfo.exceptions.TimeoutExceededError): - handler.getBatchDetails( - ips, batch_size=batch_size, timeout_total=0.001 - ) + handler.getBatchDetails(ips, batch_size=batch_size, timeout_total=0.001) @pytest.mark.parametrize("batch_size", [None, 1, 2, 3]) @@ -300,3 +331,122 @@ def mock_get(*args, **kwargs): details = handler.getResproxy("8.8.8.8") assert isinstance(details, Details) assert details.all == {} + + +def test_get_batch_details_with_resproxy(monkeypatch): + """Prefixed lookups like 'resproxy/IP' should not crash in getBatchDetails.""" + mock_api_response = { + "resproxy/1.2.3.4": {"ip": "1.2.3.4", "service": "example"}, + "8.8.8.8": {"ip": "8.8.8.8", "country": "US"}, + } + + def mock_post(*args, **kwargs): + response = requests.Response() + response.status_code = 200 + response.headers = {"Content-Type": "application/json"} + response._content = json.dumps(mock_api_response).encode() + return response + + monkeypatch.setattr(requests, "post", mock_post) + handler = Handler("test_token") + result = handler.getBatchDetails(["resproxy/1.2.3.4", "8.8.8.8"]) + assert "resproxy/1.2.3.4" in result + assert "8.8.8.8" in result + + +def test_get_batch_details_resproxy_skips_bogon(monkeypatch): + """'resproxy/127.0.0.1' should NOT be treated as a bogon.""" + posted_data = [] + + def mock_post(*args, **kwargs): + posted_data.append(kwargs.get("json", [])) + response = requests.Response() + response.status_code = 200 + response.headers = {"Content-Type": "application/json"} + response._content = json.dumps( + {"resproxy/127.0.0.1": {"ip": "127.0.0.1"}} + ).encode() + return response + + monkeypatch.setattr(requests, "post", mock_post) + handler = Handler("test_token") + result = handler.getBatchDetails(["resproxy/127.0.0.1"]) + + # The prefixed string should have been sent to the API, not treated as bogon + assert len(posted_data) == 1 + assert "resproxy/127.0.0.1" in posted_data[0] + assert "resproxy/127.0.0.1" in result + # Should NOT have bogon flag + assert result["resproxy/127.0.0.1"].get("bogon") is None + + +def test_get_batch_details_resproxy_caching(monkeypatch): + """Prefixed lookups should be cached after the first batch call.""" + call_count = 0 + + def mock_post(*args, **kwargs): + nonlocal call_count + call_count += 1 + response = requests.Response() + response.status_code = 200 + response.headers = {"Content-Type": "application/json"} + response._content = json.dumps( + {"resproxy/1.2.3.4": {"ip": "1.2.3.4", "service": "example"}} + ).encode() + return response + + monkeypatch.setattr(requests, "post", mock_post) + handler = Handler("test_token") + + # First call should hit the API + result1 = handler.getBatchDetails(["resproxy/1.2.3.4"]) + assert "resproxy/1.2.3.4" in result1 + + # Second call should use cache, no additional API call + result2 = handler.getBatchDetails(["resproxy/1.2.3.4"]) + assert "resproxy/1.2.3.4" in result2 + assert call_count == 1 + + +def test_get_batch_details_iter_with_resproxy(monkeypatch): + """getBatchDetailsIter should handle prefixed lookups without crashing.""" + + def mock_post(*args, **kwargs): + response = requests.Response() + response.status_code = 200 + response.headers = {"Content-Type": "application/json"} + response._content = json.dumps( + {"resproxy/1.2.3.4": {"ip": "1.2.3.4", "service": "example"}} + ).encode() + return response + + monkeypatch.setattr(requests, "post", mock_post) + handler = Handler("test_token") + results = list(handler.getBatchDetailsIter(["resproxy/1.2.3.4"])) + assert len(results) > 0 + + +def test_get_batch_details_mixed_resproxy_and_bogon(monkeypatch): + """Mixing prefixed lookups, plain IPs, and bogons in one batch call.""" + + def mock_post(*args, **kwargs): + response = requests.Response() + response.status_code = 200 + response.headers = {"Content-Type": "application/json"} + response._content = json.dumps( + { + "resproxy/1.2.3.4": {"ip": "1.2.3.4", "service": "ex"}, + "8.8.8.8": {"ip": "8.8.8.8", "country": "US"}, + } + ).encode() + return response + + monkeypatch.setattr(requests, "post", mock_post) + handler = Handler("test_token") + result = handler.getBatchDetails(["resproxy/1.2.3.4", "8.8.8.8", "127.0.0.1"]) + assert "resproxy/1.2.3.4" in result + assert "8.8.8.8" in result + assert "127.0.0.1" in result + bogon_result = result["127.0.0.1"] + assert isinstance(bogon_result, Details) + assert bogon_result.bogon is True