fix(html): keep a URL from being cut at a space or a stray parenthesis

packages/markitdown/src/markitdown/converters/_markdownify.py

@@ -7,6 +7,10 @@
 
 _PERCENT_ENCODED_OCTET = re.compile(r"%[0-9A-Fa-f]{2}")
 
+# Whitespace ends a bare Markdown destination, parentheses have to balance
+# inside one, and an angle bracket would close it early.
+_NEEDS_ANGLE_BRACKETS = re.compile(r"[\s()<>]")
+
 
 def _quote_path_preserving_percent_encoded_octets(path: str) -> str:
     """Quote a URL path while preserving existing %HH byte encodings."""
@@ -22,6 +26,36 @@ def _quote_path_preserving_percent_encoded_octets(path: str) -> str:
     return "".join(parts)
 
 
+def _escape_uri(url: str) -> str:
+    """Quote a URL's path, leaving the rest of it alone."""
+    try:
+        parsed_url = urlparse(url)
+    except ValueError:  # It's not clear if this ever gets thrown
+        return url
+    return urlunparse(
+        parsed_url._replace(
+            path=_quote_path_preserving_percent_encoded_octets(parsed_url.path)
+        )
+    )
+
+
+def _format_destination(url: str) -> str:
+    """Render a URL as a Markdown inline link destination.
+
+    A bare destination ends at the first whitespace and at an unbalanced
+    closing parenthesis, so a URL carrying either is truncated when the
+    Markdown is read back. Both are legal in a query string or a fragment,
+    which this converter does not percent-encode because doing so would
+    rewrite sub-delimiters the server may be reading. Wrapping the
+    destination in angle brackets is what CommonMark provides for the case,
+    and it leaves the URL itself untouched.
+    """
+    if not _NEEDS_ANGLE_BRACKETS.search(url):
+        return url
+    # A `<...>` destination may not contain an unescaped angle bracket.
+    return "<{}>".format(url.replace("<", "%3C").replace(">", "%3E"))
+
+
 class _CustomMarkdownify(markdownify.MarkdownConverter):
     """
     A custom version of markdownify's MarkdownConverter. Changes include:
@@ -77,13 +111,7 @@ def convert_a(
                 parsed_url = urlparse(href)  # type: ignore
                 if parsed_url.scheme and parsed_url.scheme.lower() not in ["http", "https", "file"]:  # type: ignore
                     return "%s%s%s" % (prefix, text, suffix)
-                href = urlunparse(
-                    parsed_url._replace(
-                        path=_quote_path_preserving_percent_encoded_octets(
-                            parsed_url.path
-                        )
-                    )
-                )  # type: ignore
+                href = _escape_uri(href)
             except ValueError:  # It's not clear if this ever gets thrown
                 return "%s%s%s" % (prefix, text, suffix)
 
@@ -100,7 +128,8 @@ def convert_a(
             title = href
         title_part = ' "%s"' % title.replace('"', r"\"") if title else ""
         return (
-            "%s[%s](%s%s)%s" % (prefix, text, href, title_part, suffix)
+            "%s[%s](%s%s)%s"
+            % (prefix, text, _format_destination(href), title_part, suffix)
             if href
             else text
         )
@@ -138,10 +167,16 @@ def convert_img(
             return alt
 
         # Remove dataURIs
-        if src[:5].lower() == "data:" and not self.options["keep_data_uris"]:
-            src = src.split(",")[0] + "..."
-
-        return "![%s](%s%s)" % (alt, src, title_part)
+        if src[:5].lower() == "data:":
+            if not self.options["keep_data_uris"]:
+                src = src.split(",")[0] + "..."
+        else:
+            # The same treatment convert_a gives an href: the destination of an
+            # image is parsed exactly like the destination of a link. A data URI
+            # is left alone, since its payload is not a path to quote.
+            src = _escape_uri(src)
+
+        return "![%s](%s%s)" % (alt, _format_destination(src), title_part)
 
     def convert_input(
         self,

packages/markitdown/tests/test_html_converter.py

@@ -92,7 +92,48 @@ def test_html_href_does_not_quote_query_or_fragment() -> None:
 
     markdown = _convert_html(f'<a href="{href}">example</a>')
 
-    assert f"[example]({expected_href})" in markdown
+    # The URL is still not re-encoded. The angle brackets are what CommonMark
+    # provides for a destination that holds a space, so the URL above survives
+    # a round trip through a Markdown parser instead of being cut at the space.
+    assert f"[example](<{expected_href}>)" in markdown
+
+
+def test_html_href_with_an_unbalanced_parenthesis_is_delimited() -> None:
+    """A bare destination ends at an unbalanced `)`, truncating the URL."""
+    href = "https://example.com/s?q=a)b"
+
+    markdown = _convert_html(f'<a href="{href}">result</a>')
+
+    assert f"[result](<{href}>)" in markdown
+
+
+def test_html_href_without_anything_to_delimit_stays_bare() -> None:
+    """Guard: an ordinary URL is not wrapped."""
+    href = "https://example.com/a/b?x=1&y=2"
+
+    markdown = _convert_html(f'<a href="{href}">ok</a>')
+
+    assert f"[ok]({href})" in markdown
+
+
+def test_img_src_is_quoted_like_an_href() -> None:
+    """An image destination is parsed exactly like a link destination."""
+    markdown = _convert_html('<img src="https://example.com/a b.png" alt="pic">')
+
+    assert "![pic](https://example.com/a%20b.png)" in markdown
+
+
+def test_img_src_with_an_unbalanced_parenthesis_is_quoted() -> None:
+    markdown = _convert_html('<img src="https://example.com/a)b.png" alt="pic">')
+
+    assert "![pic](https://example.com/a%29b.png)" in markdown
+
+
+def test_img_data_uri_is_left_alone() -> None:
+    """Guard: a data URI is a payload, not a path to quote."""
+    markdown = _convert_html('<img src="data:image/png;base64,iVBORw0KGgo" alt="d">')
+
+    assert "![d](data:image/png;base64...)" in markdown
 
 
 def test_img_prefers_data_src_over_placeholder_data_uri() -> None: