Skip to content

Commit 9df659b

Browse files
rjsparksrjsparks
authored
fix: better draft name validation. Fixes ietf-tools#3539. (ietf-tools#3671)
1 parent ec3b525 commit 9df659b

File tree

5 files changed

+106
-95
lines changed

5 files changed

+106
-95
lines changed

‎ietf/meeting/tests_views.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,7 @@ def build_session_setup(self):
799799
"1. WG status (15 minutes)\n\n2. Status of %s\n\n" % draft2.name)
800800
filenames = []
801801
for d in (draft1, draft2):
802-
file,_ = submission_file(name=d.name,format='txt',templatename='test_submission.txt',group=session.group,rev="00")
802+
file,_ = submission_file(name_in_doc=f'{d.name}-00',name_in_post=f'{d.name}-00.txt',templatename='test_submission.txt',group=session.group)
803803
filename = os.path.join(d.get_file_path(),file.name)
804804
with io.open(filename,'w') as draftbits:
805805
draftbits.write(file.getvalue())

‎ietf/submit/forms.py‎

Lines changed: 78 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -207,84 +207,85 @@ def cleanup(): # called when context exited, even in case of exception
207207
self.add_error('xml', "No docName attribute found in the xml root element")
208208
name_error = validate_submission_name(draftname)
209209
if name_error:
210-
self.add_error('xml', name_error)
211-
revmatch = re.search("-[0-9][0-9]$", draftname)
212-
if revmatch:
213-
self.revision = draftname[-2:]
214-
self.filename = draftname[:-3]
210+
self.add_error('xml', name_error) # This is a critical and immediate failure - do not proceed with other validation.
215211
else:
216-
self.revision = None
217-
self.filename = draftname
218-
self.title = self.xmlroot.findtext('front/title').strip()
219-
if type(self.title) is str:
220-
self.title = unidecode(self.title)
221-
self.title = normalize_text(self.title)
222-
self.abstract = (self.xmlroot.findtext('front/abstract') or '').strip()
223-
if type(self.abstract) is str:
224-
self.abstract = unidecode(self.abstract)
225-
author_info = self.xmlroot.findall('front/author')
226-
for author in author_info:
227-
info = {
228-
"name": author.attrib.get('fullname'),
229-
"email": author.findtext('address/email'),
230-
"affiliation": author.findtext('organization'),
231-
}
232-
elem = author.find('address/postal/country')
233-
if elem != None:
234-
ascii_country = elem.get('ascii', None)
235-
info['country'] = ascii_country if ascii_country else elem.text
236-
237-
for item in info:
238-
if info[item]:
239-
info[item] = info[item].strip()
240-
self.authors.append(info)
241-
242-
# --- Prep the xml ---
243-
file_name['xml'] = os.path.join(settings.IDSUBMIT_STAGING_PATH, '%s-%s%s' % (self.filename, self.revision, ext))
244-
try:
245-
prep = xml2rfc.PrepToolWriter(self.xmltree, quiet=True, liberal=True, keep_pis=[xml2rfc.V3_PI_TARGET])
246-
prep.options.accept_prepped = True
247-
self.xmltree.tree = prep.prep()
248-
if self.xmltree.tree == None:
249-
self.add_error('xml', "Error from xml2rfc (prep): %s" % prep.errors)
250-
except Exception as e:
251-
msgs = format_messages('prep', e, xml2rfc.log)
252-
self.add_error('xml', msgs)
253-
254-
# --- Convert to txt ---
255-
if not ('txt' in self.cleaned_data and self.cleaned_data['txt']):
256-
file_name['txt'] = os.path.join(settings.IDSUBMIT_STAGING_PATH, '%s-%s.txt' % (self.filename, self.revision))
212+
revmatch = re.search("-[0-9][0-9]$", draftname)
213+
if revmatch:
214+
self.revision = draftname[-2:]
215+
self.filename = draftname[:-3]
216+
else:
217+
self.revision = None
218+
self.filename = draftname
219+
self.title = self.xmlroot.findtext('front/title').strip()
220+
if type(self.title) is str:
221+
self.title = unidecode(self.title)
222+
self.title = normalize_text(self.title)
223+
self.abstract = (self.xmlroot.findtext('front/abstract') or '').strip()
224+
if type(self.abstract) is str:
225+
self.abstract = unidecode(self.abstract)
226+
author_info = self.xmlroot.findall('front/author')
227+
for author in author_info:
228+
info = {
229+
"name": author.attrib.get('fullname'),
230+
"email": author.findtext('address/email'),
231+
"affiliation": author.findtext('organization'),
232+
}
233+
elem = author.find('address/postal/country')
234+
if elem != None:
235+
ascii_country = elem.get('ascii', None)
236+
info['country'] = ascii_country if ascii_country else elem.text
237+
238+
for item in info:
239+
if info[item]:
240+
info[item] = info[item].strip()
241+
self.authors.append(info)
242+
243+
# --- Prep the xml ---
244+
file_name['xml'] = os.path.join(settings.IDSUBMIT_STAGING_PATH, '%s-%s%s' % (self.filename, self.revision, ext))
257245
try:
258-
writer = xml2rfc.TextWriter(self.xmltree, quiet=True)
259-
writer.options.accept_prepped = True
260-
writer.write(file_name['txt'])
246+
prep = xml2rfc.PrepToolWriter(self.xmltree, quiet=True, liberal=True, keep_pis=[xml2rfc.V3_PI_TARGET])
247+
prep.options.accept_prepped = True
248+
self.xmltree.tree = prep.prep()
249+
if self.xmltree.tree == None:
250+
self.add_error('xml', "Error from xml2rfc (prep): %s" % prep.errors)
251+
except Exception as e:
252+
msgs = format_messages('prep', e, xml2rfc.log)
253+
self.add_error('xml', msgs)
254+
255+
# --- Convert to txt ---
256+
if not ('txt' in self.cleaned_data and self.cleaned_data['txt']):
257+
file_name['txt'] = os.path.join(settings.IDSUBMIT_STAGING_PATH, '%s-%s.txt' % (self.filename, self.revision))
258+
try:
259+
writer = xml2rfc.TextWriter(self.xmltree, quiet=True)
260+
writer.options.accept_prepped = True
261+
writer.write(file_name['txt'])
262+
log.log("In %s: xml2rfc %s generated %s from %s (version %s)" %
263+
( os.path.dirname(file_name['xml']),
264+
xml2rfc.__version__,
265+
os.path.basename(file_name['txt']),
266+
os.path.basename(file_name['xml']),
267+
self.xml_version))
268+
except Exception as e:
269+
msgs = format_messages('txt', e, xml2rfc.log)
270+
log.log('\n'.join(msgs))
271+
self.add_error('xml', msgs)
272+
273+
# --- Convert to html ---
274+
try:
275+
file_name['html'] = os.path.join(settings.IDSUBMIT_STAGING_PATH, '%s-%s.html' % (self.filename, self.revision))
276+
writer = xml2rfc.HtmlWriter(self.xmltree, quiet=True)
277+
writer.write(file_name['html'])
278+
self.file_types.append('.html')
261279
log.log("In %s: xml2rfc %s generated %s from %s (version %s)" %
262-
( os.path.dirname(file_name['xml']),
263-
xml2rfc.__version__,
264-
os.path.basename(file_name['txt']),
265-
os.path.basename(file_name['xml']),
266-
self.xml_version))
280+
( os.path.dirname(file_name['xml']),
281+
xml2rfc.__version__,
282+
os.path.basename(file_name['html']),
283+
os.path.basename(file_name['xml']),
284+
self.xml_version))
267285
except Exception as e:
268-
msgs = format_messages('txt', e, xml2rfc.log)
269-
log.log('\n'.join(msgs))
286+
msgs = format_messages('html', e, xml2rfc.log)
270287
self.add_error('xml', msgs)
271288

