fix: prefer IPv4 for extension gallery downloads to prevent timeout on broken IPv6

Clean rebase of rumotoshino's #5458 (rebase-first).

Co-authored-by: rumotoshino <rumotoshino@users.noreply.github.com>

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

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]