Skip to content

Commit d8762bb

Browse files
committed
checks.network: key shared URL futures by verification method too
The https probe for http://foo.org was stored under https://foo.org, so another package's https://foo.org check reused it: it was told https was available, and a dead URL went unreported. Signed-off-by: Arthur Zamarin <arthurzam@gentoo.org>
1 parent c3fd9a2 commit d8762bb

3 files changed

Lines changed: 54 additions & 7 deletions

File tree

‎NEWS.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ pkgcheck 0.10.49 (unreleased)
2121
- SuspiciousSrcUriChange: no longer reported when a SRC_URI only switches from
2222
``http://`` to ``https://`` (Arthur Zamarin)
2323

24+
- HttpsUrlAvailable, DeadUrl: a package using an ``https://`` URL that another
25+
package uses as ``http://`` is no longer told https is available, and is
26+
reported as DeadUrl when that URL is dead (Arthur Zamarin)
27+
2428
-----------------------------
2529
pkgcheck 0.10.48 (2026-10-08)
2630
-----------------------------

‎src/pkgcheck/checks/network.py‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -207,13 +207,16 @@ def _schedule_check(self, func, attr, url, executor, futures, **kwargs):
207207
208208
Note that this tries to avoid hitting the network for the same URL
209209
twice using a mapping from requested URLs to future objects, adding
210-
result-checking callbacks to the futures of existing URLs.
210+
result-checking callbacks to the futures of existing URLs. Futures are
211+
keyed by verification method too, so an https:// probe for an http://
212+
URL isn't mistaken for a check of the https:// URL itself.
211213
"""
212-
future = futures.get(url)
214+
key = (func.__name__, url)
215+
future = futures.get(key)
213216
if future is None:
214217
future = executor.submit(func, attr, url, **kwargs)
215218
future.add_done_callback(partial(self.task_done, None, None))
216-
futures[url] = future
219+
futures[key] = future
217220
else:
218221
future.add_done_callback(partial(self.task_done, kwargs["pkg"], attr))
219222

@@ -234,7 +237,7 @@ def schedule(self, pkg, executor, futures):
234237
if url.startswith("http://")
235238
)
236239
for attr, orig_url, url in http_to_https_urls:
237-
future = futures[orig_url]
240+
future = futures[(self._http_check.__name__, orig_url)]
238241
self._schedule_check(
239242
self._https_available_check,
240243
attr,
@@ -421,11 +424,12 @@ def _schedule_check(self, filename, url, executor, futures, **kwargs):
421424
twice using a mapping from requested URLs to future objects, adding
422425
result-checking callbacks to the futures of existing URLs.
423426
"""
424-
future = futures.get(url)
427+
key = (self._provenance_check.__name__, url)
428+
future = futures.get(key)
425429
if future is None:
426430
future = executor.submit(self._provenance_check, filename, url, **kwargs)
427431
future.add_done_callback(partial(self.task_done, None, None))
428-
futures[url] = future
432+
futures[key] = future
429433
else:
430434
future.add_done_callback(partial(self.task_done, kwargs["pkg"], filename))
431435

‎tests/checks/test_network.py‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
import importlib.util
22
import os
3+
import queue
34
import tempfile
45
import urllib.request
6+
from concurrent.futures import ThreadPoolExecutor
7+
from contextlib import contextmanager
58
from functools import partial
69
from operator import attrgetter
10+
from types import SimpleNamespace
711
from unittest.mock import patch
812

913
import pytest
14+
from pkgcore.test.misc import FakePkg
1015
from snakeoil.formatters import PlainTextFormatter
1116

1217
from pkgcheck import objects, reporters, scan
1318
from pkgcheck.base import PkgcheckUserException
14-
from pkgcheck.checks import NetworkCheck
19+
from pkgcheck.checks import NetworkCheck, network
1520
from pkgcheck.checks.network import DeadUrl, FetchablesUrlCheck, HomepageUrlCheck
1621
from pkgcheck.packages import RawCPV
1722

@@ -140,3 +145,37 @@ def test_scan_ftp(self, check, result):
140145
else:
141146
assert results == [expected_result]
142147
assert self._render_results(results), "failed rendering results"
148+
149+
150+
@pytest.mark.parametrize("https_alive", (True, False))
151+
def test_https_probe_not_shared_with_https_url(https_alive):
152+
"""An http:// URL's https probe and another package's https:// URL are separate checks."""
153+
154+
@contextmanager
155+
def get(url, **kwargs):
156+
if url.startswith("https://") and not https_alive:
157+
raise network.RequestError(None, "dead https")
158+
yield SimpleNamespace(history=[])
159+
160+
check = object.__new__(HomepageUrlCheck)
161+
check.session = SimpleNamespace(get=get)
162+
check.results_q = queue.SimpleQueue()
163+
futures = {}
164+
http_pkg = FakePkg("cat/http-1", data={"HOMEPAGE": "http://foo.org/"})
165+
https_pkg = FakePkg("cat/https-1", data={"HOMEPAGE": "https://foo.org/"})
166+
with ThreadPoolExecutor(max_workers=1) as executor:
167+
for pkg in (http_pkg, https_pkg):
168+
check.schedule(pkg, executor, futures)
169+
170+
results = []
171+
while not check.results_q.empty():
172+
results.extend(check.results_q.get())
173+
if https_alive:
174+
expected = [
175+
network.HttpsUrlAvailable(
176+
"HOMEPAGE", "http://foo.org/", "https://foo.org/", pkg=http_pkg
177+
)
178+
]
179+
else:
180+
expected = [network.DeadUrl("HOMEPAGE", "https://foo.org/", "dead https", pkg=https_pkg)]
181+
assert results == expected

0 commit comments

Comments
 (0)