272-
# --- Convert to html ---
273-
try:
274-
file_name['html'] = os.path.join(settings.IDSUBMIT_STAGING_PATH, '%s-%s.html' % (self.filename, self.revision))
275-
writer = xml2rfc.HtmlWriter(self.xmltree, quiet=True)
276-
writer.write(file_name['html'])
277-
self.file_types.append('.html')
278-
log.log("In %s: xml2rfc %s generated %s from %s (version %s)" %
279-
( os.path.dirname(file_name['xml']),
280-
xml2rfc.__version__,
281-
os.path.basename(file_name['html']),
282-
os.path.basename(file_name['xml']),
283-
self.xml_version))
284-
except Exception as e:
285-
msgs = format_messages('html', e, xml2rfc.log)
286-
self.add_error('xml', msgs)
287-
288289
except Exception as e:
289290
try:
290291
msgs = format_messages('txt', e, xml2rfc.log)
@@ -293,6 +294,11 @@ def cleanup(): # called when context exited, even in case of exception
293294
except Exception:
294295
self.add_error('xml', "An exception occurred when trying to process the XML file: %s" % e)
295296

297+
# The following errors are likely noise if we have previous field
298+
# errors:
299+
if self.errors:
300+
raise forms.ValidationError('')
301+
296302
if self.cleaned_data.get('txt'):
297303
# try to parse it
298304
txt_file = self.cleaned_data['txt']

