Skip to content

Commit 6c253ee

Browse files
committed
Improved the verification of submitted file extensions and mimetype.
- Legacy-Id: 9986
1 parent b751aeb commit 6c253ee

10 files changed

Lines changed: 78 additions & 19 deletions

File tree

bin/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,4 @@
2121
/ipcontroller*
2222
/ipengine*
2323
/iptest*
24+
/xml2rfc

ietf/submit/forms.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ def clean(self):
130130
continue
131131
self.file_types.append('.%s' % ext)
132132
if not ('.txt' in self.file_types or '.xml' in self.file_types):
133-
raise forms.ValidationError('You must submit either a .txt or an .xml file; didn\'t find either.')
133+
raise forms.ValidationError('You must submit at least a valid .txt or a valid .xml file; didn\'t find either.')
134134

135135
#debug.show('self.cleaned_data["xml"]')
136136
if self.cleaned_data.get('xml'):

ietf/submit/parsers/base.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import os
1+
22
import re
33
import magic
44
import datetime
@@ -40,6 +40,8 @@ def add_warning(self, warning_type, warning_str):
4040

4141

4242
class FileParser(object):
43+
ext = None
44+
mimetype = None
4345

4446
def __init__(self, fd):
4547
self.fd = fd
@@ -50,6 +52,8 @@ def __init__(self, fd):
5052
def critical_parse(self):
5153
self.parse_invalid_chars_in_filename()
5254
self.parse_max_size();
55+
self.parse_filename_extension()
56+
self.parse_file_type()
5357
self.parsed_info.metadata.submission_date = datetime.date.today()
5458
return self.parsed_info
5559

@@ -61,20 +65,18 @@ def parse_invalid_chars_in_filename(self):
6165
self.parsed_info.add_error('Invalid characters were found in the name of the file which was just submitted: %s' % ', '.join(set(chars)))
6266

6367
def parse_max_size(self):
64-
__, ext = os.path.splitext(self.fd.name)
65-
ext = ext.lstrip('.')
66-
max_size = settings.IDSUBMIT_MAX_DRAFT_SIZE[ext]
68+
max_size = settings.IDSUBMIT_MAX_DRAFT_SIZE[self.ext]
6769
if self.fd.size > max_size:
6870
self.parsed_info.add_error('File size is larger than the permitted maximum of %s' % filesizeformat(max_size))
6971
self.parsed_info.metadata.file_size = self.fd.size
7072

71-
def parse_filename_extension(self, ext):
72-
if not self.fd.name.lower().endswith('.'+ext):
73-
self.parsed_info.add_error('Expected the %s file to have extension ".%s", found "%s"' % (ext.upper(), ext, self.fd.name))
73+
def parse_filename_extension(self):
74+
if not self.fd.name.lower().endswith('.'+self.ext):
75+
self.parsed_info.add_error('Expected the %s file to have extension ".%s", found the name "%s"' % (self.ext.upper(), self.ext, self.fd.name))
7476

75-
def parse_file_type(self, ext, expected):
77+
def parse_file_type(self):
7678
self.fd.file.seek(0)
7779
content = self.fd.file.read(4096)
7880
mimetype = magic.from_buffer(content, mime=True)
79-
if not mimetype == expected:
80-
self.parsed_info.add_error('Expected an %s file of type "%s", found one of type "%s"' % (ext.upper(), expected, mimetype))
81+
if not mimetype == self.mimetype:
82+
self.parsed_info.add_error('Expected an %s file of type "%s", found one of type "%s"' % (self.ext.upper(), self.mimetype, mimetype))

ietf/submit/parsers/pdf_parser.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22

33

44
class PDFParser(FileParser):
5+
ext = 'pdf'
6+
mimetype = 'application/pdf'
57

68
# If some error is found after this method invocation
79
# no other file parsing is recommended
810
def critical_parse(self):
911
super(PDFParser, self).critical_parse()
10-
self.parse_filename_extension('pdf')
11-
self.parse_file_type('pdf', 'application/pdf')
1212
return self.parsed_info

ietf/submit/parsers/plain_parser.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55

66
class PlainParser(FileParser):
7+
ext = 'txt'
8+
mimetype = 'text/plain'
79

