fix(catalog): a <...> URL keeps its pipe and a destination its spaces; pin the bracket counter

dokimi/changelog.d/20261114_100000_delimited-url-pipe-space.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- `catalog check --online` reads a `<...>` link destination up to its `>`: `[a](<https://acme.dev/x y>)` is requested as `https://acme.dev/x%20y` (it was cut at the space, so `/x` was probed), and a `|` inside an autolink or a `<...>` destination stays in the URL (`<https://acme.dev/a|b>` was cut at the pipe). In a table row (a line that starts with `|`) an unescaped pipe still ends the URL. A backslash-escaped `>` or `<` inside the `<...>` does not end it: `[a](<https://acme.dev/x y\>z>)` is requested as `https://acme.dev/x%20y%3Ez`. (#459)

dokimi/dokimi/external/online_checks.py

@@ -99,6 +99,8 @@
 _URL_RE = re.compile(r"https?://(?:[^\s\"`<>'“”‘’—，。、；：！？（）「」]|'(?=\w))+", re.IGNORECASE)  # an apostrophe in a word is in a URL ("Moore's_law")
 # Where a literal URL starts: after a markdown destination's ``](``, or a reference definition's ``[id]: ``.
 _LITERAL_START_RE = re.compile(r"\]\(|^[ \t]*\[(?!\^)[^\]\n]*\]:[ \t]*", re.MULTILINE)  # not a footnote "[^1]:"
+# a "<...>" link destination: the text up to its unescaped ">", blanks and escaped "<" ">" included
+_ANGLE_REST_RE = re.compile(r"((?:\\[!-/:-@\[-`{-~]|[^<>\n\\]|\\(?![!-/:-@\[-`{-~]))*)>")
 _ESCAPE_RE = re.compile(r"\\([!-/:-@\[-`{-~])")  # a backslash escape of ASCII punctuation, as CommonMark reads it
 # An escape or a character reference of a destination, in one pass so that "\&amp;" is the text "&amp;".
 _DECODE_RE = re.compile(r"\\([!-/:-@\[-`{-~])|&(#[xX][0-9a-fA-F]{1,6}|#[0-9]{1,7}|[A-Za-z][A-Za-z0-9]{1,31});")
@@ -388,7 +390,8 @@ def _decode(match: re.Match[str]) -> str:
     return html.unescape(text) if text[1] == "#" or text[1:] in html5 else text
 
 
-def _clean_url(raw: str, literal: bool = False, decode: bool = False, delimited: bool = False) -> Optional[str]:
+def _clean_url(raw: str, literal: bool = False, decode: bool = False, delimited: bool = False,
+               table_row: bool = False) -> Optional[str]:
     """``raw`` without what a sentence, a table cell, emphasis or a markdown link puts after it: the URL ends before
     the first ``)`` or ``]`` that closes no ``(`` or ``[`` of the URL, and before a ``|``; then trailing punctuation
     and ``*`` emphasis go. A balanced bracket stays ("f[0]=1", "[2001:db8::1]"). None for an empty result.
@@ -398,7 +401,8 @@ def _clean_url(raw: str, literal: bool = False, decode: bool = False, delimited:
     decodes its backslash escapes (``guide\\_notes`` is ``guide_notes``) and its character references (``a&amp;b``
     is ``a&b``), and an escaped character never ends it
     (``a\\)b`` is ``a)b``). ``delimited`` is a destination or autolink in ``<...>``: it ends at ``>``, so an
-    unmatched ``)`` or ``]`` is part of it.
+    unmatched ``)`` or ``]`` and a ``|`` are part of it, unless ``table_row``: a GFM table splits its row at an
+    unescaped ``|`` before it reads the inline text, so the pipe ends the URL there.
     """
     parens = brackets = 0
     end = len(raw)
@@ -407,7 +411,8 @@ def _clean_url(raw: str, literal: bool = False, decode: bool = False, delimited:
             parens += 1
         elif ch == "[":
             brackets += 1
-        elif ch == "|" or (not delimited and ((ch == ")" and parens == 0) or (ch == "]" and brackets == 0))):
+        elif (ch == "|" and (table_row or not delimited)) or (
+                not delimited and ((ch == ")" and parens == 0) or (ch == "]" and brackets == 0))):
             end = i
             break
         elif ch == ")":
@@ -486,13 +491,20 @@ def _distinct_urls(body: str) -> tuple[list[str], list[str]]:
     body = body.replace("](", "  ")  # a markdown link's text and its target are both URLs; the length is kept
     code = [m.span() for m in _URL_CODE_SPAN_RE.finditer(body)]  # a URL in a code span is an example, as in a fence
     for m in _URL_RE.finditer(body):
-        if _TEMPLATE_RE.search(m.group()) or body[m.end():m.end() + 1] in ("<", "{", "$"):
-            continue
         if any(start <= m.start() < end for start, end in code):
             continue
         destination = _starts_destination(body, m.start(), literal)
         autolink = m.start() > 0 and body[m.start() - 1] == "<" and body[m.end():m.end() + 1] == ">"
-        url = _clean_url(m.group(), literal=destination or autolink, decode=destination, delimited=autolink)
+        raw, angled = m.group(), False
+        if destination and body[m.start() - 1] == "<":  # a "<...>" destination may hold spaces; it ends at ">"
+            close = _ANGLE_REST_RE.match(body, m.start())
+            if close:
+                raw, angled = close.group(1), True
+        if _TEMPLATE_RE.search(raw) or (not angled and body[m.end():m.end() + 1] in ("<", "{", "$")):
+            continue
+        line_start = body.rfind("\n", 0, m.start()) + 1
+        url = _clean_url(raw, literal=destination or autolink, decode=destination, delimited=autolink or angled,
+                         table_row=body[line_start:m.start()].lstrip().startswith("|"))
         if url is not None:
             url = url.partition("#")[0]  # the fragment is never sent: anchors of one page are one request
         if url is not None and not _placeholder_url(url):

dokimi/tests/test_catalog_online.py

@@ -348,6 +348,41 @@ def test_an_escaped_paren_bracket_or_pipe_stays_in_a_destination():
     assert online_checks.extract_urls("[x](https://x.dev/p\\\\)") == ["https://x.dev/p\\"]
 
 
+def test_a_pipe_in_an_angle_delimited_url_is_part_of_it_outside_a_table_row():
+    assert online_checks.extract_urls("<https://acme.dev/a|b>") == ["https://acme.dev/a|b"]
+    assert online_checks.extract_urls("[a](<https://acme.dev/a|b>)") == ["https://acme.dev/a|b"]
+    assert online_checks.extract_urls("see https://acme.dev/a|b now") == ["https://acme.dev/a"]
+    # A GFM table splits its row at an unescaped "|" before it reads links: the pipe ends the URL there.
+    assert online_checks.extract_urls("| <https://acme.dev/a|b> | x |") == ["https://acme.dev/a"]
+    assert online_checks.extract_urls("  | [a](<https://acme.dev/a|b>) | x |") == ["https://acme.dev/a"]
+
+
+def test_an_angle_destination_keeps_its_spaces_and_is_probed_with_the_path_encoded(tmp_path):
+    assert online_checks.extract_urls("[a](<https://acme.dev/x y>)") == ["https://acme.dev/x y"]
+    assert online_checks.extract_urls("[a](<https://acme.dev/x y)z>)") == ["https://acme.dev/x y)z"]
+    assert online_checks.extract_urls("[a](<https://acme.dev/x >)") == ["https://acme.dev/x "]  # a trailing blank is part of it
+    assert online_checks.extract_urls("[a](<https://acme.dev/x y)") == ["https://acme.dev/x"]  # no ">" closes it: read as a bare URL
+    assert online_checks.extract_urls("<https://acme.dev/x y>") == ["https://acme.dev/x"]  # an autolink holds no space
+    assert [online_checks.extract_urls(f"[a](<https://acme.dev/x{b}\\{c}z>)") for b in ("", " y") for c in "<>"] == [["https://acme.dev/x<z"], ["https://acme.dev/x>z"], ["https://acme.dev/x y<z"], ["https://acme.dev/x y>z"]]  # an escaped bracket neither ends nor stops it, blank or not
+    assert online_checks.extract_urls("[a](<https://acme.dev/x<z>)") == []  # an unescaped "<" is a template
+    web = Web({"https://acme.dev/x%20y": 404})
+    root = _one_skill(tmp_path, "See [a](<https://acme.dev/x y>).\n")
+
+    [review] = _reviews(root, web)
+    assert _codes(review) == ["link-dead"]
+    assert {seen for _, seen in web.requests} == {"https://acme.dev/x%20y"}
+    trailing = Web({"https://acme.dev/x%20": 404})
+    [review] = _reviews(_one_skill(tmp_path / "t", "See [a](<https://acme.dev/x >).\n"), trailing)
+    assert _codes(review) == ["link-dead"]
+    assert {seen for _, seen in trailing.requests} == {"https://acme.dev/x%20"}
+    [review] = _reviews(_one_skill(tmp_path / "e", "See [a](<https://acme.dev/x y\\>z>).\n"), escaped := Web({"https://acme.dev/x%20y%3Ez": 404}))
+    assert _codes(review) == ["link-dead"] and {seen for _, seen in escaped.requests} == {"https://acme.dev/x%20y%3Ez"}
+
+
+def test_an_unbalanced_closing_bracket_ends_the_url_after_a_balanced_pair():
+    assert online_checks.extract_urls("[see https://acme.dev/a[b]c] for more") == ["https://acme.dev/a[b]c"]
+
+
 def test_a_backslash_in_an_autolink_is_not_decoded_and_a_hash_after_a_command_word_is_prose():
     # Self-review: CommonMark has no escapes inside an autolink; and "npm (see #setup)" is prose, not an install.
     assert online_checks.extract_urls("<https://x.dev/a\\_b>") == ["https://x.dev/a\\_b"]