From cd2638d186064963e879aec4a45e2bd7b507fe59 Mon Sep 17 00:00:00 2001 From: L4XB Date: Wed, 16 Sep 2026 16:41:39 +0200 Subject: [PATCH] fix(html): keep a URL from being cut at a space or a stray parenthesis A bare Markdown link destination ends at the first whitespace and at an unbalanced closing parenthesis. Both are legal in a query string or a fragment, which this converter deliberately does not re-encode, so a URL carrying either was written out in a form that reads back truncated: -> [spaced](https://example.com/s?q=a b) -> [result](https://example.com/s?q=a)b) Wrap such a destination in angle brackets, which is what CommonMark provides for the case and which leaves the URL itself byte for byte as it was. An image destination is parsed exactly like a link destination, and convert_img was not escaping its src at all -- not even the path quoting convert_a has applied for a while. It now shares both. A data URI is left alone, since its payload is not a path to quote. --- .../src/markitdown/converters/_markdownify.py | 59 +++++++++++++++---- .../markitdown/tests/test_html_converter.py | 43 +++++++++++++- 2 files changed, 89 insertions(+), 13 deletions(-) diff --git a/packages/markitdown/src/markitdown/converters/_markdownify.py b/packages/markitdown/src/markitdown/converters/_markdownify.py index ed3414486..a7341926d 100644 --- a/packages/markitdown/src/markitdown/converters/_markdownify.py +++ b/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, diff --git a/packages/markitdown/tests/test_html_converter.py b/packages/markitdown/tests/test_html_converter.py index 3c5d8976a..0f481a205 100644 --- a/packages/markitdown/tests/test_html_converter.py +++ b/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'example') - 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'result') + + 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'ok') + + 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('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('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('d') + + assert "![d](data:image/png;base64...)" in markdown def test_img_prefers_data_src_over_placeholder_data_uri() -> None: