diff --git a/api/extensions.py b/api/extensions.py index be14155e..8c86514a 100644 --- a/api/extensions.py +++ b/api/extensions.py @@ -6,11 +6,13 @@ It is disabled by default and never executes or fetches third-party URLs. """ import html +import http.client import json import math import logging import os import re +import socket import threading from pathlib import Path from typing import Any, Dict, List, Optional, Set, Tuple @@ -19,7 +21,11 @@ import hashlib import io import time import zipfile -from urllib.request import HTTPRedirectHandler, build_opener, urlopen +from urllib.request import ( + HTTPRedirectHandler, + HTTPSHandler, + build_opener, +) from api.helpers import _security_headers, j @@ -108,9 +114,80 @@ class _AllowlistRedirectHandler(HTTPRedirectHandler): return super().redirect_request(req, fp, code, msg, headers, newurl) +def _connect_ipv4_first(address, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, source_address=None): + """Connect to *address*, preferring IPv4 to avoid dead-route stalls. + + Some networks advertise IPv6 routes (router advertisements, tunnel brokers) + that do not actually reach the public internet. ``socket.create_connection`` + follows the OS address ordering, which typically tries IPv6 first; each dead + address stalls for the full TCP timeout (~75–130 s) before falling back to + IPv4. This makes gallery installs and registry fetches appear to hang until + the browser or curl timeout kills them. + + We iterate families in IPv4→IPv6 order so the common case (IPv4 works, IPv6 + is a broken route) completes instantly. IPv6 is still tried as a fallback so + IPv6-only networks are unaffected. + + Signature matches ``socket.create_connection`` including the + ``_GLOBAL_DEFAULT_TIMEOUT`` sentinel so callers that rely on the stdlib + default-timeout behaviour are not broken. + """ + host, port = address + if timeout is socket._GLOBAL_DEFAULT_TIMEOUT: + timeout = socket.getdefaulttimeout() + last_error: OSError | None = None + for family in (socket.AF_INET, socket.AF_INET6): + try: + infos = socket.getaddrinfo(host, port, family, socket.SOCK_STREAM) + except socket.gaierror: + continue + for _fam, socktype, proto, _canon, sockaddr in infos: + sock = None + try: + sock = socket.socket(_fam, socktype, proto) + if timeout is not None: + sock.settimeout(timeout) + if source_address is not None: + sock.bind(source_address) + sock.connect(sockaddr) + return sock + except OSError as exc: + last_error = exc + if sock is not None: + sock.close() + if last_error is not None: + raise last_error + raise OSError(f"Could not connect to {host}:{port}") + + +class _IPv4FirstHTTPSConnection(http.client.HTTPSConnection): + """HTTPS connection that resolves IPv4 addresses before IPv6. + + Overrides the class-level ``_create_connection`` that + ``HTTPConnection.connect()`` calls — the stdlib extension point designed for + exactly this kind of per-class customisation (added in Python 3.9, + bpo-37830). No global state is mutated, so the change is thread-safe under + ``ThreadingHTTPServer``. + """ + + _create_connection = staticmethod(_connect_ipv4_first) + + +class _IPv4FirstHTTPSHandler(HTTPSHandler): + """HTTPS handler that builds IPv4-first connections for gallery downloads.""" + + def https_open(self, req): + return self.do_open(_IPv4FirstHTTPSConnection, req) + + +def _build_gallery_opener(): + """Opener with IPv4-first downloads and redirect-host allowlist.""" + return build_opener(_IPv4FirstHTTPSHandler, _AllowlistRedirectHandler) + + def _safe_download(url: str, max_bytes: int, timeout: int = 30) -> bytes: """Download from an allowlisted host, rejecting cross-host redirects.""" - opener = build_opener(_AllowlistRedirectHandler) + opener = _build_gallery_opener() resp = opener.open(url, timeout=timeout) try: return resp.read(max_bytes + 1) @@ -1813,7 +1890,8 @@ def get_extension_registry() -> Dict[str, Any]: if cached is not None and (now - cached_at) < _REGISTRY_TTL_SECONDS: return {"entries": cached} try: - raw = urlopen(_REGISTRY_URL, timeout=10).read(2 * 1024 * 1024) + opener = _build_gallery_opener() + raw = opener.open(_REGISTRY_URL, timeout=10).read(2 * 1024 * 1024) data = json.loads(raw.decode("utf-8")) if isinstance(data, list): entries = data diff --git a/tests/test_issue4746_extension_gallery.py b/tests/test_issue4746_extension_gallery.py index 2fe5cf78..283887fd 100644 --- a/tests/test_issue4746_extension_gallery.py +++ b/tests/test_issue4746_extension_gallery.py @@ -3,9 +3,10 @@ import hashlib import io import json +import socket import zipfile from pathlib import Path -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest @@ -419,7 +420,7 @@ def test_gallery_registry_list_format(monkeypatch, tmp_path): mock_resp.read.return_value = json.dumps(registry_data).encode("utf-8") ext_mod._REGISTRY_CACHE.clear() - monkeypatch.setattr(ext_mod, "urlopen", lambda *a, **kw: mock_resp) + monkeypatch.setattr(ext_mod, "_build_gallery_opener", lambda: MagicMock(open=lambda *a, **kw: mock_resp)) result = ext_mod.get_extension_registry() assert "entries" in result @@ -444,7 +445,7 @@ def test_gallery_registry_extensions_format(monkeypatch, tmp_path): mock_resp.read.return_value = json.dumps(registry_data).encode("utf-8") ext_mod._REGISTRY_CACHE.clear() - monkeypatch.setattr(ext_mod, "urlopen", lambda *a, **kw: mock_resp) + monkeypatch.setattr(ext_mod, "_build_gallery_opener", lambda: MagicMock(open=lambda *a, **kw: mock_resp)) result = ext_mod.get_extension_registry() assert len(result["entries"]) == 1 @@ -522,3 +523,94 @@ def test_install_rejects_redirect_to_disallowed_host(monkeypatch, tmp_path): "https://hermes-webui.github.io/exts/redir-ext.zip", "a" * 64, ) + + +def test_connect_ipv4_first_tries_inet_before_inet6(monkeypatch): + """_connect_ipv4_first calls getaddrinfo with AF_INET before AF_INET6.""" + import api.extensions as ext_mod + + call_order = [] + + real_getaddrinfo = socket.getaddrinfo + + def tracking_getaddrinfo(host, port, family, *args, **kwargs): + call_order.append(family) + return real_getaddrinfo(host, port, family, *args, **kwargs) + + monkeypatch.setattr(socket, "getaddrinfo", tracking_getaddrinfo) + monkeypatch.setattr(socket, "getdefaulttimeout", lambda: None) + + # We don't actually need to connect; just verify family ordering. + try: + ext_mod._connect_ipv4_first(("localhost", 80), timeout=None) + except OSError: + pass # connection failure is fine — we only care about call order + + assert socket.AF_INET in call_order + if socket.AF_INET6 in call_order: + assert call_order.index(socket.AF_INET) < call_order.index(socket.AF_INET6) + + +def test_connect_ipv4_first_handles_global_default_timeout(): + """The _GLOBAL_DEFAULT_TIMEOUT sentinel is resolved correctly.""" + import api.extensions as ext_mod + + # Should not raise TypeError when passed the stdlib sentinel. + sentinel = socket._GLOBAL_DEFAULT_TIMEOUT + # Force default timeout to None so no actual timeout is set on the socket. + with patch.object(socket, "getdefaulttimeout", return_value=None): + try: + ext_mod._connect_ipv4_first(("127.0.0.1", 1), timeout=sentinel) + except OSError: + pass # connection refused is fine + except TypeError: + pytest.fail("TypeError — sentinel not resolved") + + +def test_gallery_opener_includes_ipv4_first_handler(): + """_build_gallery_opener produces an opener wired to the IPv4-first HTTPS handler.""" + import api.extensions as ext_mod + + opener = ext_mod._build_gallery_opener() + # build_opener instantiates handler classes internally; find the one whose + # https_open uses our IPv4-first connection class. + handler_classes = [type(h) for h in opener.handlers] + assert ext_mod._IPv4FirstHTTPSHandler in handler_classes + + +def test_safe_download_uses_gallery_opener(monkeypatch): + """_safe_download routes through _build_gallery_opener (which is IPv4-first).""" + import api.extensions as ext_mod + + fake_response = MagicMock() + fake_response.read.return_value = b"file-content" + fake_response.close = MagicMock() + fake_opener = MagicMock(open=lambda url, timeout=None: fake_response) + + called = [] + monkeypatch.setattr(ext_mod, "_build_gallery_opener", lambda: (called.append(True), fake_opener)[1]) + + result = ext_mod._safe_download("https://example.com/x.zip", 1024) + assert result == b"file-content" + assert called == [True] + + +def test_registry_fetch_uses_gallery_opener(monkeypatch): + """get_extension_registry routes through _build_gallery_opener (IPv4-first).""" + import api.extensions as ext_mod + + mock_resp = MagicMock() + mock_resp.read.return_value = json.dumps( + {"extensions": [{"id": "test-ext", "name": "Test"}]} + ).encode("utf-8") + + ext_mod._REGISTRY_CACHE.clear() + + called = [] + fake_opener = MagicMock(open=lambda *a, **kw: mock_resp) + monkeypatch.setattr(ext_mod, "_build_gallery_opener", lambda: (called.append(True), fake_opener)[1]) + + result = ext_mod.get_extension_registry() + assert len(result["entries"]) == 1 + assert result["entries"][0]["id"] == "test-ext" + assert called == [True]