From 03b9134ee00e98df4f8b5f6d22f345cdafe31870 Mon Sep 17 00:00:00 2001
From: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Date: Mon, 12 Aug 2024 03:49:58 +0100
Subject: [PATCH] Short-circuit HTTP requests in tests (#12772)

---
 sphinx/environment/__init__.py               |   2 +-
 sphinx/environment/adapters/asset.py         |   5 +-
 sphinx/transforms/post_transforms/images.py  | 105 ++++++++++---------
 tests/conftest.py                            |  18 ++++
 tests/test_builders/test_build.py            |   1 +
 tests/test_builders/test_build_epub.py       |   1 +
 tests/test_builders/test_build_html.py       |   1 +
 tests/test_builders/test_build_html_image.py |   2 +
 tests/test_builders/test_build_latex.py      |   1 +
 tests/test_builders/test_build_texinfo.py    |   1 +
 tests/test_extensions/test_ext_math.py       |   1 +
 tests/test_intl/test_intl.py                 |   2 +
 12 files changed, 90 insertions(+), 50 deletions(-)

diff --git a/sphinx/environment/__init__.py b/sphinx/environment/__init__.py
index 637fe185f7..67087e6182 100644
--- a/sphinx/environment/__init__.py
+++ b/sphinx/environment/__init__.py
@@ -248,7 +248,7 @@ class BuildEnvironment:
         self.dlfiles: DownloadFiles = DownloadFiles()
 
         # the original URI for images
-        self.original_image_uri: dict[str, str] = {}
+        self.original_image_uri: dict[_StrPath, str] = {}
 
         # temporary data storage while reading a document
         self.temp_data: dict[str, Any] = {}
diff --git a/sphinx/environment/adapters/asset.py b/sphinx/environment/adapters/asset.py
index 57fdc91f14..dc0cf76695 100644
--- a/sphinx/environment/adapters/asset.py
+++ b/sphinx/environment/adapters/asset.py
@@ -1,6 +1,7 @@
 """Assets adapter for sphinx.environment."""
 
 from sphinx.environment import BuildEnvironment
+from sphinx.util._pathlib import _StrPath
 
 
 class ImageAdapter:
@@ -9,7 +10,7 @@ class ImageAdapter:
 
     def get_original_image_uri(self, name: str) -> str:
         """Get the original image URI."""
-        while name in self.env.original_image_uri:
-            name = self.env.original_image_uri[name]
+        while _StrPath(name) in self.env.original_image_uri:
+            name = self.env.original_image_uri[_StrPath(name)]
 
         return name
diff --git a/sphinx/transforms/post_transforms/images.py b/sphinx/transforms/post_transforms/images.py
index b679481e83..05b07dd223 100644
--- a/sphinx/transforms/post_transforms/images.py
+++ b/sphinx/transforms/post_transforms/images.py
@@ -6,6 +6,7 @@ import os
 import re
 from hashlib import sha1
 from math import ceil
+from pathlib import Path
 from typing import TYPE_CHECKING, Any
 
 from docutils import nodes
@@ -13,6 +14,7 @@ from docutils import nodes
 from sphinx.locale import __
 from sphinx.transforms import SphinxTransform
 from sphinx.util import logging, requests
+from sphinx.util._pathlib import _StrPath
 from sphinx.util.http_date import epoch_to_rfc1123, rfc1123_to_epoch
 from sphinx.util.images import get_image_extension, guess_mimetype, parse_data_uri
 from sphinx.util.osutil import ensuredir
@@ -65,50 +67,58 @@ class ImageDownloader(BaseImageConverter):
             basename = CRITICAL_PATH_CHAR_RE.sub("_", basename)
 
             uri_hash = sha1(node['uri'].encode(), usedforsecurity=False).hexdigest()
-            ensuredir(os.path.join(self.imagedir, uri_hash))
-            path = os.path.join(self.imagedir, uri_hash, basename)
+            path = Path(self.imagedir, uri_hash, basename)
+            path.parent.mkdir(parents=True, exist_ok=True)
+            self._download_image(node, path)
 
-            headers = {}
-            if os.path.exists(path):
-                timestamp: float = ceil(os.stat(path).st_mtime)
-                headers['If-Modified-Since'] = epoch_to_rfc1123(timestamp)
-
-            config = self.app.config
-            r = requests.get(
-                node['uri'], headers=headers,
-                _user_agent=config.user_agent,
-                _tls_info=(config.tls_verify, config.tls_cacerts),
-            )
-            if r.status_code >= 400:
-                logger.warning(__('Could not fetch remote image: %s [%d]'),
-                               node['uri'], r.status_code)
-            else:
-                self.app.env.original_image_uri[path] = node['uri']
-
-                if r.status_code == 200:
-                    with open(path, 'wb') as f:
-                        f.write(r.content)
-
-                last_modified = r.headers.get('last-modified')
-                if last_modified:
-                    timestamp = rfc1123_to_epoch(last_modified)
-                    os.utime(path, (timestamp, timestamp))
-
-                mimetype = guess_mimetype(path, default='*')
-                if mimetype != '*' and os.path.splitext(basename)[1] == '':
-                    # append a suffix if URI does not contain suffix
-                    ext = get_image_extension(mimetype)
-                    newpath = os.path.join(self.imagedir, uri_hash, basename + ext)
-                    os.replace(path, newpath)
-                    self.app.env.original_image_uri.pop(path)
-                    self.app.env.original_image_uri[newpath] = node['uri']
-                    path = newpath
-                node['candidates'].pop('?')
-                node['candidates'][mimetype] = path
-                node['uri'] = path
-                self.app.env.images.add_file(self.env.docname, path)
         except Exception as exc:
-            logger.warning(__('Could not fetch remote image: %s [%s]'), node['uri'], exc)
+            msg = __('Could not fetch remote image: %s [%s]')
+            logger.warning(msg, node['uri'], exc)
+
+    def _download_image(self, node: nodes.image, path: Path) -> None:
+        headers = {}
+        if path.exists():
+            timestamp: float = ceil(path.stat().st_mtime)
+            headers['If-Modified-Since'] = epoch_to_rfc1123(timestamp)
+
+        config = self.app.config
+        r = requests.get(
+            node['uri'], headers=headers,
+            _user_agent=config.user_agent,
+            _tls_info=(config.tls_verify, config.tls_cacerts),
+        )
+        if r.status_code >= 400:
+            msg = __('Could not fetch remote image: %s [%d]')
+            logger.warning(msg, node['uri'], r.status_code)
+        else:
+            self.app.env.original_image_uri[_StrPath(path)] = node['uri']
+
+            if r.status_code == 200:
+                path.write_bytes(r.content)
+            if last_modified := r.headers.get('Last-Modified'):
+                timestamp = rfc1123_to_epoch(last_modified)
+                os.utime(path, (timestamp, timestamp))
+
+            self._process_image(node, path)
+
+    def _process_image(self, node: nodes.image, path: Path) -> None:
+        str_path = _StrPath(path)
+        self.app.env.original_image_uri[str_path] = node['uri']
+
+        mimetype = guess_mimetype(path, default='*')
+        if mimetype != '*' and path.suffix == '':
+            # append a suffix if URI does not contain suffix
+            ext = get_image_extension(mimetype) or ''
+            with_ext = path.with_name(path.name + ext)
+            os.replace(path, with_ext)
+            self.app.env.original_image_uri.pop(str_path)
+            self.app.env.original_image_uri[_StrPath(with_ext)] = node['uri']
+            path = with_ext
+        path_str = str(path)
+        node['candidates'].pop('?')
+        node['candidates'][mimetype] = path_str
+        node['uri'] = path_str
+        self.app.env.images.add_file(self.env.docname, path_str)
 
 
 class DataURIExtractor(BaseImageConverter):
@@ -130,16 +140,17 @@ class DataURIExtractor(BaseImageConverter):
 
         ensuredir(os.path.join(self.imagedir, 'embeded'))
         digest = sha1(image.data, usedforsecurity=False).hexdigest()
-        path = os.path.join(self.imagedir, 'embeded', digest + ext)
+        path = _StrPath(self.imagedir, 'embeded', digest + ext)
         self.app.env.original_image_uri[path] = node['uri']
 
         with open(path, 'wb') as f:
             f.write(image.data)
 
+        path_str = str(path)
         node['candidates'].pop('?')
-        node['candidates'][image.mimetype] = path
-        node['uri'] = path
-        self.app.env.images.add_file(self.env.docname, path)
+        node['candidates'][image.mimetype] = path_str
+        node['uri'] = path_str
+        self.app.env.images.add_file(self.env.docname, path_str)
 
 
 def get_filename_for(filename: str, mimetype: str) -> str:
@@ -258,7 +269,7 @@ class ImageConverter(BaseImageConverter):
                 node['candidates'][_to] = destpath
             node['uri'] = destpath
 
-            self.env.original_image_uri[destpath] = srcpath
+            self.env.original_image_uri[_StrPath(destpath)] = srcpath
             self.env.images.add_file(self.env.docname, destpath)
 
     def convert(self, _from: str, _to: str) -> bool:
diff --git a/tests/conftest.py b/tests/conftest.py
index 6e3b83b139..a9ef8e7c92 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -4,6 +4,7 @@ import gettext
 import os
 import sys
 from pathlib import Path
+from types import SimpleNamespace
 from typing import TYPE_CHECKING
 
 import docutils
@@ -60,3 +61,20 @@ def _cleanup_docutils() -> Iterator[None]:
     sys.path[:] = saved_path
 
     _clean_up_global_state()
+
+
+@pytest.fixture
+def _http_teapot(monkeypatch: pytest.MonkeyPatch) -> Iterator[None]:
+    """Short-circuit HTTP requests.
+
+    Windows takes too long to fail on connections, hence this fixture.
+    """
+    # https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/418
+    response = SimpleNamespace(status_code=418)
+
+    def _request(*args, **kwargs):
+        return response
+
+    with monkeypatch.context() as m:
+        m.setattr('sphinx.util.requests._Session.request', _request)
+        yield
diff --git a/tests/test_builders/test_build.py b/tests/test_builders/test_build.py
index 5ccef02c94..6a5ed0b6d2 100644
--- a/tests/test_builders/test_build.py
+++ b/tests/test_builders/test_build.py
@@ -100,6 +100,7 @@ def test_numbered_circular_toctree(app):
     ) in warnings
 
 
