config option to limit nosy attachments based on size

reworking of patch [SF#772323] from Philipp Gortan

It tries to avoid reading the file contents just to
get the file size but that was too hard for metakit backends.
They don't inherit from blobfiles.FileStorage which makes
it more challenging. Really that backend should be reworked
to inherit from FileStorage.

I'm not sure I like the default being sys.maxint. Maybe have
0 == unlimited? But what if someone really wanted to set it to
0 to mean "don't attach anything"?

Author: Justus Pendleton

roundup/configuration.py

@@ -1,6 +1,6 @@
 # Roundup Issue Tracker configuration support
 #
-# $Id: configuration.py,v 1.46 2007-09-02 06:48:13 forsberg Exp $
+# $Id: configuration.py,v 1.47 2007-09-03 17:14:08 jpend Exp $
 #
 __docformat__ = "restructuredtext"
 
@@ -744,6 +744,10 @@ def str2value(self, value):
             "\"multiple\" then a separate email is sent to each\n"
             "recipient. If \"single\" then a single email is sent with\n"
             "each recipient as a CC address."),
+        (IntegerNumberOption, "max_attachment_size", sys.maxint,
+            "Attachments larger than the given number of bytes\n"
+            "won't be attached to nosy mails. They will be replaced by\n"
+            "a link to the tracker's download page for the file.")
     ), "Nosy messages sending"),
 )
 

roundup/roundupdb.py

@@ -16,14 +16,15 @@
 # BASIS, AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
 # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
 #
-# $Id: roundupdb.py,v 1.129 2007-09-01 16:30:11 forsberg Exp $
+# $Id: roundupdb.py,v 1.130 2007-09-03 17:14:08 jpend Exp $
 
 """Extending hyperdb with types specific to issue-tracking.
 """
 __docformat__ = 'restructuredtext'
 
 import re, os, smtplib, socket, time, random
 import cStringIO, base64, quopri, mimetypes
+import os.path
 
 from rfc2822 import encode_header
 
@@ -322,6 +323,29 @@ def send_message(self, nodeid, msgid, note, sendto, from_address=None,
         if msgid is not None:
             m.append(messages.get(msgid, 'content', ''))
 
+        # get the files for this message
+        message_files = []
+        if msgid :
+            for fileid in messages.get(msgid, 'files') :
+                # try to avoid reading in the file contents just to check the size
+                # backends that inherit from blobfiles.FileStorage have a filename class
+                if hasattr(self.db, 'filename'):
+                    filename = self.db.filename('file', fileid, None)
+                    filesize = os.path.getsize(filename)
+                else:
+                    # metakit doesn't inherit from FileStorage so we read the
+                    # full file contents to get the size :-/
+                    filesize = len(self.db.file.get(fileid, 'content'))
+
+                if filesize <= self.db.config.NOSY_MAX_ATTACHMENT_SIZE:
+                    message_files.append(fileid)
+                else:
+                    base = self.db.config.TRACKER_WEB
+                    link = "".join((base, files.classname, fileid))
+                    filename = files.get(fileid, 'name')
+                    m.append(_("File '%(filename)s' not attached - you can "
+                               "download it from %(link)s." % locals()))
+
         # add the change note
         if note:
             m.append(note)
@@ -340,12 +364,6 @@ def send_message(self, nodeid, msgid, note, sendto, from_address=None,
         quopri.encode(content, content_encoded, 0)
         content_encoded = content_encoded.getvalue()
 
-        # get the files for this message
-        if msgid is None:
-            message_files = None
-        else:
-            message_files = messages.get(msgid, 'files')
-
         # make sure the To line is always the same (for testing mostly)
         sendto.sort()
 

test/db_test_base.py

@@ -15,12 +15,13 @@
 # BASIS, AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
 # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
 #
-# $Id: db_test_base.py,v 1.88 2007-08-31 15:44:03 jpend Exp $
+# $Id: db_test_base.py,v 1.89 2007-09-03 17:14:09 jpend Exp $
 
-import unittest, os, shutil, errno, imp, sys, time, pprint, sets
+import unittest, os, shutil, errno, imp, sys, time, pprint, sets, base64
 
 from roundup.hyperdb import String, Password, Link, Multilink, Date, \
     Interval, DatabaseError, Boolean, Number, Node
+from roundup.mailer import Mailer
 from roundup import date, password, init, instance, configuration, support
 
 from mocknull import MockNull
@@ -69,8 +70,8 @@ def setupSchema(db, create, module):
     priority = module.Class(db, "priority", name=String(), order=String())
     priority.setkey("name")
     user = module.Class(db, "user", username=String(), password=Password(),
-        assignable=Boolean(), age=Number(), roles=String(),
-        supervisor=Link('user'))
+        assignable=Boolean(), age=Number(), roles=String(), address=String(),
+        supervisor=Link('user'),realname=String())
     user.setkey("username")
     file = module.FileClass(db, "file", name=String(), type=String(),
         comment=String(indexme="yes"), fooz=Password())
@@ -82,14 +83,18 @@ def setupSchema(db, create, module):
     stuff = module.Class(db, "stuff", stuff=String())
     session = module.Class(db, 'session', title=String())
     msg = module.FileClass(db, "msg", date=Date(),
-                           author=Link("user", do_journal='no'))
+                           author=Link("user", do_journal='no'),
+                           files=Multilink('file'), inreplyto=String(),
+                           messageid=String(),
+                           recipients=Multilink("user", do_journal='no')
+                           )
     session.disableJournalling()
     db.post_init()
     if create:
         user.create(username="admin", roles='Admin',
             password=password.Password('sekrit'))
         user.create(username="fred", roles='User',
-            password=password.Password('sekrit'))
+            password=password.Password('sekrit'), address='[email protected]')
         status.create(name="unread")
         status.create(name="in-progress")
         status.create(name="testing")
