From 872bab0db86a6fcd856a21e9c552b877dfe11a81 Mon Sep 17 00:00:00 2001 From: Robert Martin Date: Thu, 13 Aug 2026 11:11:20 -0500 Subject: [PATCH 1/7] Send request headers on the file-extension pre-flight request guess_url_file_extension() made its own GET request to sniff the Content-Type before the real download, but never carried any custom headers. On a header-gated host (e.g. one requiring Referer) that pre-flight request would fail even after headers are wired through the rest of the download path. --- openaddr/cache.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openaddr/cache.py b/openaddr/cache.py index d1ce4f70..b190abb8 100644 --- a/openaddr/cache.py +++ b/openaddr/cache.py @@ -145,7 +145,7 @@ def from_protocol_string(clz, protocol_string, source_prefix=None): def download(self, source_urls, workdir, source_config): raise NotImplementedError() -def guess_url_file_extension(url): +def guess_url_file_extension(url, headers=None): ''' Get a filename extension for a URL using various hints. ''' scheme, _, path, _, query, _ = urlparse(url) @@ -172,7 +172,7 @@ def guess_url_file_extension(url): # Get a dictionary of headers and a few bytes of content from the URL. # if scheme in ('http', 'https'): - response = request('GET', url, stream=True) + response = request('GET', url, headers=headers or {}, stream=True) handle, file = mkstemp() for chunk in response.iter_content(chunk_size=8192): @@ -256,7 +256,7 @@ def get_file_path(self, url, dir_path): hash = sha1((host + path_base).encode('utf-8')) name_base = u'{}-{}'.format(self.source_prefix, hash.hexdigest()[:8]) - path_ext = guess_url_file_extension(url) + path_ext = guess_url_file_extension(url, self.headers) _L.debug(u'Guessed {}{} for {}'.format(name_base, path_ext, url)) return os.path.join(dir_path, name_base + path_ext) From 4e16cec28a367ca9d0747a2468585c1c4007b6ea Mon Sep 17 00:00:00 2001 From: Robert Martin Date: Thu, 13 Aug 2026 11:11:42 -0500 Subject: [PATCH 2/7] Forward custom headers through DownloadTask construction and Esri downloads DownloadTask.__init__ already accepted a headers dict, but from_protocol_string() - the only production call site - never passed one through, so self.headers was always just the default User-Agent. EsriRestDownloadTask also built its EsriDumper without headers despite pyesridump already supporting extra_headers. --- openaddr/cache.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/openaddr/cache.py b/openaddr/cache.py index b190abb8..939ba5f6 100644 --- a/openaddr/cache.py +++ b/openaddr/cache.py @@ -130,15 +130,16 @@ def __init__(self, source_prefix, params={}, headers={}): @classmethod - def from_protocol_string(clz, protocol_string, source_prefix=None): + def from_protocol_string(clz, protocol_string, source_prefix=None, headers=None): + headers = headers or {} if protocol_string.lower() == 'http': - return URLDownloadTask(source_prefix) + return URLDownloadTask(source_prefix, headers=headers) elif protocol_string.lower() == 'file': - return URLDownloadTask(source_prefix) + return URLDownloadTask(source_prefix, headers=headers) elif protocol_string.lower() == 'ftp': - return URLDownloadTask(source_prefix) + return URLDownloadTask(source_prefix, headers=headers) elif protocol_string.lower() == 'esri': - return EsriRestDownloadTask(source_prefix) + return EsriRestDownloadTask(source_prefix, headers=headers) else: raise KeyError("I don't know how to extract for protocol {}".format(protocol_string)) @@ -391,7 +392,7 @@ def download(self, source_urls, workdir, source_config): _L.debug("File exists %s", file_path) continue - downloader = EsriDumper(source_url, parent_logger=_L, timeout=300) + downloader = EsriDumper(source_url, parent_logger=_L, timeout=300, extra_headers=self.headers) metadata = downloader.get_metadata() From 01c02887978266c329fcfcb9a7154330ce1c2cbd Mon Sep 17 00:00:00 2001 From: Robert Martin Date: Thu, 13 Aug 2026 11:12:10 -0500 Subject: [PATCH 3/7] Read source-config headers in cache() and pass them to the download task Wires the source-supplied 'headers' dict (added to the schema separately in openaddresses/openaddresses) into the download path. conform() deliberately does not forward headers: it re-downloads from the OA-owned cache artifact, not the contributor's host. --- openaddr/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/openaddr/__init__.py b/openaddr/__init__.py index b3fc1098..fe873c45 100644 --- a/openaddr/__init__.py +++ b/openaddr/__init__.py @@ -75,8 +75,9 @@ def cache(source_config, destdir, extras): source_urls = [source_urls] protocol_string = source_config.data_source.get('protocol') + source_headers = source_config.data_source.get('headers') or {} - task = DownloadTask.from_protocol_string(protocol_string, source_config) + task = DownloadTask.from_protocol_string(protocol_string, source_config, headers=source_headers) downloaded_files = task.download(source_urls, workdir, source_config) # FIXME: I wrote the download stuff to assume multiple files because @@ -128,6 +129,9 @@ def conform(source_config, destdir, extras): if not isinstance(source_urls, list): source_urls = [source_urls] + # source_config.data_source['headers'] is intentionally not passed here: + # this re-downloads from the OA-owned cache artifact (S3), not the + # contributor's original host, so contributor-supplied headers don't apply. task1 = URLDownloadTask(source_config.data_source_name) downloaded_path = task1.download(source_urls, workdir, source_config) _L.info("Downloaded to %s", downloaded_path) From f7009ed110280ed3601b0a423752cbb10da86c7c Mon Sep 17 00:00:00 2001 From: Robert Martin Date: Thu, 13 Aug 2026 11:14:25 -0500 Subject: [PATCH 4/7] Add tests for custom header propagation through DownloadTask Covers: from_protocol_string() forwarding headers to URLDownloadTask and EsriRestDownloadTask, default User-Agent survives alongside custom headers (and can be overridden), headers reaching both the extension-guessing pre-flight request and the real download request, and EsriRestDownloadTask passing headers to EsriDumper as extra_headers. --- openaddr/tests/cache.py | 94 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 1 deletion(-) diff --git a/openaddr/tests/cache.py b/openaddr/tests/cache.py index 37ccafef..2f1891ff 100644 --- a/openaddr/tests/cache.py +++ b/openaddr/tests/cache.py @@ -16,7 +16,13 @@ import httmock import tempfile -from ..cache import guess_url_file_extension, EsriRestDownloadTask +import sys +from ..cache import guess_url_file_extension, EsriRestDownloadTask, URLDownloadTask, DownloadTask + +# openaddr/__init__.py defines a `cache` function that shadows the `cache` +# submodule on the `openaddr` package object, so `from .. import cache` +# would grab the function, not the module. Go through sys.modules instead. +cache_module = sys.modules['openaddr.cache'] class TestCacheExtensionGuessing (unittest.TestCase): @@ -392,3 +398,89 @@ def test_handle_feature_server_with_lat_lon_in_conform(self): self.assertEqual(len(all_data), 5) self.assertTrue('oa:geom' in all_data[0]) self.assertEqual(all_data[0]['oa:geom'], 'POINT (-86.82960553 34.18671398)') + +class TestFromProtocolStringHeaders (unittest.TestCase): + + def test_headers_reach_url_download_task(self): + task = DownloadTask.from_protocol_string('http', 'us-il-champaign', headers={'Referer': 'https://example.gov/'}) + self.assertIsInstance(task, URLDownloadTask) + self.assertEqual(task.headers['Referer'], 'https://example.gov/') + # The default User-Agent is still present alongside custom headers. + self.assertIn('User-Agent', task.headers) + + def test_headers_reach_esri_download_task(self): + task = DownloadTask.from_protocol_string('ESRI', 'us-il-champaign', headers={'Referer': 'https://example.gov/'}) + self.assertIsInstance(task, EsriRestDownloadTask) + self.assertEqual(task.headers['Referer'], 'https://example.gov/') + + def test_headers_reach_esri_dumper(self): + ''' EsriRestDownloadTask.download() must pass its headers to + EsriDumper as extra_headers, since pyesridump makes its own + requests independent of openaddr.cache.request(). + ''' + workdir = tempfile.mkdtemp(prefix='testCacheHeaders-') + try: + task = EsriRestDownloadTask('us-fl-palmbeach', headers={'Referer': 'https://example.gov/'}) + c = SourceConfig(dict({ + "schema": 2, + "layers": { + "addresses": [{ + "name": "default", + "conform": None + }] + } + }), "addresses", "default") + + with patch.object(cache_module, 'EsriDumper') as dumper_patch: + dumper_patch.return_value.get_metadata.return_value = {'fields': []} + dumper_patch.return_value.get_feature_count.return_value = 0 + dumper_patch.return_value.__iter__.return_value = iter([]) + + task.download(['http://example.com/'], workdir, c) + + _, kwargs = dumper_patch.call_args + self.assertEqual(kwargs.get('extra_headers'), task.headers) + self.assertEqual(kwargs['extra_headers']['Referer'], 'https://example.gov/') + finally: + shutil.rmtree(workdir) + + def test_no_headers_still_gets_default_user_agent(self): + task = DownloadTask.from_protocol_string('http', 'us-il-champaign') + self.assertEqual(list(task.headers.keys()), ['User-Agent']) + + def test_custom_user_agent_overrides_default(self): + task = DownloadTask.from_protocol_string('http', 'us-il-champaign', headers={'User-Agent': 'custom-agent/1.0'}) + self.assertEqual(task.headers['User-Agent'], 'custom-agent/1.0') + +class TestURLDownloadTaskHeaders (unittest.TestCase): + ''' Confirm that a source's custom headers are sent on both the + file-extension pre-flight request and the real download request. + ''' + + def setUp(self): + self.workdir = tempfile.mkdtemp(prefix='testCacheHeaders-') + self.seen_referers = [] + + def tearDown(self): + shutil.rmtree(self.workdir) + + def response_content(self, url, request): + scheme, host, path, _, query, _ = urlparse(url.geturl()) + + # A query string forces guess_url_file_extension() to make a + # sniffing request instead of trusting the URL's extension, + # so this URL exercises both the pre-flight and real download. + if (host, path, query) == ('headers-test.local', '/addresses.csv', 'download=true'): + self.seen_referers.append(request.headers.get('Referer')) + return httmock.response(200, b'FAKE,FAKE\n', headers={'Content-Type': 'text/csv'}) + + raise NotImplementedError(url.geturl()) + + def test_headers_sent_on_preflight_and_download_requests(self): + task = URLDownloadTask('us-il-champaign', headers={'Referer': 'https://example.gov/gis/'}) + with httmock.HTTMock(self.response_content): + output_files = task.download(['http://headers-test.local/addresses.csv?download=true'], self.workdir, None) + + self.assertEqual(len(output_files), 1) + # One request for the extension-guessing pre-flight, one for the real download. + self.assertEqual(self.seen_referers, ['https://example.gov/gis/', 'https://example.gov/gis/']) From 45a083ea42122b7374279f0961a41858e037a32d Mon Sep 17 00:00:00 2001 From: Robert Martin Date: Thu, 13 Aug 2026 11:14:53 -0500 Subject: [PATCH 5/7] Bump version to 10.1.0 Fill in the CHANGELOG's PR link once this is merged. --- CHANGELOG | 3 +++ openaddr/VERSION | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 898cf584..6f98e89b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,6 @@ +2026-08-13 v10.1.0 +- Support custom HTTP request headers for a source, including on the file-extension pre-flight request (needed for downloads gated on Referer/etc.) + 2026-03-27 v10.0.0 - Upgrade base Docker image from GDAL 3.7.1 to 3.11.0 https://github.com/openaddresses/batch-machine/pull/99 - Update SSL CA certificates to fix download failures https://github.com/openaddresses/batch-machine/pull/98 diff --git a/openaddr/VERSION b/openaddr/VERSION index a13e7b9c..4149c39e 100644 --- a/openaddr/VERSION +++ b/openaddr/VERSION @@ -1 +1 @@ -10.0.0 +10.1.0 From 74c8dbdc8f8a431d1ef8d6fea431c10f8b2ef77f Mon Sep 17 00:00:00 2001 From: Robert Martin Date: Thu, 13 Aug 2026 13:20:36 -0500 Subject: [PATCH 6/7] Nest source headers under http_request_settings Review feedback on openaddresses/openaddresses#8306: wrapping headers in a container leaves room for query params (e.g. a token) later without a second schema migration. No source uses the flat key yet, so this costs nothing now. --- CHANGELOG | 2 +- openaddr/__init__.py | 10 ++++--- openaddr/tests/cache.py | 58 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 6f98e89b..c4119dc8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,5 @@ 2026-08-13 v10.1.0 -- Support custom HTTP request headers for a source, including on the file-extension pre-flight request (needed for downloads gated on Referer/etc.) +- Support custom HTTP request headers for a source via `http_request_settings.headers`, including on the file-extension pre-flight request (needed for downloads gated on Referer/etc.) 2026-03-27 v10.0.0 - Upgrade base Docker image from GDAL 3.7.1 to 3.11.0 https://github.com/openaddresses/batch-machine/pull/99 diff --git a/openaddr/__init__.py b/openaddr/__init__.py index fe873c45..a2754ca8 100644 --- a/openaddr/__init__.py +++ b/openaddr/__init__.py @@ -75,7 +75,8 @@ def cache(source_config, destdir, extras): source_urls = [source_urls] protocol_string = source_config.data_source.get('protocol') - source_headers = source_config.data_source.get('headers') or {} + http_request_settings = source_config.data_source.get('http_request_settings') or {} + source_headers = http_request_settings.get('headers') or {} task = DownloadTask.from_protocol_string(protocol_string, source_config, headers=source_headers) downloaded_files = task.download(source_urls, workdir, source_config) @@ -129,9 +130,10 @@ def conform(source_config, destdir, extras): if not isinstance(source_urls, list): source_urls = [source_urls] - # source_config.data_source['headers'] is intentionally not passed here: - # this re-downloads from the OA-owned cache artifact (S3), not the - # contributor's original host, so contributor-supplied headers don't apply. + # source_config.data_source['http_request_settings'] is intentionally not + # passed here: this re-downloads from the OA-owned cache artifact (S3), + # not the contributor's original host, so contributor-supplied headers + # don't apply. task1 = URLDownloadTask(source_config.data_source_name) downloaded_path = task1.download(source_urls, workdir, source_config) _L.info("Downloaded to %s", downloaded_path) diff --git a/openaddr/tests/cache.py b/openaddr/tests/cache.py index 2f1891ff..83fe0e77 100644 --- a/openaddr/tests/cache.py +++ b/openaddr/tests/cache.py @@ -3,6 +3,7 @@ import csv from .. import SourceConfig +from .. import cache as cache_fn from urllib.parse import urlparse, parse_qs from os.path import join, dirname @@ -484,3 +485,60 @@ def test_headers_sent_on_preflight_and_download_requests(self): self.assertEqual(len(output_files), 1) # One request for the extension-guessing pre-flight, one for the real download. self.assertEqual(self.seen_referers, ['https://example.gov/gis/', 'https://example.gov/gis/']) + +class TestCacheHttpRequestSettings (unittest.TestCase): + ''' Confirm that openaddr.cache() reads headers from the nested + http_request_settings.headers key in the source config, and that a + source with no http_request_settings at all still works. + ''' + + def setUp(self): + self.destdir = tempfile.mkdtemp(prefix='testCacheHttpSettings-') + self.seen_referers = [] + + def tearDown(self): + shutil.rmtree(self.destdir) + + def response_content(self, url, request): + scheme, host, path, _, query, _ = urlparse(url.geturl()) + + # A query string forces guess_url_file_extension() to make a + # sniffing request instead of trusting the URL's extension, + # so this URL exercises both the pre-flight and real download. + if (host, path, query) == ('http-request-settings-test.local', '/addresses.csv', 'download=true'): + self.seen_referers.append(request.headers.get('Referer')) + return httmock.response(200, b'FAKE,FAKE\n', headers={'Content-Type': 'text/csv'}) + + raise NotImplementedError(url.geturl()) + + def make_source_config(self, layersource_extra): + return SourceConfig(dict({ + "schema": 2, + "layers": { + "addresses": [dict({ + "name": "default", + "protocol": "http", + "data": "http://http-request-settings-test.local/addresses.csv?download=true", + }, **layersource_extra)] + } + }), "addresses", "default") + + def test_cache_reads_headers_from_http_request_settings(self): + source_config = self.make_source_config({ + "http_request_settings": { + "headers": {"Referer": "https://example.gov/gis/"} + } + }) + + with httmock.HTTMock(self.response_content): + cache_fn(source_config, self.destdir, {}) + + self.assertEqual(self.seen_referers, ['https://example.gov/gis/', 'https://example.gov/gis/']) + + def test_cache_without_http_request_settings_sends_no_referer(self): + source_config = self.make_source_config({}) + + with httmock.HTTMock(self.response_content): + cache_fn(source_config, self.destdir, {}) + + self.assertEqual(self.seen_referers, [None, None]) From 8d8fb59a85a11011463c9627541ac172fb8d1218 Mon Sep 17 00:00:00 2001 From: Robert Martin Date: Thu, 13 Aug 2026 17:58:23 -0500 Subject: [PATCH 7/7] Rename http_request_settings to request Further review feedback on openaddresses/openaddresses#8306: iandees agreed 'request' is shorter and reads better than http_request_settings. Source key is now request.headers. --- CHANGELOG | 2 +- openaddr/__init__.py | 12 ++++++------ openaddr/tests/cache.py | 18 +++++++++--------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index c4119dc8..566e813b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,5 @@ 2026-08-13 v10.1.0 -- Support custom HTTP request headers for a source via `http_request_settings.headers`, including on the file-extension pre-flight request (needed for downloads gated on Referer/etc.) +- Support custom HTTP request headers for a source via `request.headers`, including on the file-extension pre-flight request (needed for downloads gated on Referer/etc.) 2026-03-27 v10.0.0 - Upgrade base Docker image from GDAL 3.7.1 to 3.11.0 https://github.com/openaddresses/batch-machine/pull/99 diff --git a/openaddr/__init__.py b/openaddr/__init__.py index a2754ca8..f16cbe9a 100644 --- a/openaddr/__init__.py +++ b/openaddr/__init__.py @@ -75,8 +75,8 @@ def cache(source_config, destdir, extras): source_urls = [source_urls] protocol_string = source_config.data_source.get('protocol') - http_request_settings = source_config.data_source.get('http_request_settings') or {} - source_headers = http_request_settings.get('headers') or {} + request_settings = source_config.data_source.get('request') or {} + source_headers = request_settings.get('headers') or {} task = DownloadTask.from_protocol_string(protocol_string, source_config, headers=source_headers) downloaded_files = task.download(source_urls, workdir, source_config) @@ -130,10 +130,10 @@ def conform(source_config, destdir, extras): if not isinstance(source_urls, list): source_urls = [source_urls] - # source_config.data_source['http_request_settings'] is intentionally not - # passed here: this re-downloads from the OA-owned cache artifact (S3), - # not the contributor's original host, so contributor-supplied headers - # don't apply. + # source_config.data_source['request'] is intentionally not passed here: + # this re-downloads from the OA-owned cache artifact (S3), not the + # contributor's original host, so contributor-supplied headers don't + # apply. task1 = URLDownloadTask(source_config.data_source_name) downloaded_path = task1.download(source_urls, workdir, source_config) _L.info("Downloaded to %s", downloaded_path) diff --git a/openaddr/tests/cache.py b/openaddr/tests/cache.py index 83fe0e77..ad2d947e 100644 --- a/openaddr/tests/cache.py +++ b/openaddr/tests/cache.py @@ -486,14 +486,14 @@ def test_headers_sent_on_preflight_and_download_requests(self): # One request for the extension-guessing pre-flight, one for the real download. self.assertEqual(self.seen_referers, ['https://example.gov/gis/', 'https://example.gov/gis/']) -class TestCacheHttpRequestSettings (unittest.TestCase): +class TestCacheRequestSettings (unittest.TestCase): ''' Confirm that openaddr.cache() reads headers from the nested - http_request_settings.headers key in the source config, and that a - source with no http_request_settings at all still works. + request.headers key in the source config, and that a source with + no request settings at all still works. ''' def setUp(self): - self.destdir = tempfile.mkdtemp(prefix='testCacheHttpSettings-') + self.destdir = tempfile.mkdtemp(prefix='testCacheRequestSettings-') self.seen_referers = [] def tearDown(self): @@ -505,7 +505,7 @@ def response_content(self, url, request): # A query string forces guess_url_file_extension() to make a # sniffing request instead of trusting the URL's extension, # so this URL exercises both the pre-flight and real download. - if (host, path, query) == ('http-request-settings-test.local', '/addresses.csv', 'download=true'): + if (host, path, query) == ('request-settings-test.local', '/addresses.csv', 'download=true'): self.seen_referers.append(request.headers.get('Referer')) return httmock.response(200, b'FAKE,FAKE\n', headers={'Content-Type': 'text/csv'}) @@ -518,14 +518,14 @@ def make_source_config(self, layersource_extra): "addresses": [dict({ "name": "default", "protocol": "http", - "data": "http://http-request-settings-test.local/addresses.csv?download=true", + "data": "http://request-settings-test.local/addresses.csv?download=true", }, **layersource_extra)] } }), "addresses", "default") - def test_cache_reads_headers_from_http_request_settings(self): + def test_cache_reads_headers_from_request_settings(self): source_config = self.make_source_config({ - "http_request_settings": { + "request": { "headers": {"Referer": "https://example.gov/gis/"} } }) @@ -535,7 +535,7 @@ def test_cache_reads_headers_from_http_request_settings(self): self.assertEqual(self.seen_referers, ['https://example.gov/gis/', 'https://example.gov/gis/']) - def test_cache_without_http_request_settings_sends_no_referer(self): + def test_cache_without_request_settings_sends_no_referer(self): source_config = self.make_source_config({}) with httmock.HTTMock(self.response_content):