+@pytest.mark.usefixtures('_http_teapot')
 @pytest.mark.sphinx('dummy', testroot='images')
 def test_image_glob(app):
     app.build(force_all=True)
diff --git a/tests/test_builders/test_build_epub.py b/tests/test_builders/test_build_epub.py
index 263fefe809..178b46ff0a 100644
--- a/tests/test_builders/test_build_epub.py
+++ b/tests/test_builders/test_build_epub.py
@@ -473,6 +473,7 @@ def test_xml_name_pattern_check():
     assert not _XML_NAME_PATTERN.match('1bfda21')
 
 
+@pytest.mark.usefixtures('_http_teapot')
 @pytest.mark.sphinx('epub', testroot='images')
 def test_copy_images(app):
     app.build()
diff --git a/tests/test_builders/test_build_html.py b/tests/test_builders/test_build_html.py
index 93e770cca9..ecaab1651a 100644
--- a/tests/test_builders/test_build_html.py
+++ b/tests/test_builders/test_build_html.py
@@ -266,6 +266,7 @@ def test_html_inventory(app):
     )
 
 
+@pytest.mark.usefixtures('_http_teapot')
 @pytest.mark.sphinx(
     'html',
     testroot='images',
diff --git a/tests/test_builders/test_build_html_image.py b/tests/test_builders/test_build_html_image.py
index 93777c7632..15b0be7aae 100644
--- a/tests/test_builders/test_build_html_image.py
+++ b/tests/test_builders/test_build_html_image.py
@@ -5,6 +5,7 @@ import docutils
 import pytest
 
 
+@pytest.mark.usefixtures('_http_teapot')
 @pytest.mark.sphinx('html', testroot='images')
 def test_html_remote_images(app):
     app.build(force_all=True)
@@ -77,6 +78,7 @@ def test_html_scaled_image_link(app):
     )
 
 
