Skip to content

Commit 5f8d4ed

Browse files
larseggertlarseggert
authored
More HTML nitfixing (ietf-tools#3934)
* Unicode messages are triggered by both db content and tests * Make ids unique * Avoid "No value found" message on page * Strip HTML from history entries, it's often broken * Check HTML sources for occurrences of "** No value found for" and fix them * Fix another occurrence of "** No value found for" * Fix more occurrences of "** No value found for" * Fix document revision stripping * Force breaks of long (garbage) words * Check URL validity before urlizing them * Handle some additional corner cases * Linkify action items * Don't create profile/email links for System * Handle headings with HTML elements in them better * Fix comment * Fix another occurrence of "** No value found for" * Better I-D URLization that handles more edge cases. Also, test for them. * Remove print * Handle charters better * Cache for one day
1 parent 47b89c1 commit 5f8d4ed

15 files changed

Lines changed: 286 additions & 123 deletions

‎bin/test-crawl‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ args = parser.parse_args()
4444

4545
# Import Django, call setup()
4646
os.environ.setdefault("DJANGO_SETTINGS_MODULE", args.settings or "ietf.settings_testcrawl")
47-
os.environ["DJANGO_URLIZE_IETF_DOCS_PRODUCTION"] = "1"
4847

4948
import django
5049
import django.test
@@ -175,7 +174,7 @@ def check_html_valid(url, response, args):
175174
assert ret
176175
for m in json.loads(ret)["messages"]:
177176
if "lastLine" not in m:
178-
tag = m # just dump the raw JSON for now
177+
tag = m["message"]
179178
else:
180179
tag = vnu_fmt_message(url, m, content.decode())
181180
# disregard some HTML issues that are (usually) due to invalid
@@ -211,7 +210,7 @@ def skip_url(url):
211210
r"^/wg/[a-z0-9-]+/deps/svg/",
212211
# Skip other bad urls
213212
r"^/dir/tsvdir/reviews/",
214-
r"^/ipr/\d{,3}/history/",
213+
# r"^/ipr/\d{,3}/history/",
215214
# Skip most html conversions, not worth the time
216215
r"^/doc/html/draft-[0-9ac-z]",
217216
r"^/doc/html/draft-b[0-9b-z]",

‎ietf/doc/templatetags/ietf_filters.py‎

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import datetime
66
import re
7-
import os
87
from urllib.parse import urljoin
98

109
from email.utils import parseaddr
@@ -19,7 +18,6 @@
1918
from django.utils.encoding import force_str # pyflakes:ignore force_str is used in the doctests
2019
from django.urls import reverse as urlreverse
2120
from django.core.cache import cache
22-
from django.core.validators import URLValidator
2321
from django.core.exceptions import ValidationError
2422

2523
import debug # pyflakes:ignore
@@ -29,7 +27,7 @@
2927
from ietf.utils.html import sanitize_fragment
3028
from ietf.utils import log
3129
from ietf.doc.utils import prettify_std_name
32-
from ietf.utils.text import wordwrap, fill, wrap_text_if_unwrapped, bleach_linker
30+
from ietf.utils.text import wordwrap, fill, wrap_text_if_unwrapped, bleach_linker, bleach_cleaner, validate_url
3331

3432
register = template.Library()
3533

@@ -189,69 +187,82 @@ def rfceditor_info_url(rfcnum : str):
189187
return urljoin(settings.RFC_EDITOR_INFO_BASE_URL, f'rfc{rfcnum}')
190188

191189

192-
def doc_exists(name):
193-
"""Check whether a given document exists"""
190+
def doc_canonical_name(name):
191+
"""Check whether a given document exists, and return its canonical name"""
192+
194193
def find_unique(n):
195194
key = hash(n)
196195
found = cache.get(key)
197196
if not found:
198197
exact = DocAlias.objects.filter(name=n).first()
199198
found = exact.name if exact else "_"
200-
cache.set(key, found)
199+
cache.set(key, found, timeout=60*60*24) # cache for one day
201200
return None if found == "_" else found
202201

203-
# all documents exist when tests are running
204-
if settings.SERVER_MODE == 'test':
205-
# unless we are running test-crawl, which would otherwise 404
206-
if "DJANGO_URLIZE_IETF_DOCS_PRODUCTION" not in os.environ:
207-
return True
208-
209202
# chop away extension
210-
extension_split = re.search(r"^(.+)\.(txt|ps|pdf)$", name)
203+
extension_split = re.search(r"^(.+)\.(txt|ps|pdf|html)$", name)
211204
if extension_split:
212205
name = extension_split.group(1)
213206

214207
if find_unique(name):
215-
return True
208+
return name
216209

217210
# check for embedded rev - this may be ambiguous, so don't
218211
# chop it off if we don't find a match
219-
rev_split = re.search("^(.+)-([0-9]{2,})$", name)
212+
rev_split = re.search(r"^(charter-.+)-(\d{2}-\d{2})$", name) or re.search(
213+
r"^(.+)-(\d{2}|[1-9]\d{2,})$", name
214+
)
220215
if rev_split:
221216
name = rev_split.group(1)
222217
if find_unique(name):
223-
return True
218+
return name
224219

225-
return False
220+
return ""
226221

227222

228223
def link_charter_doc_match1(match):
229-
if not doc_exists(match[0]):
224+
if not doc_canonical_name(match[0]):
230225
return match[0]
231226
return f'<a href="/doc/{match[1][:-1]}/{match[2]}/">{match[0]}</a>'
232227

233228

234229
def link_charter_doc_match2(match):
235-
if not doc_exists(match[0]):
230+
if not doc_canonical_name(match[0]):
236231
return match[0]
237232
return f'<a href="/doc/{match[1][:-1]}/{match[2]}/">{match[0]}</a>'
238233

239234

240235
def link_non_charter_doc_match(match):
241-
if not doc_exists(match[0]):
236+
name = match[0]
237+
cname = doc_canonical_name(name)
238+
if not cname:
242239
return match[0]
243-
if len(match[3]) == 2 and match[3].isdigit():
244-
return f'<a href="/doc/{match[2][:-1]}/{match[3]}/">{match[0]}</a>'
240+
if name == cname:
241+
return f'<a href="/doc/{cname}/">{match[0]}</a>'
242+
243+
# if we get here, the name probably has a version number and/or extension at the end
244+
rev_split = re.search(r"^(" + re.escape(cname) + r")-(\d{2,})", name)
245+
if rev_split:
246+
name = rev_split.group(1)
245247
else:
246-
return f'<a href="/doc/{match[2]}{match[3]}/">{match[0]}</a>'
248+
return f'<a href="/doc/{cname}/">{match[0]}</a>'
249+
250+
cname = doc_canonical_name(name)
251+
if not cname:
252+
return match[0]
253+
if name == cname:
254+
return f'<a href="/doc/{cname}/{rev_split.group(2)}/">{match[0]}</a>'
255+
256+
# if we get here, we can't linkify
257+
return match[0]
247258

248259

249260
def link_other_doc_match(match):
250-
# there may be whitespace in the match
251-
doc = re.sub(r"\s+", "", match[0])
252-
if not doc_exists(doc):
261+
doc = match[2].strip().lower()
262+
rev = match[3]
263+
if not doc_canonical_name(doc + rev):
253264
return match[0]
254-
return f'<a href="/doc/{match[2].strip().lower()}{match[3]}/">{match[1]}</a>'
265+
return f'<a href="/doc/{doc}{rev}/">{match[1]}</a>'
255266

256267

257268
@register.filter(name="urlize_ietf_docs", is_safe=True, needs_autoescape=True)
@@ -264,8 +275,8 @@ def urlize_ietf_docs(string, autoescape=None):
264275
string = escape(string)
265276
else:
266277
string = mark_safe(string)
267-
exp1 = r"\b(?<![/\-:=#])(charter-(?:[\d\w\.+]+-)*)(\d\d-\d\d)(\.txt)?\b"
268-
exp2 = r"\b(?<![/\-:=#])(charter-(?:[\d\w\.+]+-)*)(\d\d)(\.txt)?\b"
278+
exp1 = r"\b(?<![/\-:=#])(charter-(?:[\d\w\.+]+-)*)(\d{2}-\d{2})(\.(?:txt|ps|pdf|html))?\b"
279+
exp2 = r"\b(?<![/\-:=#])(charter-(?:[\d\w\.+]+-)*)(\d{2})(\.(?:txt|ps|pdf|html))?\b"
269280
if re.search(exp1, string):
270281
string = re.sub(
271282
exp1,
@@ -281,7 +292,8 @@ def urlize_ietf_docs(string, autoescape=None):
281292
flags=re.IGNORECASE | re.ASCII,
282293
)
283294
string = re.sub(
284-
r"\b(?<![/\-:=#])(((?:draft-|bofreq-|conflict-review-|status-change-)(?:[\d\w\.+]+-)*)([\d\w\.+]+?)(\.txt)?)\b(?![-@])",
295+
r"\b(?<![/\-:=#])((?:draft-|bofreq-|conflict-review-|status-change-)[\d\w\.+-]+(?![-@]))",
296+
# r"\b(?<![/\-:=#])(((?:draft-|bofreq-|conflict-review-|status-change-)(?:[\d\w\.+]+-)*)([\d\w\.+]+?)(\.(?:txt|ps|pdf|html))?)\b(?![-@])",
285297
link_non_charter_doc_match,
286298
string,
287299
flags=re.IGNORECASE | re.ASCII,
@@ -295,6 +307,7 @@ def urlize_ietf_docs(string, autoescape=None):
295307
)
296308
return mark_safe(string)
297309

310+
298311
urlize_ietf_docs = stringfilter(urlize_ietf_docs)
299312

300313
@register.filter(name='urlize_related_source_list', is_safe=True, needs_autoescape=True)
@@ -492,10 +505,8 @@ def ad_area(user):
492505
@register.filter
493506
def format_history_text(text, trunc_words=25):
494507
"""Run history text through some cleaning and add ellipsis if it's too long."""
495-
full = mark_safe(text)
496-
if "</a>" not in full:
497-
full = urlize_ietf_docs(full)
498-
full = bleach_linker.linkify(full)
508+
full = mark_safe(bleach_cleaner.clean(text))
509+
full = bleach_linker.linkify(urlize_ietf_docs(full))
499510

500511
return format_snippet(full, trunc_words)
501512

@@ -840,7 +851,6 @@ def is_valid_url(url):
840851
"""
841852
Check if the given URL is syntactically valid
842853
"""
843-
validate_url = URLValidator()
844854
try:
845855
validate_url(url)
846856
except ValidationError:

‎ietf/doc/templatetags/tests_ietf_filters.py‎

Lines changed: 109 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,76 +1,147 @@
11
# Copyright The IETF Trust 2022, All Rights Reserved
22

3-
from ietf.doc.templatetags.ietf_filters import urlize_ietf_docs
3+
from django.conf import settings
4+
5+
from ietf.doc.factories import (
6+
WgDraftFactory,
7+
IndividualDraftFactory,
8+
CharterFactory,
9+
NewRevisionDocEventFactory,
10+
)
11+
from ietf.doc.models import State, DocEvent, DocAlias
12+
from ietf.doc.templatetags.ietf_filters import urlize_ietf_docs, is_valid_url
13+
from ietf.person.models import Person
414
from ietf.utils.test_utils import TestCase
515

6-
import debug # pyflakes: ignore
16+
import debug # pyflakes: ignore
17+
718
# TODO: most other filters need test cases, too
819

920

1021
class IetfFiltersTests(TestCase):
22+
def test_is_valid_url(self):
23+
cases = [(settings.IDTRACKER_BASE_URL, True), ("not valid", False)]
24+
for url, result in cases:
25+
self.assertEqual(is_valid_url(url), result)
26+
1127
def test_urlize_ietf_docs(self):
28+
wg_id = WgDraftFactory()
29+
wg_id.set_state(State.objects.get(type="draft", slug="rfc"))
30+
wg_id.std_level_id = "bcp"
31+
wg_id.save_with_history(
32+
[
33+
DocEvent.objects.create(
34+
doc=wg_id,
35+
rev=wg_id.rev,
36+
type="published_rfc",
37+
by=Person.objects.get(name="(System)"),
38+
)
39+
]
40+
)
41+
DocAlias.objects.create(name="rfc123456").docs.add(wg_id)
42+
DocAlias.objects.create(name="bcp123456").docs.add(wg_id)
43+
DocAlias.objects.create(name="std123456").docs.add(wg_id)
44+
DocAlias.objects.create(name="fyi123456").docs.add(wg_id)
45+
46+
id = IndividualDraftFactory(name="draft-me-rfc123456bis")
47+
id_num = IndividualDraftFactory(name="draft-rosen-rfcefdp-update-2026")
48+
id_num_two = IndividualDraftFactory(name="draft-spaghetti-idr-deprecate-8-9-10")
49+
id_plus = IndividualDraftFactory(name="draft-odell-8+8")
50+
id_plus_end = IndividualDraftFactory(name="draft-durand-gse+")
51+
id_dot = IndividualDraftFactory(name="draft-ietf-pem-ansix9.17")
52+
charter = CharterFactory()
53+
e = NewRevisionDocEventFactory(doc=charter, rev="01")
54+
charter.rev = e.rev
55+
charter.save_with_history([e])
56+
e = NewRevisionDocEventFactory(doc=charter, rev="01-00")
57+
charter.rev = e.rev
58+
charter.save_with_history([e])
59+
1260
cases = [
1361
("no change", "no change"),
14-
("bcp1", '<a href="/doc/bcp1/">bcp1</a>'),
15-
("Std 003", '<a href="/doc/std3/">Std 003</a>'),
62+
("bCp123456", '<a href="/doc/bcp123456/">bCp123456</a>'),
63+
("Std 00123456", '<a href="/doc/std123456/">Std 00123456</a>'),
1664
(
17-
"FYI02 changes Std 003",
18-
'<a href="/doc/fyi2/">FYI02</a> changes <a href="/doc/std3/">Std 003</a>',
65+
"FyI 0123456 changes std 00123456",
66+
'<a href="/doc/fyi123456/">FyI 0123456</a> changes <a href="/doc/std123456/">std 00123456</a>',
1967
),
20-
("rfc2119", '<a href="/doc/rfc2119/">rfc2119</a>'),
21-
("Rfc 02119", '<a href="/doc/rfc2119/">Rfc 02119</a>'),
22-
("draft-abc-123", '<a href="/doc/draft-abc-123/">draft-abc-123</a>'),
68+
("rfc123456", '<a href="/doc/rfc123456/">rfc123456</a>'),
69+
("Rfc 0123456", '<a href="/doc/rfc123456/">Rfc 0123456</a>'),
70+
(wg_id.name, f'<a href="/doc/{wg_id.name}/">{wg_id.name}</a>'),
2371
(
24-
"draft-ietf-rfc9999-bis-01.txt",
25-
'<a href="/doc/draft-ietf-rfc9999-bis/01/">draft-ietf-rfc9999-bis-01.txt</a>',
72+
f"{id.name}-{id.rev}.txt",
73+
f'<a href="/doc/{id.name}/{id.rev}/">{id.name}-{id.rev}.txt</a>',
2674
),
2775
(
28-
"foo RFC 9999 draft-ietf-rfc9999-bis-01 bar",
29-
'foo <a href="/doc/rfc9999/">RFC 9999</a> <a href="/doc/draft-ietf-rfc9999-bis/01/">draft-ietf-rfc9999-bis-01</a> bar',
76+
f"foo RFC 123456 {id.name}-{id.rev} bar",
77+
f'foo <a href="/doc/rfc123456/">RFC 123456</a> <a href="/doc/{id.name}/{id.rev}/">{id.name}-{id.rev}</a> bar',
3078
),
3179
(
32-
"New version available: <b>draft-bryan-sipping-p2p-03.txt</b>",
33-
'New version available: <b><a href="/doc/draft-bryan-sipping-p2p/03/">draft-bryan-sipping-p2p-03.txt</a></b>',
80+
f"New version available: <b>{id.name}-{id.rev}.txt</b>",
81+
f'New version available: <b><a href="/doc/{id.name}/{id.rev}/">{id.name}-{id.rev}.txt</a></b>',
3482
),
3583
(
36-
"New version available: <b>charter-ietf-6man-04.txt</b>",
37-
'New version available: <b><a href="/doc/charter-ietf-6man/04/">charter-ietf-6man-04.txt</a></b>'
84+
f"New version available: <b>{charter.name}-{charter.rev}.txt</b>",
85+
f'New version available: <b><a href="/doc/{charter.name}/{charter.rev}/">{charter.name}-{charter.rev}.txt</a></b>',
3886
),
3987
(
40-
"New version available: <b>charter-ietf-6man-03-07.txt</b>",
41-
'New version available: <b><a href="/doc/charter-ietf-6man/03-07/">charter-ietf-6man-03-07.txt</a></b>'
88+
f"New version available: <b>{charter.name}-01-00.txt</b>",
89+
f'New version available: <b><a href="/doc/{charter.name}/01-00/">{charter.name}-01-00.txt</a></b>',
4290
),
4391
(
4492
"repository https://github.com/tlswg/draft-ietf-tls-ticketrequest",
45-
'repository https://github.com/tlswg/draft-ietf-tls-ticketrequest'
93+
"repository https://github.com/tlswg/draft-ietf-tls-ticketrequest",
94+
),
95+
(
96+
'<a href="mailto:draft-ietf-some-names@ietf.org">draft-ietf-some-names@ietf.org</a>',
97+
'<a href="mailto:draft-ietf-some-names@ietf.org">draft-ietf-some-names@ietf.org</a>',
4698
),
4799
(
48-
"draft-madanapalli-nd-over-802.16-problems",
49-
'<a href="/doc/draft-madanapalli-nd-over-802.16-problems/">draft-madanapalli-nd-over-802.16-problems</a>'
100+
"http://ieee802.org/1/files/public/docs2015/cn-thaler-Qcn-draft-PAR.pdf",
101+
"http://ieee802.org/1/files/public/docs2015/cn-thaler-Qcn-draft-PAR.pdf",
50102
),
51103
(
52-
"draft-madanapalli-nd-over-802.16-problems-02.txt",
53-
'<a href="/doc/draft-madanapalli-nd-over-802.16-problems/02/">draft-madanapalli-nd-over-802.16-problems-02.txt</a>'
104+
f"{id_num.name}.pdf",
105+
f'<a href="/doc/{id_num.name}/">{id_num.name}.pdf</a>',
54106
),
55107
(
56-
'<a href="mailto:draft-ietf-some-names@ietf.org">draft-ietf-some-names@ietf.org</a>',
57-
'<a href="mailto:draft-ietf-some-names@ietf.org">draft-ietf-some-names@ietf.org</a>',
108+
f"{id_num.name}-{id_num.rev}.txt",
109+
f'<a href="/doc/{id_num.name}/{id_num.rev}/">{id_num.name}-{id_num.rev}.txt</a>',
58110
),
59111
(
60-
"http://ieee802.org/1/files/public/docs2015/cn-thaler-Qcn-draft-PAR.pdf",
61-
"http://ieee802.org/1/files/public/docs2015/cn-thaler-Qcn-draft-PAR.pdf"
62-
)
112+
f"{id_num_two.name}.pdf",
113+
f'<a href="/doc/{id_num_two.name}/">{id_num_two.name}.pdf</a>',
114+
),
115+
(
116+
f"{id_num_two.name}-{id_num_two.rev}.txt",
117+
f'<a href="/doc/{id_num_two.name}/{id_num_two.rev}/">{id_num_two.name}-{id_num_two.rev}.txt</a>',
118+
),
119+
(
120+
f"{id_plus.name}",
121+
f'<a href="/doc/{id_plus.name}/">{id_plus.name}</a>',
122+
),
123+
(
124+
f"{id_plus.name}-{id_plus.rev}.txt",
125+
f'<a href="/doc/{id_plus.name}/{id_plus.rev}/">{id_plus.name}-{id_plus.rev}.txt</a>',
126+
),
127+
(
128+
f"{id_plus_end.name}",
129+
f'<a href="/doc/{id_plus_end.name}/">{id_plus_end.name}</a>',
130+
),
131+
(
132+
f"{id_plus_end.name}-{id_plus_end.rev}.txt",
133+
f'<a href="/doc/{id_plus_end.name}/{id_plus_end.rev}/">{id_plus_end.name}-{id_plus_end.rev}.txt</a>',
134+
),
135+
(
136+
f"{id_dot.name}",
137+
f'<a href="/doc/{id_dot.name}/">{id_dot.name}</a>',
138+
),
139+
(
140+
f"{id_dot.name}-{id_dot.rev}.txt",
141+
f'<a href="/doc/{id_dot.name}/{id_dot.rev}/">{id_dot.name}-{id_dot.rev}.txt</a>',
142+
),
63143
]
64144

65-
# Some edge cases scraped from existing old draft names
66-
for name in [
67-
# "draft-odell-8+8", # This fails since + matches the right side of \b
68-
# "draft-durand-gse+", # same failure
69-
"draft-kim-xcast+-few-2-few",
70-
#"draft-ietf-pem-ansix9.17", # Fails because of not being greedy with . before txt
71-
]:
72-
cases.append((name,f'<a href="/doc/{name}/">{name}</a>'))
73-
74145
for input, output in cases:
75146
#debug.show("(urlize_ietf_docs(input),output)")
76147
self.assertEqual(urlize_ietf_docs(input), output)

0 commit comments

Comments
 (0)