‎ietf/submit/tests.py‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ def repository_dir(self):
8383
def archive_dir(self):
8484
return settings.INTERNET_DRAFT_ARCHIVE_DIR
8585

86-
def submission_file(name, rev, group, format, templatename, author=None, email=None, title=None, year=None, ascii=True):
86+
def submission_file(name_in_doc, name_in_post, group, templatename, author=None, email=None, title=None, year=None, ascii=True):
8787
# construct appropriate text draft
8888
f = io.open(os.path.join(settings.BASE_DIR, "submit", templatename))
8989
template = f.read()
@@ -104,7 +104,7 @@ def submission_file(name, rev, group, format, templatename, author=None, email=N
104104
year=year,
105105
month=datetime.date.today().strftime("%B"),
106106
day=datetime.date.today().strftime("%d"),
107-
name="%s-%s" % (name, rev),
107+
name=name_in_doc,
108108
group=group or "",
109109
author=author.ascii if ascii else author.name,
110110
asciiAuthor=author.ascii,
@@ -115,7 +115,7 @@ def submission_file(name, rev, group, format, templatename, author=None, email=N
115115
title=title,
116116
)
117117
file = StringIO(submission_text)
118-
file.name = "%s-%s.%s" % (name, rev, format)
118+
file.name = name_in_post
119119
return file, author
120120

121121
def create_draft_submission_with_rev_mismatch(rev='01'):
@@ -168,7 +168,7 @@ def create_and_post_submission(self, name, rev, author, group=None, formats=("tx
168168

169169
for format in formats:
170170
fn = '.'.join((base_filename or 'test_submission', format))
171-
files[format], __ = submission_file(name, rev, group, format, fn, author=author)
171+
files[format], __ = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, fn, author=author)
172172

173173
r = self.client.post(url, files)
174174
if r.status_code != 302:
@@ -1599,7 +1599,7 @@ def test_no_blackout_at_all(self):
15991599

16001600

16011601
def submit_bad_file(self, name, formats):
1602-
rev = ""
1602+
rev = "00"
16031603
group = None
16041604

16051605
# break early in case of missing configuration
@@ -1614,7 +1614,7 @@ def submit_bad_file(self, name, formats):
16141614
# submit
16151615
files = {}
16161616
for format in formats:
1617-
files[format], author = submission_file(name, rev, group, "bad", "test_submission.bad")
1617+
files[format], author = submission_file(f'{name}-{rev}', f'{name}-{rev}.bad', group, "test_submission.bad")
16181618

16191619
r = self.client.post(url, files)
16201620

@@ -1625,16 +1625,15 @@ def submit_bad_file(self, name, formats):
16251625

16261626
return r, q, m
16271627

1628-
def submit_bad_doc_name_with_ext(self, name, formats):
1628+
def submit_bad_doc_name_with_ext(self, name_in_doc, name_in_post, formats):
16291629
group = None
16301630
url = urlreverse('ietf.submit.views.upload_submission')
16311631

16321632
# submit
16331633
files = {}
16341634
for format in formats:
1635-
rev = '00.%s' % format
1636-
files[format], author = submission_file(name, rev, group, format, "test_submission.%s" % format)
1637-
files[format].name = "%s-%s.%s" % (name, 00, format)
1635+
files[format], author = submission_file(name_in_doc, name_in_post, group, "test_submission.%s" % format)
1636+
files[format].name = name_in_post
16381637

16391638
r = self.client.post(url, files)
16401639

@@ -1652,10 +1651,15 @@ def test_submit_bad_file_txt(self):
16521651
self.assertIn('Expected an TXT file of type "text/plain"', m)
16531652
self.assertIn('document does not contain a legitimate name', m)
16541653

1655-
def test_submit_bad_doc_name_txt(self):
1656-
r, q, m = self.submit_bad_doc_name_with_ext("draft-foo.dot-bar", ["txt"])
1654+
def test_submit_bad_doc_name(self):
1655+
r, q, m = self.submit_bad_doc_name_with_ext(name_in_doc="draft-foo.dot-bar", name_in_post="draft-foo.dot-bar", formats=["txt"])
16571656
self.assertIn('contains a disallowed character with byte code: 46', m)
1658-
r, q, m = self.submit_bad_doc_name_with_ext("draft-foo-bar", ["xml"])
1657+
# This actually is allowed by the existing code. A significant rework of the validation mechanics is needed.
1658+
# r, q, m = self.submit_bad_doc_name_with_ext(name_in_doc="draft-foo-bar-00.txt", name_in_post="draft-foo-bar-00.txt", formats=["txt"])
1659+
# self.assertIn('Did you include a filename extension in the name by mistake?', m)
1660+
r, q, m = self.submit_bad_doc_name_with_ext(name_in_doc="draft-foo-bar-00.xml", name_in_post="draft-foo-bar-00.xml", formats=["xml"])
1661+
self.assertIn('Did you include a filename extension in the name by mistake?', m)
1662+
r, q, m = self.submit_bad_doc_name_with_ext(name_in_doc="../malicious-name-in-content-00", name_in_post="../malicious-name-in-post-00.xml", formats=["xml"])
16591663
self.assertIn('Did you include a filename extension in the name by mistake?', m)
16601664

16611665
def test_submit_bad_file_xml(self):
@@ -1692,7 +1696,7 @@ def test_submit_file_in_archive(self):
16921696
fn = os.path.join(dir, "%s-%s.%s" % (name, rev, format))
16931697
with io.open(fn, 'w') as f:
16941698
f.write("a" * 2000)
1695-
files[format], author = submission_file(name, rev, group, format, "test_submission.%s" % format)
1699+
files[format], author = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, "test_submission.%s" % format)
16961700
r = self.client.post(url, files)
16971701

16981702
self.assertEqual(r.status_code, 200)
@@ -1717,7 +1721,7 @@ def test_submit_nonascii_name(self):
17171721
user = UserFactory(first_name="Jörgen", last_name="Nilsson")
17181722
author = PersonFactory(user=user)
17191723

1720-
file, __ = submission_file(name, rev, group, "txt", "test_submission.nonascii", author=author, ascii=False)
1724+
file, __ = submission_file(f'{name}-{rev}', f'{name}-{rev}.txt', group, "test_submission.nonascii", author=author, ascii=False)
17211725
files = {"txt": file }
17221726

17231727
r = self.client.post(url, files)
@@ -1738,7 +1742,7 @@ def test_submit_missing_author_email(self):
17381742
for e in author.email_set.all():
17391743
e.delete()
17401744

1741-
files = {"txt": submission_file(name, rev, group, "txt", "test_submission.txt", author=author, ascii=True)[0] }
1745+
files = {"txt": submission_file(f'{name}-{rev}', f'{name}-{rev}.txt', group, "test_submission.txt", author=author, ascii=True)[0] }
17421746

17431747
# submit
17441748
url = urlreverse('ietf.submit.views.upload_submission')
@@ -1762,7 +1766,7 @@ def test_submit_bad_author_email(self):
17621766
email.address = '@bad.email'
17631767
email.save()
17641768

1765-
files = {"xml": submission_file(name, rev, group, "xml", "test_submission.xml", author=author, ascii=False)[0] }
1769+
files = {"xml": submission_file(f'{name}-{rev}',f'{name}-{rev}.xml', group, "test_submission.xml", author=author, ascii=False)[0] }
17661770

17671771
# submit
17681772
url = urlreverse('ietf.submit.views.upload_submission')
@@ -1782,7 +1786,7 @@ def test_submit_invalid_yang(self):
17821786
group = None
17831787

17841788
# submit
1785-
files = {"txt": submission_file(name, rev, group, "txt", "test_submission_invalid_yang.txt")[0] }
1789+
files = {"txt": submission_file(f'{name}-{rev}', f'{name}-{rev}.txt', group, "test_submission_invalid_yang.txt")[0] }
17861790

17871791
url = urlreverse('ietf.submit.views.upload_submission')
17881792
r = self.client.post(url, files)
@@ -2673,7 +2677,7 @@ def do_submission(self, name, rev, group=None, formats=["txt",]):
26732677
# submit
26742678
files = {}
26752679
for format in formats:
2676-
files[format], author = submission_file(name, rev, group, format, "test_submission.%s" % format)
2680+
files[format], author = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, "test_submission.%s" % format)
26772681

