Mercurial > p > roundup > code
changeset 6190:15fd91fd3c4c
Quote all exported CSV data
Quote all non-numeric data in csv export functions. Report that a
title like '=a2+b3' could be interpreted as a function in Excel and
executed. csv.writer now includes quoting=csv.QUOTE_NONNUMERIC to
generate quoted values for all fields. This should make the string
starting with = be interpreted as a string and not a formula.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Mon, 08 Jun 2020 16:18:21 -0400 |
| parents | 7458211ca6f3 |
| children | 5713ddd87fd3 |
| files | CHANGES.txt doc/upgrading.txt roundup/cgi/actions.py test/test_cgi.py |
| diffstat | 4 files changed, 67 insertions(+), 30 deletions(-) [+] |
line wrap: on
line diff
--- a/CHANGES.txt Sun Jun 07 18:10:51 2020 -0400 +++ b/CHANGES.txt Mon Jun 08 16:18:21 2020 -0400 @@ -19,6 +19,12 @@ Meerwald) - exception in logout action when there is no session (Christof Meerwald) +- quote all non-numeric data in csv export functions. Report that a + title like '=a2+b3' could be interpreted as a function in Excel and + executed. csv.writer now includes quoting=csv.QUOTE_NONNUMERIC to + generate quoted values for all fields. This makes the string + starting with = be interpreted as a string and not a formula. (John + Rouillard as reported in the decomissioned bpo meta tracker IIRC.) Features:
--- a/doc/upgrading.txt Sun Jun 07 18:10:51 2020 -0400 +++ b/doc/upgrading.txt Mon Jun 08 16:18:21 2020 -0400 @@ -160,6 +160,37 @@ if db.tx_Source in ['web', 'rest', 'xmlrpc', 'email-sig-openpgp', 'cli' ]: + +CSV export changes +------------------ + +The original Roundup CSV export function for indexes reported id +numbers for links. The wiki had a version that resolved the id's to +names, so it would report ``open`` rather than ``2`` or +``user2;user3`` rather than ``[2,3]``. + +Many people added the enhanced version to their extensions directory. + +The enhanced version was made the default in roundup 2.0. If you want +to use the old version (that returns id's), you can replace references +to ``export_csv`` with ``export_csv_id`` in templates. + +Both core csv export functions have been changed to force quoting of +all exported fields. To incorporate this change in any CSV export +extension you may have added, change references in your code from:: + + writer = csv.writer(wfile) + +to:: + + writer = csv.writer(wfile, quoting=csv.QUOTE_NONNUMERIC) + +this forces all (non-numeric) fields to be quoted and empty quotes to +be added for missing parameters. + +This turns exported values that may look like formulas into strings so +some versions of Excel won't try to interpret them as a formula. + Update userauditor.py to restrict usernames -------------------------------------------
--- a/roundup/cgi/actions.py Sun Jun 07 18:10:51 2020 -0400 +++ b/roundup/cgi/actions.py Mon Jun 08 16:18:21 2020 -0400 @@ -1438,7 +1438,7 @@ self.client.STORAGE_CHARSET, self.client.charset, 'replace') - writer = csv.writer(wfile) + writer = csv.writer(wfile, quoting=csv.QUOTE_NONNUMERIC) # handle different types of columns. def repr_no_right(cls, col): @@ -1603,7 +1603,7 @@ self.client.STORAGE_CHARSET, self.client.charset, 'replace') - writer = csv.writer(wfile) + writer = csv.writer(wfile, quoting=csv.QUOTE_NONNUMERIC) self.client._socket_op(writer.writerow, columns) # and search
--- a/test/test_cgi.py Sun Jun 07 18:10:51 2020 -0400 +++ b/test/test_cgi.py Mon Jun 08 16:18:21 2020 -0400 @@ -1792,25 +1792,25 @@ cl.request.wfile = output # call export version that outputs names actions.ExportCSVAction(cl).handle() - #print(output.getvalue()) - should_be=(s2b('id,title,status,keyword,assignedto,nosy\r\n' - '1,foo1,deferred,,"Contrary, Mary","Bork, Chef;Contrary, Mary;demo"\r\n' - '2,bar2,unread,keyword1;keyword2,"Bork, Chef","Bork, Chef"\r\n' - '3,baz32,need-eg,,,\r\n')) + should_be=(s2b('"id","title","status","keyword","assignedto","nosy"\r\n' + '"1","foo1","deferred","","Contrary, Mary","Bork, Chef;Contrary, Mary;demo"\r\n' + '"2","bar2","unread","keyword1;keyword2","Bork, Chef","Bork, Chef"\r\n' + '"3","baz32","need-eg","","",""\r\n')) #print(should_be) - #print(output.getvalue()) + print(output.getvalue()) self.assertEqual(output.getvalue(), should_be) output = io.BytesIO() cl.request = MockNull() cl.request.wfile = output # call export version that outputs id numbers actions.ExportCSVWithIdAction(cl).handle() + should_be = s2b('"id","title","status","keyword","assignedto","nosy"\r\n' + "\"1\",\"foo1\",\"2\",\"[]\",\"4\",\"['3', '4', '5']\"\r\n" + "\"2\",\"bar2\",\"1\",\"['1', '2']\",\"3\",\"['3']\"\r\n" + '\"3\","baz32",\"4\","[]","None","[]"\r\n') + #print(should_be) print(output.getvalue()) - self.assertEqual(s2b('id,title,status,keyword,assignedto,nosy\r\n' - "1,foo1,2,[],4,\"['3', '4', '5']\"\r\n" - "2,bar2,1,\"['1', '2']\",3,['3']\r\n" - '3,baz32,4,[],None,[]\r\n'), - output.getvalue()) + self.assertEqual(output.getvalue(), should_be) def testCSVExportCharset(self): cl = self._make_client( @@ -1827,8 +1827,8 @@ cl.request.wfile = output # call export version that outputs names actions.ExportCSVAction(cl).handle() - should_be=(b'id,title,status,keyword,assignedto,nosy\r\n' - b'1,foo1\xc3\xa4,deferred,,"Contrary, Mary","Bork, Chef;Contrary, Mary;demo"\r\n') + should_be=(b'"id","title","status","keyword","assignedto","nosy"\r\n' + b'"1","foo1\xc3\xa4","deferred","","Contrary, Mary","Bork, Chef;Contrary, Mary;demo"\r\n') self.assertEqual(output.getvalue(), should_be) output = io.BytesIO() @@ -1837,8 +1837,8 @@ # call export version that outputs id numbers actions.ExportCSVWithIdAction(cl).handle() print(output.getvalue()) - self.assertEqual(b'id,title,status,keyword,assignedto,nosy\r\n' - b"1,foo1\xc3\xa4,2,[],4,\"['3', '4', '5']\"\r\n", + self.assertEqual(b'"id","title","status","keyword","assignedto","nosy"\r\n' + b"\"1\",\"foo1\xc3\xa4\",\"2\",\"[]\",\"4\",\"['3', '4', '5']\"\r\n", output.getvalue()) # again with ISO-8859-1 client charset @@ -1848,8 +1848,8 @@ cl.request.wfile = output # call export version that outputs names actions.ExportCSVAction(cl).handle() - should_be=(b'id,title,status,keyword,assignedto,nosy\r\n' - b'1,foo1\xe4,deferred,,"Contrary, Mary","Bork, Chef;Contrary, Mary;demo"\r\n') + should_be=(b'"id","title","status","keyword","assignedto","nosy"\r\n' + b'"1","foo1\xe4","deferred","","Contrary, Mary","Bork, Chef;Contrary, Mary;demo"\r\n') self.assertEqual(output.getvalue(), should_be) output = io.BytesIO() @@ -1858,8 +1858,8 @@ # call export version that outputs id numbers actions.ExportCSVWithIdAction(cl).handle() print(output.getvalue()) - self.assertEqual(b'id,title,status,keyword,assignedto,nosy\r\n' - b"1,foo1\xe4,2,[],4,\"['3', '4', '5']\"\r\n", + self.assertEqual(b'"id","title","status","keyword","assignedto","nosy"\r\n' + b"\"1\",\"foo1\xe4\",\"2\",\"[]\",\"4\",\"['3', '4', '5']\"\r\n", output.getvalue()) def testCSVExportBadColumnName(self): @@ -1903,12 +1903,12 @@ actions.ExportCSVAction(cl).handle() #print(output.getvalue()) - self.assertEqual(s2b('id,username,address,password\r\n' - '1,admin,[hidden],[hidden]\r\n' - '2,anonymous,[hidden],[hidden]\r\n' - '3,Chef,[hidden],[hidden]\r\n' - '4,mary,[hidden],[hidden]\r\n' - '5,demo,demo@test.test,%s\r\n'%(passwd)), + self.assertEqual(s2b('"id","username","address","password"\r\n' + '"1","admin","[hidden]","[hidden]"\r\n' + '"2","anonymous","[hidden]","[hidden]"\r\n' + '"3","Chef","[hidden]","[hidden]"\r\n' + '"4","mary","[hidden]","[hidden]"\r\n' + '"5","demo","demo@test.test","%s"\r\n'%(passwd)), output.getvalue()) def testCSVExportWithId(self): @@ -1919,9 +1919,9 @@ cl.request = MockNull() cl.request.wfile = output actions.ExportCSVWithIdAction(cl).handle() - self.assertEqual(s2b('id,name\r\n1,unread\r\n2,deferred\r\n3,chatting\r\n' - '4,need-eg\r\n5,in-progress\r\n6,testing\r\n7,done-cbb\r\n' - '8,resolved\r\n'), + self.assertEqual(s2b('"id","name"\r\n"1","unread"\r\n"2","deferred"\r\n"3","chatting"\r\n' + '"4","need-eg"\r\n"5","in-progress"\r\n"6","testing"\r\n"7","done-cbb"\r\n' + '"8","resolved"\r\n'), output.getvalue()) def testCSVExportWithIdBadColumnName(self):
