URI-escape image filenames (#10268)

Without this change, local images with `#` in their name result in incorrect URLs

There is already a similar call to `urllib.parse.quote` for file downloads, suggesting this is a sensible approach.

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Takeshi KOMIYA <i.tkomiya@gmail.com>

CHANGES

@@ -20,6 +20,7 @@ Features added
   ``:option:`--module[=foobar]``` or ``:option:`--module foobar```.
   Patch by Martin Liska.
 * #10881: autosectionlabel: Record the generated section label to the debug log.
+* #10268: Correctly URI-escape image filenames.
 
 Bugs fixed
 ----------

sphinx/builders/_epub_base.py

@@ -5,6 +5,7 @@
 import re
 from os import path
 from typing import Any, Dict, List, NamedTuple, Optional, Set, Tuple
+from urllib.parse import quote
 from zipfile import ZIP_DEFLATED, ZIP_STORED, ZipFile
 
 from docutils import nodes
@@ -524,7 +525,7 @@ def build_content(self) -> None:
                                        type='epub', subtype='unknown_project_files')
                     continue
                 filename = filename.replace(os.sep, '/')
-                item = ManifestItem(html.escape(filename),
+                item = ManifestItem(html.escape(quote(filename)),
                                     html.escape(self.make_id(filename)),
                                     html.escape(self.media_types[ext]))
                 metadata['manifest_items'].append(item)

sphinx/writers/html.py

@@ -620,7 +620,7 @@ def visit_image(self, node: Element) -> None:
         # rewrite the URI if the environment knows about it
         if olduri in self.builder.images:
             node['uri'] = posixpath.join(self.builder.imgpath,
-                                         self.builder.images[olduri])
+                                         urllib.parse.quote(self.builder.images[olduri]))
 
         if 'scale' in node:
             # Try to figure out image height and width.  Docutils does that too,

sphinx/writers/html5.py

@@ -567,7 +567,7 @@ def visit_image(self, node: Element) -> None:
         # rewrite the URI if the environment knows about it
         if olduri in self.builder.images:
             node['uri'] = posixpath.join(self.builder.imgpath,
-                                         self.builder.images[olduri])
+                                         urllib.parse.quote(self.builder.images[olduri]))
 
         if 'scale' in node:
             # Try to figure out image height and width.  Docutils does that too,

sphinx/writers/latex.py

@@ -1319,14 +1319,17 @@ def visit_image(self, node: Element) -> None:
         if include_graphics_options:
             options = '[%s]' % ','.join(include_graphics_options)
         base, ext = path.splitext(uri)
+
         if self.in_title and base:
             # Lowercase tokens forcely because some fncychap themes capitalize
             # the options of \sphinxincludegraphics unexpectedly (ex. WIDTH=...).
-            self.body.append(r'\lowercase{\sphinxincludegraphics%s}{{%s}%s}' %
-                             (options, base, ext))
+            cmd = r'\lowercase{\sphinxincludegraphics%s}{{%s}%s}' % (options, base, ext)
         else:
-            self.body.append(r'\sphinxincludegraphics%s{{%s}%s}' %
-                             (options, base, ext))
+            cmd = r'\sphinxincludegraphics%s{{%s}%s}' % (options, base, ext)
+        # escape filepath for includegraphics, https://tex.stackexchange.com/a/202714/41112
+        if '#' in base:
+            cmd = r'{\catcode`\#=12' + cmd + '}'
+        self.body.append(cmd)
         self.body.extend(post)
 
     def depart_image(self, node: Element) -> None:

tests/roots/test-image-escape/index.rst

@@ -0,0 +1,5 @@
+Sphinx image handling
+=====================
+
+.. an image with a character that is valid in a local file path but not a URL
+.. image:: img_#1.png

tests/test_build_html.py

@@ -1397,6 +1397,15 @@ def test_html_remote_images(app, status, warning):
     assert not (app.outdir / 'python-logo.png').exists()
 
 
+@pytest.mark.sphinx('html', testroot='image-escape')
+def test_html_encoded_image(app, status, warning):
+    app.builder.build_all()
+
+    result = (app.outdir / 'index.html').read_text()
+    assert ('<img alt="_images/img_%231.png" src="_images/img_%231.png" />' in result)
+    assert (app.outdir / '_images/img_#1.png').exists()
+
+
 @pytest.mark.sphinx('html', testroot='remote-logo')
 def test_html_remote_logo(app, status, warning):
     app.builder.build_all()

tests/test_build_latex.py

@@ -59,8 +59,8 @@ def compile_latex_document(app, filename='python.tex'):
     except OSError as exc:  # most likely the latex executable was not found
         raise pytest.skip.Exception from exc
     except CalledProcessError as exc:
-        print(exc.stdout)
-        print(exc.stderr)
+        print(exc.stdout.decode('utf8'))
+        print(exc.stderr.decode('utf8'))
         raise AssertionError('%s exited with return code %s' % (app.config.latex_engine,
                                                                 exc.returncode))