26782682
r = self.client.post(url, files)
26792683
if r.status_code != 302:
@@ -2736,7 +2740,7 @@ def do_post_submission(self, rev, author=None, name=None, group=None, email=None
27362740
email = author.user.username
27372741
# submit
27382742
data = {}
2739-
data['xml'], author = submission_file(name, rev, group, 'xml', "test_submission.xml", author=author, email=email, title=title, year=year)
2743+
data['xml'], author = submission_file(f'{name}-{rev}', f'{name}-{rev}.xml', group, "test_submission.xml", author=author, email=email, title=title, year=year)
27402744
data['user'] = email
27412745
r = self.client.post(url, data)
27422746
return r, author, name
@@ -2857,7 +2861,7 @@ class RefsTests(BaseSubmitTestCase):
28572861
def test_draft_refs_identification(self):
28582862

28592863
group = None
2860-
file, __ = submission_file('draft-some-subject', '00', group, 'txt', "test_submission.txt", )
2864+
file, __ = submission_file('draft-some-subject-00', 'draft-some-subject-00.txt', group, "test_submission.txt", )
28612865
draft = PlaintextDraft(file.read(), file.name)
28622866
refs = draft.get_refs()
28632867
self.assertEqual(refs['rfc2119'], 'norm')

‎ietf/submit/utils.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ def validate_submission_name(name):
125125
if '.' in name:
126126
msg += " Did you include a filename extension in the name by mistake?"
127127
return msg
128+
return None
128129

129130
def validate_submission_rev(name, rev):
130131
if not rev:

0 commit comments

Comments
 (0)