@@ -917,14 +922,15 @@ def testForcedReindexing(self):
 
     def testIndexingOnImport(self):
         msgcontent = 'Glrk'
-        msgid = self.db.msg.import_list(['content'], [repr(msgcontent)])
+        msgid = self.db.msg.import_list(['content', 'files', 'recipients'],
+                                        [repr(msgcontent), '[]', '[]'])
         msg_filename = self.db.filename(self.db.msg.classname, msgid,
                                         create=1)
         support.ensureParentsExist(msg_filename)
         msg_file = open(msg_filename, 'w')
         msg_file.write(msgcontent)
         msg_file.close()
-        
+
 
         filecontent = 'Brrk'
         fileid = self.db.file.import_list(['content'], [repr(filecontent)])
@@ -933,7 +939,7 @@ def testIndexingOnImport(self):
         support.ensureParentsExist(file_filename)
         file_file = open(file_filename, 'w')
         file_file.write(filecontent)
-        file_file.close()        
+        file_file.close()
 
         title = 'Bzzt'
         nodeid = self.db.issue.import_list(['title', 'messages', 'files',
@@ -958,7 +964,7 @@ def testIndexingOnImport(self):
         self.assertEquals(self.db.indexer.search([filecontent], self.db.issue),
                           {str(nodeid):{'files':[str(fileid)]}})
 
-        
+
 
     #
     # searching tests follow
@@ -1240,8 +1246,8 @@ def testFilteringStringSort(self):
 
     def testFilteringMultilinkSort(self):
         # 1: []                 Reverse:  1: []
-        # 2: []                           2: []              
-        # 3: ['admin','fred']             3: ['fred','admin']       
+        # 2: []                           2: []
+        # 3: ['admin','fred']             3: ['fred','admin']
         # 4: ['admin','bleep','fred']     4: ['fred','bleep','admin']
         # Note the sort order for the multilink doen't change when
         # reversing the sort direction due to the re-sorting of the
@@ -1532,7 +1538,7 @@ def testFilteringTransitiveMultilinkSort(self):
             ['1', '2', '3', '4', '5', '8', '6', '7'])
         ae(filt(None, {}, [('+','messages.author'), ('+','messages')]),
             ['6', '7', '8', '5', '4', '3', '1', '2'])
-        # The following will sort by 
+        # The following will sort by
         # author.supervisor.username and then by
         # author.username
         # I've resited the tempation to implement recursive orderprop
@@ -1723,6 +1729,38 @@ def testAddRemoveProperty(self):
             'nosy', 'priority', 'spam', 'status', 'superseder'])
         self.assertEqual(self.db.issue.list(), ['1'])
 
+    def testNosyMail(self) :
+        """Creates one issue with two attachments, one smaller and one larger
+           than the set max_attachment_size.
+        """
+        db = self.db
+        db.config.NOSY_MAX_ATTACHMENT_SIZE = 4096
+        res = dict(mail_to = None, mail_msg = None)
+        def dummy_snd(s, to, msg, res=res) :
+            res["mail_to"], res["mail_msg"] = to, msg
+        backup, Mailer.smtp_send = Mailer.smtp_send, dummy_snd
+        try :
+            f1 = db.file.create(name="test1.txt", content="x" * 20)
+            f2 = db.file.create(name="test2.txt", content="y" * 5000)
+            m  = db.msg.create(content="one two", author="admin",
+                files = [f1, f2])
+            i  = db.issue.create(title='spam', files = [f1, f2],
+                messages = [m], nosy = [db.user.lookup("fred")])
+
+            db.issue.nosymessage(i, m, {})
+            mail_msg = res["mail_msg"].getvalue()
+            self.assertEqual(res["mail_to"], ["[email protected]"])
+            self.failUnless("From: admin" in mail_msg)
+            self.failUnless("Subject: [issue1] spam" in mail_msg)
+            self.failUnless("New submission from admin" in mail_msg)
+            self.failUnless("one two" in mail_msg)
+            self.failIf("File 'test1.txt' not attached" in mail_msg)
+            self.failUnless(base64.b64encode("xxx") in mail_msg)
+            self.failUnless("File 'test2.txt' not attached" in mail_msg)
+            self.failIf(base64.b64encode("yyy") in mail_msg)
+        finally :
+            Mailer.smtp_send = backup
+
 class ROTest(MyTestCase):
     def setUp(self):
         # remove previous test, ignore errors