Quote all exported CSV data

rouilj · rouilj · commit a5f8acc8e2b8 · 2020-06-08T16:18:21.000-04:00
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.
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -19,6 +19,12 @@ Fixed:
   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:
 
diff --git a/doc/upgrading.txt b/doc/upgrading.txt
@@ -160,6 +160,37 @@ or::
 
     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
 -------------------------------------------
 
diff --git a/roundup/cgi/actions.py b/roundup/cgi/actions.py
@@ -1438,7 +1438,7 @@ def handle(self):
                                        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 @@ def handle(self):
                                        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
diff --git a/test/test_cgi.py b/test/test_cgi.py
@@ -1792,25 +1792,25 @@ def testCSVExport(self):
         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 @@ def testCSVExportCharset(self):
         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 @@ def testCSVExportCharset(self):
         # 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 @@ def testCSVExportCharset(self):
         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 @@ def testCSVExportCharset(self):
         # 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 @@ def testCSVExportFailPermissionValidColumn(self):
 
         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 @@ def testCSVExportWithId(self):
         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):
