fix: better draft name validation. Fixes #3539. (#3671)

This commit is contained in:
Robert Sparks 2022-03-18 18:01:25 -05:00 committed by GitHub
parent ec3b525567
commit 9df659b06e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 105 additions and 94 deletions

View file

@ -799,7 +799,7 @@ class MeetingTests(BaseMeetingTestCase):
"1. WG status (15 minutes)\n\n2. Status of %s\n\n" % draft2.name)
filenames = []
for d in (draft1, draft2):
file,_ = submission_file(name=d.name,format='txt',templatename='test_submission.txt',group=session.group,rev="00")
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)
filename = os.path.join(d.get_file_path(),file.name)
with io.open(filename,'w') as draftbits:
draftbits.write(file.getvalue())

View file

@ -207,83 +207,84 @@ class SubmissionBaseUploadForm(forms.Form):
self.add_error('xml', "No docName attribute found in the xml root element")
name_error = validate_submission_name(draftname)
if name_error:
self.add_error('xml', name_error)
revmatch = re.search("-[0-9][0-9]$", draftname)
if revmatch:
self.revision = draftname[-2:]
self.filename = draftname[:-3]
self.add_error('xml', name_error) # This is a critical and immediate failure - do not proceed with other validation.
else:
self.revision = None
self.filename = draftname
self.title = self.xmlroot.findtext('front/title').strip()
if type(self.title) is str:
self.title = unidecode(self.title)
self.title = normalize_text(self.title)
self.abstract = (self.xmlroot.findtext('front/abstract') or '').strip()
if type(self.abstract) is str:
self.abstract = unidecode(self.abstract)
author_info = self.xmlroot.findall('front/author')
for author in author_info:
info = {
"name": author.attrib.get('fullname'),
"email": author.findtext('address/email'),
"affiliation": author.findtext('organization'),
}
elem = author.find('address/postal/country')
if elem != None:
ascii_country = elem.get('ascii', None)
info['country'] = ascii_country if ascii_country else elem.text
revmatch = re.search("-[0-9][0-9]$", draftname)
if revmatch:
self.revision = draftname[-2:]
self.filename = draftname[:-3]
else:
self.revision = None
self.filename = draftname
self.title = self.xmlroot.findtext('front/title').strip()
if type(self.title) is str:
self.title = unidecode(self.title)
self.title = normalize_text(self.title)
self.abstract = (self.xmlroot.findtext('front/abstract') or '').strip()
if type(self.abstract) is str:
self.abstract = unidecode(self.abstract)
author_info = self.xmlroot.findall('front/author')
for author in author_info:
info = {
"name": author.attrib.get('fullname'),
"email": author.findtext('address/email'),
"affiliation": author.findtext('organization'),
}
elem = author.find('address/postal/country')
if elem != None:
ascii_country = elem.get('ascii', None)
info['country'] = ascii_country if ascii_country else elem.text
for item in info:
if info[item]:
info[item] = info[item].strip()
self.authors.append(info)
for item in info:
if info[item]:
info[item] = info[item].strip()
self.authors.append(info)
# --- Prep the xml ---
file_name['xml'] = os.path.join(settings.IDSUBMIT_STAGING_PATH, '%s-%s%s' % (self.filename, self.revision, ext))
try:
prep = xml2rfc.PrepToolWriter(self.xmltree, quiet=True, liberal=True, keep_pis=[xml2rfc.V3_PI_TARGET])
prep.options.accept_prepped = True
self.xmltree.tree = prep.prep()
if self.xmltree.tree == None:
self.add_error('xml', "Error from xml2rfc (prep): %s" % prep.errors)
except Exception as e:
msgs = format_messages('prep', e, xml2rfc.log)
self.add_error('xml', msgs)
# --- Convert to txt ---
if not ('txt' in self.cleaned_data and self.cleaned_data['txt']):
file_name['txt'] = os.path.join(settings.IDSUBMIT_STAGING_PATH, '%s-%s.txt' % (self.filename, self.revision))
# --- Prep the xml ---
file_name['xml'] = os.path.join(settings.IDSUBMIT_STAGING_PATH, '%s-%s%s' % (self.filename, self.revision, ext))
try:
writer = xml2rfc.TextWriter(self.xmltree, quiet=True)
writer.options.accept_prepped = True
writer.write(file_name['txt'])
log.log("In %s: xml2rfc %s generated %s from %s (version %s)" %
( os.path.dirname(file_name['xml']),
xml2rfc.__version__,
os.path.basename(file_name['txt']),
os.path.basename(file_name['xml']),
self.xml_version))
prep = xml2rfc.PrepToolWriter(self.xmltree, quiet=True, liberal=True, keep_pis=[xml2rfc.V3_PI_TARGET])
prep.options.accept_prepped = True
self.xmltree.tree = prep.prep()
if self.xmltree.tree == None:
self.add_error('xml', "Error from xml2rfc (prep): %s" % prep.errors)
except Exception as e:
msgs = format_messages('txt', e, xml2rfc.log)
log.log('\n'.join(msgs))
self.add_error('xml', msgs)
msgs = format_messages('prep', e, xml2rfc.log)
self.add_error('xml', msgs)
# --- Convert to html ---
try:
file_name['html'] = os.path.join(settings.IDSUBMIT_STAGING_PATH, '%s-%s.html' % (self.filename, self.revision))
writer = xml2rfc.HtmlWriter(self.xmltree, quiet=True)
writer.write(file_name['html'])
self.file_types.append('.html')
log.log("In %s: xml2rfc %s generated %s from %s (version %s)" %
( os.path.dirname(file_name['xml']),
xml2rfc.__version__,
os.path.basename(file_name['html']),
os.path.basename(file_name['xml']),
self.xml_version))
except Exception as e:
msgs = format_messages('html', e, xml2rfc.log)
self.add_error('xml', msgs)
# --- Convert to txt ---
if not ('txt' in self.cleaned_data and self.cleaned_data['txt']):
file_name['txt'] = os.path.join(settings.IDSUBMIT_STAGING_PATH, '%s-%s.txt' % (self.filename, self.revision))
try:
writer = xml2rfc.TextWriter(self.xmltree, quiet=True)
writer.options.accept_prepped = True
writer.write(file_name['txt'])
log.log("In %s: xml2rfc %s generated %s from %s (version %s)" %
( os.path.dirname(file_name['xml']),
xml2rfc.__version__,
os.path.basename(file_name['txt']),
os.path.basename(file_name['xml']),
self.xml_version))
except Exception as e:
msgs = format_messages('txt', e, xml2rfc.log)
log.log('\n'.join(msgs))
self.add_error('xml', msgs)
# --- Convert to html ---
try:
file_name['html'] = os.path.join(settings.IDSUBMIT_STAGING_PATH, '%s-%s.html' % (self.filename, self.revision))
writer = xml2rfc.HtmlWriter(self.xmltree, quiet=True)
writer.write(file_name['html'])
self.file_types.append('.html')
log.log("In %s: xml2rfc %s generated %s from %s (version %s)" %
( os.path.dirname(file_name['xml']),
xml2rfc.__version__,
os.path.basename(file_name['html']),
os.path.basename(file_name['xml']),
self.xml_version))
except Exception as e:
msgs = format_messages('html', e, xml2rfc.log)
self.add_error('xml', msgs)
except Exception as e:
try:
@ -293,6 +294,11 @@ class SubmissionBaseUploadForm(forms.Form):
except Exception:
self.add_error('xml', "An exception occurred when trying to process the XML file: %s" % e)
# The following errors are likely noise if we have previous field
# errors:
if self.errors:
raise forms.ValidationError('')
if self.cleaned_data.get('txt'):
# try to parse it
txt_file = self.cleaned_data['txt']