810
def __init__(self, fd):
911
super(PlainParser, self).__init__(fd)
@@ -12,8 +14,6 @@ def __init__(self, fd):
1214
# no other file parsing is recommended
1315
def critical_parse(self):
1416
super(PlainParser, self).critical_parse()
15-
self.parse_filename_extension('txt')
16-
self.parse_file_type('txt', 'text/plain')
1717
self.parse_file_charset()
1818
self.parse_name()
1919
return self.parsed_info

ietf/submit/parsers/ps_parser.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22

33

44
class PSParser(FileParser):
5+
ext = 'ps'
6+
mimetype = 'application/postscript'
57

68
# If some error is found after this method invocation
79
# no other file parsing is recommended
810
def critical_parse(self):
911
super(PSParser, self).critical_parse()
10-
self.parse_filename_extension('ps')
11-
self.parse_file_type('ps', 'application/postscript')
1212
return self.parsed_info

ietf/submit/parsers/xml_parser.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22

33

44
class XMLParser(FileParser):
5+
ext = 'xml'
6+
mimetype = 'application/xml'
57

68
# If some error is found after this method invocation
79
# no other file parsing is recommended
810
def critical_parse(self):
911
super(XMLParser, self).critical_parse()
10-
self.parse_filename_extension('xml')
11-
self.parse_file_type('xml', 'application/xml')
1212
return self.parsed_info
1313

ietf/submit/test_submission.bad

11 Bytes
Binary file not shown.

ietf/submit/tests.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,61 @@ def test_blackout_access(self):
726726
q = PyQuery(r.content)
727727
self.assertEqual(len(q('input[type=file][name=txt]')), 1)
728728

729+
def submit_bad_file(self, name, formats):
730+
731+
make_test_data()
732+
733+
rev = ""
734+
group = None
735+
736+
# break early in case of missing configuration
737+
self.assertTrue(os.path.exists(settings.IDSUBMIT_IDNITS_BINARY))
738+
739+
# get
740+
url = urlreverse('submit_upload_submission')
741+
r = self.client.get(url)
742+
self.assertEqual(r.status_code, 200)
743+
q = PyQuery(r.content)
744+
745+
# submit
746+
files = {}
747+
for format in formats:
748+
files[format] = self.submission_file(name, rev, group, "bad", "test_submission.bad")
749+
750+
r = self.client.post(url, files)
751+
752+
self.assertEqual(r.status_code, 200)
753+
q = PyQuery(r.content)
754+
self.assertTrue(len(q("form .has-error")) > 0)
755+
m = q('div.has-error span.help-block').text()
756+
757+
return r, q, m
758+
759+
def test_submit_bad_file_txt(self):
760+
r, q, m = self.submit_bad_file("some name", ["txt"])
761+
self.assertIn('Invalid characters were found in the name', m)
762+
self.assertIn('Expected the TXT file to have extension ".txt"', m)
763+
self.assertIn('Expected an TXT file of type "text/plain"', m)
764+
self.assertIn('document does not contain a legitimate name', m)
765+
766+
def test_submit_bad_file_xml(self):
767+
r, q, m = self.submit_bad_file("some name", ["xml"])
768+
self.assertIn('Invalid characters were found in the name', m)
769+
self.assertIn('Expected the XML file to have extension ".xml"', m)
770+
self.assertIn('Expected an XML file of type "application/xml"', m)
771+
772+
def test_submit_bad_file_pdf(self):
773+
r, q, m = self.submit_bad_file("some name", ["pdf"])
774+
self.assertIn('Invalid characters were found in the name', m)
775+
self.assertIn('Expected the PDF file to have extension ".pdf"', m)
776+
self.assertIn('Expected an PDF file of type "application/pdf"', m)
777+
778+
def test_submit_bad_file_ps(self):
779+
r, q, m = self.submit_bad_file("some name", ["ps"])
780+
self.assertIn('Invalid characters were found in the name', m)
781+
self.assertIn('Expected the PS file to have extension ".ps"', m)
782+
self.assertIn('Expected an PS file of type "application/postscript"', m)
783+
729784
class ApprovalsTestCase(TestCase):
730785
def test_approvals(self):
731786
make_test_data()

static/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1+
/*
12
/*.pyc

0 commit comments

Comments
 (0)