From 127b6984e96ba5d682c3de5c085034765f600c96 Mon Sep 17 00:00:00 2001 From: schlatterbeck Date: Wed, 15 Jul 2009 12:22:35 +0000 Subject: [PATCH] fix problem with bounce-message if incoming mail has insufficient privilege, e.g., user not existing (issue 2550534) Added a regression test for this issue that reproduces the traceback reported in issue 2550534 I'm using a slightly modified variant of the original patch that avoids repeated string-concatenation (which could degenerate to quadratic runtime behaviour for large number of email headers). git-svn-id: http://svn.roundup-tracker.org/svnroot/roundup/roundup/trunk@4319 57a73879-2fb5-44c3-a270-3262357dd7e2 --- CHANGES.txt | 2 ++ roundup/mailer.py | 21 +++++++------ test/test_mailgw.py | 75 +++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 85 insertions(+), 13 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 96fc151..1b37418 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -10,6 +10,8 @@ Fixes: - fix problems with french and german locale files (issue 2550546) - Run each message of the mail-gateway in a separate transaction, see http://thread.gmane.org/gmane.comp.bug-tracking.roundup.user/9500 +- fix problem with bounce-message if incoming mail has insufficient + privilege, e.g., user not existing (issue 2550534) 2009-03-18 1.4.8 (r4209) diff --git a/roundup/mailer.py b/roundup/mailer.py index 4ab9b13..f0981aa 100644 --- a/roundup/mailer.py +++ b/roundup/mailer.py @@ -140,24 +140,25 @@ class Mailer: elif error_messages_to == "both": to.append(dispatcher_email) - message = self.get_standard_message(to, subject) + message = self.get_standard_message(to, subject, multipart=True) # add the error text - part = MIMEText(error) + part = MIMEText('\n'.join(error)) message.attach(part) # attach the original message to the returned message + body = [] + for header in bounced_message.headers: + body.append(header) try: bounced_message.rewindbody() - except IOError, message: - body.write("*** couldn't include message body: %s ***" - % bounced_message) + except IOError, errmessage: + body.append("*** couldn't include message body: %s ***" % + errmessage) else: - body.write(bounced_message.fp.read()) - part = MIMEText(bounced_message.fp.read()) - part['Content-Disposition'] = 'attachment' - for header in bounced_message.headers: - part.write(header) + body.append('\n') + body.append(bounced_message.fp.read()) + part = MIMEText(''.join(body)) message.attach(part) # send diff --git a/test/test_mailgw.py b/test/test_mailgw.py index 60d1164..749a93c 100644 --- a/test/test_mailgw.py +++ b/test/test_mailgw.py @@ -51,6 +51,7 @@ class DiffHelper: if not new == old: res = [] + replace = {} for key in new.keys(): if key.startswith('from '): # skip the unix from line @@ -60,11 +61,18 @@ class DiffHelper: if new[key] != __version__: res.append(' %s: %r != %r' % (key, __version__, new[key])) + elif key.lower() == 'content-type' and 'boundary=' in new[key]: + # handle mime messages + newmime = new[key].split('=',1)[-1].strip('"') + oldmime = old.get(key, '').split('=',1)[-1].strip('"') + replace ['--' + newmime] = '--' + oldmime + replace ['--' + newmime + '--'] = '--' + oldmime + '--' elif new.get(key, '') != old.get(key, ''): res.append(' %s: %r != %r' % (key, old.get(key, ''), new.get(key, ''))) - body_diff = self.compareStrings(new.fp.read(), old.fp.read()) + body_diff = self.compareStrings(new.fp.read(), old.fp.read(), + replace=replace) if body_diff: res.append('') res.extend(body_diff) @@ -73,13 +81,14 @@ class DiffHelper: res.insert(0, 'Generated message not correct (diff follows):') raise AssertionError, '\n'.join(res) - def compareStrings(self, s2, s1): + def compareStrings(self, s2, s1, replace={}): '''Note the reversal of s2 and s1 - difflib.SequenceMatcher wants the first to be the "original" but in the calls in this file, the second arg is the original. Ho hum. + Do replacements over the replace dict -- used for mime boundary ''' l1 = s1.strip().split('\n') - l2 = s2.strip().split('\n') + l2 = [replace.get(i,i) for i in s2.strip().split('\n')] if l1 == l2: return s = difflib.SequenceMatcher(None, l1, l2) @@ -1105,6 +1114,66 @@ This is a test submission of a new issue. name = self.db.user.get(new, 'realname') self.assertEquals(name, 'H€llo') + def testUnknownUser(self): + l = set(self.db.user.list()) + message = '''Content-Type: text/plain; + charset="iso-8859-1" +From: Nonexisting User +To: issue_tracker@your.tracker.email.domain.example +Message-Id: +Subject: [issue] Testing nonexisting user... + +This is a test submission of a new issue. +''' + self.db.close() + handler = self.instance.MailGW(self.instance) + # we want a bounce message: + handler.trapExceptions = 1 + ret = handler.main(StringIO(message)) + self.compareMessages(self._get_mail(), +'''FROM: Roundup issue tracker +TO: nonexisting@bork.bork.bork +From nobody Tue Jul 14 12:04:11 2009 +Content-Type: multipart/mixed; boundary="===============0639262320==" +MIME-Version: 1.0 +Subject: Failed issue tracker submission +To: nonexisting@bork.bork.bork +From: Roundup issue tracker +Date: Tue, 14 Jul 2009 12:04:11 +0000 +Precedence: bulk +X-Roundup-Name: Roundup issue tracker +X-Roundup-Loop: hello +X-Roundup-Version: 1.4.8 +MIME-Version: 1.0 + +--===============0639262320== +Content-Type: text/plain; charset="us-ascii" +MIME-Version: 1.0 +Content-Transfer-Encoding: 7bit + + + +You are not a registered user. + +Unknown address: nonexisting@bork.bork.bork + +--===============0639262320== +Content-Type: text/plain; charset="us-ascii" +MIME-Version: 1.0 +Content-Transfer-Encoding: 7bit + +Content-Type: text/plain; + charset="iso-8859-1" +From: Nonexisting User +To: issue_tracker@your.tracker.email.domain.example +Message-Id: +Subject: [issue] Testing nonexisting user... + +This is a test submission of a new issue. + +--===============0639262320==-- +''') + def testEnc01(self): self.doNewIssue() self._handle_mail('''Content-Type: text/plain; -- 2.30.2