Fix #152: Fix pull request links in bot answers including trailing punctuation #153

Open
typosaurus wants to merge 1 commits from typosaurus/ticket-152 into master
3 changed files with 69 additions and 8 deletions

View File

@ -79,6 +79,7 @@ _EMAIL_RE = re.compile(r"\b[A-Za-z0-9._%+\-]+@[A-Za-z0-9.\-]+\.[A-Za-z]{2,}\b")
_EMAIL_KEEP_DOMAIN = "molodetz.nl" _EMAIL_KEEP_DOMAIN = "molodetz.nl"
_MEDIA_SKIP_TAGS = {"a", "code", "pre"} _MEDIA_SKIP_TAGS = {"a", "code", "pre"}
_TRAILING_PUNCT_RE = re.compile(r"[.,;:!?)\]}\"']+$")
_TITLE_INLINE_TAGS = { _TITLE_INLINE_TAGS = {
"b", "strong", "i", "em", "code", "del", "s", "mark", "sub", "sup", "span", "br", "b", "strong", "i", "em", "code", "del", "s", "mark", "sub", "sup", "span", "br",
} }
@ -139,6 +140,11 @@ def _alt_from_url(url: str) -> str:
def _embed_url(url: str) -> str: def _embed_url(url: str) -> str:
trail = ""
punct_match = _TRAILING_PUNCT_RE.search(url)
if punct_match:
trail = punct_match.group()
url = url[: punct_match.start()]
youtube = _YOUTUBE_RE.search(url) youtube = _YOUTUBE_RE.search(url)
if youtube: if youtube:
video_id = youtube.group(1) video_id = youtube.group(1)
@ -146,19 +152,19 @@ def _embed_url(url: str) -> str:
f'<div class="embed-youtube"><iframe ' f'<div class="embed-youtube"><iframe '
f'src="https://www.youtube.com/embed/{video_id}" ' f'src="https://www.youtube.com/embed/{video_id}" '
f'frameborder="0" allowfullscreen allow="{_YOUTUBE_ALLOW}">' f'frameborder="0" allowfullscreen allow="{_YOUTUBE_ALLOW}">'
f"</iframe></div>" f"</iframe></div>{trail}"
) )
escaped = html.escape(url, quote=True) escaped = html.escape(url, quote=True)
if _IMAGE_RE.search(url): if _IMAGE_RE.search(url):
alt = html.escape(_alt_from_url(url), quote=True) alt = html.escape(_alt_from_url(url), quote=True)
return f'<img src="{escaped}" alt="{alt}" loading="lazy" data-lightbox>' return f'<img src="{escaped}" alt="{alt}" loading="lazy" data-lightbox>{trail}'
if _VIDEO_RE.search(url): if _VIDEO_RE.search(url):
return f'<video src="{escaped}" controls preload="metadata"></video>' return f'<video src="{escaped}" controls preload="metadata"></video>{trail}'
if _AUDIO_RE.search(url): if _AUDIO_RE.search(url):
return f'<audio src="{escaped}" controls preload="metadata"></audio>' return f'<audio src="{escaped}" controls preload="metadata"></audio>{trail}'
return ( return (
f'<a href="{escaped}" target="_blank" rel="noopener noreferrer">' f'<a href="{escaped}" target="_blank" rel="noopener noreferrer">'
f"{html.escape(url)}</a>" f"{html.escape(url)}</a>{trail}"
) )

View File

@ -188,17 +188,21 @@ export class ContentRenderer {
a.textContent = "@" + part.username; a.textContent = "@" + part.username;
fragment.appendChild(a); fragment.appendChild(a);
} else { } else {
const el = this.urlToEmbed(part.value); const { cleaned, trail } = this.stripTrailingPunct(part.value);
const el = this.urlToEmbed(cleaned);
if (el) { if (el) {
fragment.appendChild(el); fragment.appendChild(el);
} else { } else {
const a = document.createElement("a"); const a = document.createElement("a");
a.href = part.value; a.href = cleaned;
a.target = "_blank"; a.target = "_blank";
a.rel = "noopener noreferrer"; a.rel = "noopener noreferrer";
a.textContent = part.value; a.textContent = cleaned;
fragment.appendChild(a); fragment.appendChild(a);
} }
if (trail) {
fragment.appendChild(document.createTextNode(trail));
}
} }
} }
@ -282,6 +286,14 @@ export class ContentRenderer {
return null; return null;
} }
stripTrailingPunct(url) {
const m = url.match(/[.,;:!?)\]}\"']+$/);
if (m) {
return { cleaned: url.slice(0, m.index), trail: m[0] };
}
return { cleaned: url, trail: "" };
}
walkNodes(root, callback) { walkNodes(root, callback) {
const skipTags = new Set(["CODE", "PRE", "A", "IFRAME", "IMG", "VIDEO", "SCRIPT", "STYLE"]); const skipTags = new Set(["CODE", "PRE", "A", "IFRAME", "IMG", "VIDEO", "SCRIPT", "STYLE"]);
const iter = document.createNodeIterator(root, NodeFilter.SHOW_TEXT, null, false); const iter = document.createNodeIterator(root, NodeFilter.SHOW_TEXT, null, false);

View File

@ -263,3 +263,46 @@ def test_xss_legitimate_link_survives_audit():
out = str(render_content("see https://example.com/page ok")) out = str(render_content("see https://example.com/page ok"))
assert 'href="https://example.com/page"' in out assert 'href="https://example.com/page"' in out
assert_no_executable_html(out) assert_no_executable_html(out)
def test_bare_url_with_trailing_period():
out = str(render_content("see https://example.com/page."))
assert 'href="https://example.com/page"' in out
assert "</a>." in out
assert "https://example.com/page.</a>" not in out
def test_bare_url_with_trailing_comma():
out = str(render_content("check https://example.com/page,"))
assert 'href="https://example.com/page"' in out
assert "</a>," in out
def test_bare_url_with_trailing_exclamation():
out = str(render_content("look https://example.com/page!"))
assert 'href="https://example.com/page"' in out
assert "</a>!" in out
def test_bare_url_with_trailing_question_mark():
out = str(render_content("did you see https://example.com/page?"))
assert 'href="https://example.com/page"' in out
assert "</a>?" in out
def test_bare_url_with_trailing_paren():
out = str(render_content("see (https://example.com/page)"))
assert 'href="https://example.com/page"' in out
assert "</a>)" in out
def test_bare_url_with_trailing_multiple_punctuation():
out = str(render_content("visit https://example.com/page..."))
assert 'href="https://example.com/page"' in out
assert "</a>..." in out
def test_bare_url_without_trailing_punctuation_unchanged():
out = str(render_content("see https://example.com/page ok"))
assert 'href="https://example.com/page"' in out
assert "https://example.com/page</a> " in out