View file

@ -83,7 +83,7 @@ class BaseSubmitTestCase(TestCase):
def archive_dir(self):
return settings.INTERNET_DRAFT_ARCHIVE_DIR
def submission_file(name, rev, group, format, templatename, author=None, email=None, title=None, year=None, ascii=True):
def submission_file(name_in_doc, name_in_post, group, templatename, author=None, email=None, title=None, year=None, ascii=True):
# construct appropriate text draft
f = io.open(os.path.join(settings.BASE_DIR, "submit", templatename))
template = f.read()
@ -104,7 +104,7 @@ def submission_file(name, rev, group, format, templatename, author=None, email=N
year=year,
month=datetime.date.today().strftime("%B"),
day=datetime.date.today().strftime("%d"),
name="%s-%s" % (name, rev),
name=name_in_doc,
group=group or "",
author=author.ascii if ascii else author.name,
asciiAuthor=author.ascii,
@ -115,7 +115,7 @@ def submission_file(name, rev, group, format, templatename, author=None, email=N
title=title,
)
file = StringIO(submission_text)
file.name = "%s-%s.%s" % (name, rev, format)
file.name = name_in_post
return file, author
def create_draft_submission_with_rev_mismatch(rev='01'):
@ -168,7 +168,7 @@ class SubmitTests(BaseSubmitTestCase):
for format in formats:
fn = '.'.join((base_filename or 'test_submission', format))
files[format], __ = submission_file(name, rev, group, format, fn, author=author)
files[format], __ = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, fn, author=author)
r = self.client.post(url, files)
if r.status_code != 302:
@ -1599,7 +1599,7 @@ class SubmitTests(BaseSubmitTestCase):
def submit_bad_file(self, name, formats):
rev = ""
rev = "00"
group = None
# break early in case of missing configuration
@ -1614,7 +1614,7 @@ class SubmitTests(BaseSubmitTestCase):
# submit
files = {}
for format in formats:
files[format], author = submission_file(name, rev, group, "bad", "test_submission.bad")
files[format], author = submission_file(f'{name}-{rev}', f'{name}-{rev}.bad', group, "test_submission.bad")
r = self.client.post(url, files)
@ -1625,16 +1625,15 @@ class SubmitTests(BaseSubmitTestCase):
return r, q, m
def submit_bad_doc_name_with_ext(self, name, formats):
def submit_bad_doc_name_with_ext(self, name_in_doc, name_in_post, formats):
group = None
url = urlreverse('ietf.submit.views.upload_submission')
# submit
files = {}
for format in formats:
rev = '00.%s' % format
files[format], author = submission_file(name, rev, group, format, "test_submission.%s" % format)
files[format].name = "%s-%s.%s" % (name, 00, format)
files[format], author = submission_file(name_in_doc, name_in_post, group, "test_submission.%s" % format)
files[format].name = name_in_post
r = self.client.post(url, files)
@ -1652,10 +1651,15 @@ class SubmitTests(BaseSubmitTestCase):
self.assertIn('Expected an TXT file of type "text/plain"', m)
self.assertIn('document does not contain a legitimate name', m)
def test_submit_bad_doc_name_txt(self):
r, q, m = self.submit_bad_doc_name_with_ext("draft-foo.dot-bar", ["txt"])
def test_submit_bad_doc_name(self):
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"])
self.assertIn('contains a disallowed character with byte code: 46', m)
r, q, m = self.submit_bad_doc_name_with_ext("draft-foo-bar", ["xml"])
# This actually is allowed by the existing code. A significant rework of the validation mechanics is needed.
# 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"])
# self.assertIn('Did you include a filename extension in the name by mistake?', m)
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"])
self.assertIn('Did you include a filename extension in the name by mistake?', m)
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"])
self.assertIn('Did you include a filename extension in the name by mistake?', m)
def test_submit_bad_file_xml(self):
@ -1692,7 +1696,7 @@ class SubmitTests(BaseSubmitTestCase):
fn = os.path.join(dir, "%s-%s.%s" % (name, rev, format))
with io.open(fn, 'w') as f:
f.write("a" * 2000)
files[format], author = submission_file(name, rev, group, format, "test_submission.%s" % format)
files[format], author = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, "test_submission.%s" % format)
r = self.client.post(url, files)
self.assertEqual(r.status_code, 200)
@ -1717,7 +1721,7 @@ class SubmitTests(BaseSubmitTestCase):
user = UserFactory(first_name="Jörgen", last_name="Nilsson")
author = PersonFactory(user=user)
file, __ = submission_file(name, rev, group, "txt", "test_submission.nonascii", author=author, ascii=False)
file, __ = submission_file(f'{name}-{rev}', f'{name}-{rev}.txt', group, "test_submission.nonascii", author=author, ascii=False)
files = {"txt": file }
r = self.client.post(url, files)
@ -1738,7 +1742,7 @@ class SubmitTests(BaseSubmitTestCase):
for e in author.email_set.all():
e.delete()
files = {"txt": submission_file(name, rev, group, "txt", "test_submission.txt", author=author, ascii=True)[0] }
files = {"txt": submission_file(f'{name}-{rev}', f'{name}-{rev}.txt', group, "test_submission.txt", author=author, ascii=True)[0] }
# submit
url = urlreverse('ietf.submit.views.upload_submission')
@ -1762,7 +1766,7 @@ class SubmitTests(BaseSubmitTestCase):
email.address = '@bad.email'
email.save()
files = {"xml": submission_file(name, rev, group, "xml", "test_submission.xml", author=author, ascii=False)[0] }
files = {"xml": submission_file(f'{name}-{rev}',f'{name}-{rev}.xml', group, "test_submission.xml", author=author, ascii=False)[0] }
# submit
url = urlreverse('ietf.submit.views.upload_submission')
@ -1782,7 +1786,7 @@ class SubmitTests(BaseSubmitTestCase):
group = None
# submit
files = {"txt": submission_file(name, rev, group, "txt", "test_submission_invalid_yang.txt")[0] }
files = {"txt": submission_file(f'{name}-{rev}', f'{name}-{rev}.txt', group, "test_submission_invalid_yang.txt")[0] }
url = urlreverse('ietf.submit.views.upload_submission')
r = self.client.post(url, files)
@ -2673,7 +2677,7 @@ Subject: test
# submit
files = {}
for format in formats:
files[format], author = submission_file(name, rev, group, format, "test_submission.%s" % format)
files[format], author = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, "test_submission.%s" % format)
r = self.client.post(url, files)
if r.status_code != 302:
@ -2736,7 +2740,7 @@ class ApiSubmitTests(BaseSubmitTestCase):
email = author.user.username
# submit
data = {}
data['xml'], author = submission_file(name, rev, group, 'xml', "test_submission.xml", author=author, email=email, title=title, year=year)
data['xml'], author = submission_file(f'{name}-{rev}', f'{name}-{rev}.xml', group, "test_submission.xml", author=author, email=email, title=title, year=year)
data['user'] = email
r = self.client.post(url, data)
return r, author, name
@ -2857,7 +2861,7 @@ class RefsTests(BaseSubmitTestCase):
def test_draft_refs_identification(self):
group = None
file, __ = submission_file('draft-some-subject', '00', group, 'txt', "test_submission.txt", )
file, __ = submission_file('draft-some-subject-00', 'draft-some-subject-00.txt', group, "test_submission.txt", )
draft = PlaintextDraft(file.read(), file.name)
refs = draft.get_refs()
self.assertEqual(refs['rfc2119'], 'norm')

View file

@ -125,6 +125,7 @@ def validate_submission_name(name):
if '.' in name:
msg += " Did you include a filename extension in the name by mistake?"
return msg
return None
def validate_submission_rev(name, rev):
if not rev:

View file

@ -422,7 +422,7 @@ class PlaintextDraftTests(TestCase):
def setUp(self):
super().setUp()
file,_ = submission_file(name='draft-test-draft-class',rev='00',format='txt',templatename='test_submission.txt',group=None)
file,_ = submission_file(name_in_doc='draft-test-draft-class-00', name_in_post='draft-test-draft-class-00.txt',templatename='test_submission.txt',group=None)
self.draft = PlaintextDraft(text=file.getvalue(), source='draft-test-draft-class-00.txt', name_from_source=False)
def test_get_status(self):