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: