Merge pull request #91 from patrickfournier/url-encode

Url encode srcset values

Author: patrickfournier

pelican/plugins/image_process/image_process.py

@@ -17,6 +17,7 @@
 import shutil
 import subprocess
 import sys
+import urllib
 from urllib.parse import unquote, urlparse
 from urllib.request import pathname2url, url2pathname
 
@@ -362,7 +363,7 @@ def compute_paths(img, settings, derivative):
     derivative_path = os.path.join(process_dir, derivative)
     # urljoin truncates leading ../ elements
     base_url = posixpath.join(
-        posixpath.dirname(img["src"]), pathname2url(derivative_path)
+        posixpath.dirname(img["src"]), pathname2url(str(derivative_path))
     )
 
     PELICAN_V4 = 4
@@ -397,7 +398,7 @@ def compute_paths(img, settings, derivative):
             src_path = img_src_path.lstrip("/")
         source = os.path.join(settings["PATH"], src_path)
         base_path = os.path.join(
-            settings["OUTPUT_PATH"], os.path.dirname(src_path), derivative_path
+            settings["OUTPUT_PATH"], os.path.dirname(src_path), str(derivative_path)
         )
 
     return Path(base_url, source, base_path, filename)
@@ -408,14 +409,22 @@ def process_img_tag(img, settings, derivative):
     process = settings["IMAGE_PROCESS"][derivative]
 
     img["src"] = posixpath.join(path.base_url, path.filename)
-    destination = os.path.join(path.base_path, path.filename)
+    destination = os.path.join(str(path.base_path), path.filename)
 
     if not isinstance(process, list):
         process = process["ops"]
 
     process_image((path.source, destination, process), settings)
 
 
+def format_srcset_element(path, condition):
+    # space and comma have special meaning in srcset
+    if " " in path or "," in path:
+        path = urllib.parse.quote(path)
+
+    return f"{path} {condition}"
+
+
 def build_srcset(img, settings, derivative):
     path = compute_paths(img, settings, derivative)
     process = settings["IMAGE_PROCESS"][derivative]
@@ -434,7 +443,7 @@ def build_srcset(img, settings, derivative):
         default_name = default
     elif isinstance(default, list):
         default_name = "default"
-        destination = os.path.join(path.base_path, default_name, path.filename)
+        destination = os.path.join(str(path.base_path), default_name, path.filename)
         process_image((path.source, destination, default), settings)
 
     img["src"] = posixpath.join(path.base_url, default_name, path.filename)
@@ -445,8 +454,8 @@ def build_srcset(img, settings, derivative):
     srcset = []
     for src in process["srcset"]:
         file_path = posixpath.join(path.base_url, src[0], path.filename)
-        srcset.append(f"{file_path} {src[0]}")
-        destination = os.path.join(path.base_path, src[0], path.filename)
+        srcset.append(format_srcset_element(file_path, src[0]))
+        destination = os.path.join(str(path.base_path), src[0], path.filename)
         process_image((path.source, destination, src[1]), settings)
 
     if len(srcset) > 0:
@@ -532,12 +541,8 @@ def convert_div_to_picture_tag(soup, img, group, settings, derivative):
 
         srcset = []
         for src in s["srcset"]:
-            srcset.append(
-                "{} {}".format(
-                    os.path.join(s["base_url"], s["name"], src[0], s["filename"]),
-                    src[0],
-                )
-            )
+            url = os.path.join(s["base_url"], s["name"], src[0], s["filename"])
+            srcset.append(format_srcset_element(str(url), src[0]))
 
             source = os.path.join(settings["PATH"], s["url"][1:])
             destination = os.path.join(s["base_path"], s["name"], src[0], s["filename"])
@@ -644,12 +649,8 @@ def process_picture(soup, img, group, settings, derivative):
     for s in sources:
         srcset = []
         for src in s["srcset"]:
-            srcset.append(
-                "{} {}".format(
-                    posixpath.join(s["base_url"], s["name"], src[0], s["filename"]),
-                    src[0],
-                )
-            )
+            url = posixpath.join(s["base_url"], s["name"], src[0], s["filename"])
+            srcset.append(format_srcset_element(str(url), src[0]))
 
             source = os.path.join(settings["PATH"], s["url"][1:])
             destination = os.path.join(s["base_path"], s["name"], src[0], s["filename"])

pelican/plugins/image_process/test_image_process.py

@@ -499,7 +499,8 @@ def test_path_normalization(mocker, orig_src, orig_img, new_src, new_img):
         ),
     ],
 )
-def test_picture_generation(mocker, orig_tag, new_tag, call_args):
+def test_html_and_pictures_generation(mocker, orig_tag, new_tag, call_args):
+    """Tests that the generated html is as expected and the images are processed."""
     process = mocker.patch("pelican.plugins.image_process.image_process.process_image")
 
     settings = get_settings(
@@ -508,6 +509,8 @@ def test_picture_generation(mocker, orig_tag, new_tag, call_args):
 
     assert harvest_images_in_fragment(orig_tag, settings) == new_tag
 
+    # Check that process_image was called with the expected arguments
+    # and in the expecter order.
     for i, (orig_img, new_img, transform_params) in enumerate(call_args):
         assert process.mock_calls[i] == mocker.call(
             (
@@ -519,6 +522,140 @@ def test_picture_generation(mocker, orig_tag, new_tag, call_args):
         )
 
 
+@pytest.mark.parametrize(
+    "orig_tag, new_tag",
+    [
+        # <img/> src attribute with no quotes, spaces or commas.
+        (
+            '<img class="image-process-thumb" src="/tmp/my&amp;_dir/my!_test.jpg" />',
+            '<img class="image-process-thumb" '
+            'src="/tmp/my&amp;_dir/derivs/thumb/my!_test.jpg"/>',
+        ),
+        # <img/> src attribute with double quotes, spaces and commas.
+        (
+            '<img class="image-process-thumb" '
+            'src="/tmp/my,&quot; dir/my &#34;test,.jpg" />',
+            '<img class="image-process-thumb" '
+            "src='/tmp/my,\" dir/derivs/thumb/my \"test,.jpg'/>",
+        ),
+        # <img/> src attribute with single and double quotes, spaces and commas.
+        (
+            '<img class="image-process-thumb" '
+            'src="/tmp/m\'y,&quot; dir/my &#34;test,.jpg" />',
+            '<img class="image-process-thumb" '
+            'src="/tmp/m\'y,&quot; dir/derivs/thumb/my &quot;test,.jpg"/>',
+        ),
+        # <img/> srcset attribute with no quotes, spaces or commas.
+        (
+            '<img class="image-process-crisp" src="/tmp/my&amp;_dir/my!_test.jpg" />',
+            '<img class="image-process-crisp" '
+            'src="/tmp/my&amp;_dir/derivs/crisp/1x/my!_test.jpg" '
+            'srcset="/tmp/my&amp;_dir/derivs/crisp/1x/my!_test.jpg 1x, '
+            "/tmp/my&amp;_dir/derivs/crisp/2x/my!_test.jpg 2x, "
+            '/tmp/my&amp;_dir/derivs/crisp/4x/my!_test.jpg 4x"/>',
+        ),
+        # <img/> srcset attribute with double quotes, spaces and commas.
+        (
+            '<img class="image-process-crisp" '
+            'src="/tmp/my,&quot; dir/my &#34;test,.jpg" />',
+            '<img class="image-process-crisp" '
+            "src='/tmp/my,\" dir/derivs/crisp/1x/my \"test,.jpg' "
+            'srcset="/tmp/my%2C%22%20dir/derivs/crisp/1x/my%20%22test%2C.jpg 1x, '
+            "/tmp/my%2C%22%20dir/derivs/crisp/2x/my%20%22test%2C.jpg 2x, "
+            '/tmp/my%2C%22%20dir/derivs/crisp/4x/my%20%22test%2C.jpg 4x"/>',
+        ),
+        # <img/> srcset attribute with single and double quotes, spaces and commas.
+        (
+            '<img class="image-process-crisp" '
+            'src="/tmp/m\'y,&quot; dir/my &#34;test,.jpg" />',
+            '<img class="image-process-crisp" '
+            'src="/tmp/m\'y,&quot; dir/derivs/crisp/1x/my &quot;test,.jpg" '
+            'srcset="/tmp/m%27y%2C%22%20dir/derivs/crisp/1x/my%20%22test%2C.jpg 1x, '
+            "/tmp/m%27y%2C%22%20dir/derivs/crisp/2x/my%20%22test%2C.jpg 2x, "
+            '/tmp/m%27y%2C%22%20dir/derivs/crisp/4x/my%20%22test%2C.jpg 4x"/>',
+        ),
+        # <picture/> src and srcset attributes with no quotes, spaces or commas.
+        (
+            '<picture><source class="source-1" '
+            'src="/my&amp;_dir/my!_pelican-closeup.jpg"/><img '
+            'class="image-process-pict" src="/my&amp;_dir/my!_pelican.jpg"/>'
+            "</picture>",
+            '<picture><source media="(min-width: 640px)" sizes="100vw" '
+            'srcset="/my&amp;_dir/derivs/pict/default/640w/'
+            "my!_pelican.jpg 640w, "
+            "/my&amp;_dir/derivs/pict/default/1024w/my!_pelican.jpg 1024w, "
+            '/my&amp;_dir/derivs/pict/default/1600w/my!_pelican.jpg 1600w"/>'
+            '<source srcset="/my&amp;_dir/derivs/pict/source-1/1x/'
+            "my!_pelican-closeup.jpg 1x, "
+            "/my&amp;_dir/derivs/pict/source-1/2x/"
+            'my!_pelican-closeup.jpg 2x"/><img '
+            'class="image-process-pict" '
+            'src="/my&amp;_dir/derivs/pict/default/640w/my!_pelican.jpg"/>'
+            "</picture>",
+        ),
+        # <picture/> src and srcset attributes with double quotes, spaces and commas.
+        (
+            '<picture><source class="source-1" '
+            'src="/my,&quot; dir/my &#34;pelican-closeup,.jpg"/><img '
+            'class="image-process-pict" src="/my,&quot; dir/my &#34;pelican,.jpg"/>'
+            "</picture>",
+            '<picture><source media="(min-width: 640px)" sizes="100vw" '
+            'srcset="/my%2C%22%20dir/derivs/pict/default/640w/'
+            "my%20%22pelican%2C.jpg 640w, "
+            "/my%2C%22%20dir/derivs/pict/default/1024w/my%20%22pelican%2C.jpg 1024w, "
+            '/my%2C%22%20dir/derivs/pict/default/1600w/my%20%22pelican%2C.jpg 1600w"/>'
+            '<source srcset="/my%2C%22%20dir/derivs/pict/source-1/1x/'
+            "my%20%22pelican-closeup%2C.jpg 1x, "
+            "/my%2C%22%20dir/derivs/pict/source-1/2x/"
+            'my%20%22pelican-closeup%2C.jpg 2x"/><img '
+            'class="image-process-pict" '
+            "src='/my,\" dir/derivs/pict/default/640w/my \"pelican,.jpg'/>"
+            "</picture>",
+        ),
+        # <picture/> src and srcset attributes
+        # with single and double quotes, spaces and commas.
+        (
+            '<picture><source class="source-1" '
+            'src="/m\'y,&quot; dir/my &#34;pelican-closeup,.jpg"/><img '
+            'class="image-process-pict" src="/m\'y,&quot; dir/my &#34;pelican,.jpg"/>'
+            "</picture>",
+            '<picture><source media="(min-width: 640px)" sizes="100vw" '
+            'srcset="/m%27y%2C%22%20dir/derivs/pict/default/640w/'
+            "my%20%22pelican%2C.jpg 640w, "
+            "/m%27y%2C%22%20dir/derivs/pict/default/1024w/"
+            "my%20%22pelican%2C.jpg 1024w, "
+            "/m%27y%2C%22%20dir/derivs/pict/default/1600w/"
+            'my%20%22pelican%2C.jpg 1600w"/>'
+            '<source srcset="/m%27y%2C%22%20dir/derivs/pict/source-1/1x/'
+            "my%20%22pelican-closeup%2C.jpg 1x, "
+            "/m%27y%2C%22%20dir/derivs/pict/source-1/2x/"
+            'my%20%22pelican-closeup%2C.jpg 2x"/><img '
+            'class="image-process-pict" '
+            'src="/m\'y,&quot; dir/derivs/pict/default/640w/my &quot;pelican,.jpg"/>'
+            "</picture>",
+        ),
+    ],
+)
+def test_special_chars_in_image_path_are_handled_properly(mocker, orig_tag, new_tag):
+    """Tests that special characters in image paths are handled properly.
+
+    For the src attribute, single or double quotes may need to be escaped,
+    according to the quotation mark used to enclose the attribute value.
+
+    For the srcset attribute, in addition to quotes, spaces and commas
+    need to be url-encoded.
+
+    Related to issue #78 https://github.com/pelican-plugins/image-process/issues/78
+    """
+    mocker.patch("pelican.plugins.image_process.image_process.process_image")
+
+    settings = get_settings(
+        IMAGE_PROCESS=COMPLEX_TRANSFORMS, IMAGE_PROCESS_DIR="derivs"
+    )
+
+    assert harvest_images_in_fragment(orig_tag, settings) == new_tag
+
+
 def process_image_mock_exif_tool_started(image, settings):
     assert ExifTool._instance is not None