+@pytest.mark.usefixtures('_http_teapot')
 @pytest.mark.sphinx('html', testroot='images')
 def test_copy_images(app):
     app.build()
diff --git a/tests/test_builders/test_build_latex.py b/tests/test_builders/test_build_latex.py
index fb0f1bf4c5..0c1d382baf 100644
--- a/tests/test_builders/test_build_latex.py
+++ b/tests/test_builders/test_build_latex.py
@@ -2157,6 +2157,7 @@ def test_latex_code_role(app):
     ) in content
 
 
+@pytest.mark.usefixtures('_http_teapot')
 @pytest.mark.sphinx('latex', testroot='images')
 def test_copy_images(app):
     app.build()
diff --git a/tests/test_builders/test_build_texinfo.py b/tests/test_builders/test_build_texinfo.py
index 07934ad182..7f229db83d 100644
--- a/tests/test_builders/test_build_texinfo.py
+++ b/tests/test_builders/test_build_texinfo.py
@@ -131,6 +131,7 @@ def test_texinfo_samp_with_variable(app):
     assert '@code{Show @var{variable} in the middle}' in output
 
 
+@pytest.mark.usefixtures('_http_teapot')
 @pytest.mark.sphinx('texinfo', testroot='images')
 def test_copy_images(app):
     app.build()
diff --git a/tests/test_extensions/test_ext_math.py b/tests/test_extensions/test_ext_math.py
index a18a410c42..5a866520af 100644
--- a/tests/test_extensions/test_ext_math.py
+++ b/tests/test_extensions/test_ext_math.py
@@ -288,6 +288,7 @@ def test_mathjax_numsep_html(app):
     assert html in content
 
 
+@pytest.mark.usefixtures('_http_teapot')
 @pytest.mark.sphinx(
     'html',
     testroot='ext-math',
diff --git a/tests/test_intl/test_intl.py b/tests/test_intl/test_intl.py
index e664d47999..74e9cf57e6 100644
--- a/tests/test_intl/test_intl.py
+++ b/tests/test_intl/test_intl.py
@@ -1705,6 +1705,7 @@ SUBSTITUTED IMAGE [image: SUBST_EPILOG_2 TRANSLATED][image] HERE.
     )
 
 
+@pytest.mark.usefixtures('_http_teapot')
 @pytest.mark.sphinx(
     'dummy',
     testroot='images',
@@ -1782,6 +1783,7 @@ def test_image_glob_intl(app):
     )
 
 
+@pytest.mark.usefixtures('_http_teapot')
 @pytest.mark.sphinx(
     'dummy',
     testroot='images',