Skip to content

Commit 1b765d1

Browse files
committed
Fix subject parsing in mail gateway.
The previous parsing routine would not ensure that arguments are at the end of the subject and when subject_suffix_parsing was configured to be 'loose' it would truncate the subject when encountering a double prefix, e.g. Subject: [frobulated] [frobulatedagain] this part would be lost
1 parent 611c27d commit 1b765d1

File tree

3 files changed

+60
-19
lines changed

3 files changed

+60
-19
lines changed

CHANGES.txt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,12 @@ Fixed:
293293
of the tracker html directory. This has been fixed by normalizing
294294
the path and comparing to the normalized path for the html
295295
directory. See ``doc/upgrading.txt``. (John Rouillard)
296+
- Fix subject parsing in mail gateway. The previous parsing routine
297+
would not ensure that arguments are at the end of the subject and when
298+
subject_suffix_parsing was configured to be 'loose' it would truncate
299+
the subject when encountering a double prefix, e.g.
300+
Subject: [frobulated] [frobulatedagain] this part would be lost
301+
(Ralf Schlatterbeck)
296302

297303
2016-01-11: 1.5.1
298304

roundup/mailgw.py

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -652,33 +652,33 @@ def parse_subject(self):
652652

653653
tmpsubject = tmpsubject[m.end():]
654654

655-
# Match the title of the subject
656-
# if we've not found a valid classname prefix then force the
657-
# scanning to handle there being a leading delimiter
658-
title_re = r'(?P<title>%s[^%s]*)'%(
659-
not self.matches['classname'] and '.' or '', delim_open)
660-
m = re.match(title_re, tmpsubject.strip(), re.IGNORECASE)
655+
# Match any arguments specified *from the end*
656+
# Optionally match and strip quote at the end that dumb mailers
657+
# may put there, e.g.
658+
# Re: "[issue1] bla blah [<args>]"
659+
q = ''
660+
if self.matches['quote']:
661+
q = '"?'
662+
args_re = r'(?P<argswhole>%s(?P<args>[^%s]*)%s)%s$'%(delim_open,
663+
delim_close, delim_close, q)
664+
m = re.search(args_re, tmpsubject.strip(), re.IGNORECASE|re.VERBOSE)
661665
if m:
662666
self.matches.update(m.groupdict())
663-
tmpsubject = tmpsubject[len(self.matches['title']):] # Consume title
664-
665-
if self.matches['title']:
666-
self.matches['title'] = self.matches['title'].strip()
667+
tmpsubject = tmpsubject [:m.start()]
667668
else:
668-
self.matches['title'] = ''
669+
self.matches['argswhole'] = self.matches['args'] = None
670+
671+
# The title of the subject is the remaining tmpsubject.
672+
self.matches ['title'] = tmpsubject.strip ()
669673

670674
# strip off the quotes that dumb emailers put around the subject, like
671675
# Re: "[issue1] bla blah"
672-
if self.matches['quote'] and self.matches['title'].endswith('"'):
676+
# but only if we didn't match arguments at the end (which would
677+
# already have consumed the quote after the subject)
678+
if self.matches['quote'] and not self.matches['argswhole'] \
679+
and self.matches['title'].endswith('"'):
673680
self.matches['title'] = self.matches['title'][:-1]
674681

675-
# Match any arguments specified
676-
args_re = r'(?P<argswhole>%s(?P<args>.+?)%s)?'%(delim_open,
677-
delim_close)
678-
m = re.search(args_re, tmpsubject.strip(), re.IGNORECASE|re.VERBOSE)
679-
if m:
680-
self.matches.update(m.groupdict())
681-
682682
def rego_confirm(self):
683683
''' Check for registration OTK and confirm the registration if found
684684
'''

test/test_mailgw.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3054,6 +3054,24 @@ def testInvalidClassLooseReply(self):
30543054
30553055
Message-Id: <dummy_test_message_id>
30563056
3057+
''')
3058+
assert not os.path.exists(SENDMAILDEBUG)
3059+
self.assertEqual(self.db.issue.get(nodeid, 'title'),
3060+
'[frobulated] testing')
3061+
3062+
def testInvalidClassLooseReplyQuoted(self):
3063+
self.instance.config.MAILGW_SUBJECT_PREFIX_PARSING = 'loose'
3064+
nodeid = self._handle_mail('''Content-Type: text/plain;
3065+
charset="iso-8859-1"
3066+
From: Chef <[email protected]>
3067+
3068+
Subject: Re: "[frobulated] testing"
3069+
3070+
3071+
Message-Id: <dummy_test_message_id>
3072+
3073+
Dumb mailers may put quotes around the subject after the reply prefix,
3074+
e.g. Re: "[issue1] bla bla"
30573075
''')
30583076
assert not os.path.exists(SENDMAILDEBUG)
30593077
self.assertEqual(self.db.issue.get(nodeid, 'title'),
@@ -3091,6 +3109,23 @@ def testClassLooseOK(self):
30913109
assert not os.path.exists(SENDMAILDEBUG)
30923110
self.assertEqual(self.db.keyword.get('1', 'name'), 'Bar')
30933111

3112+
def testDoublePrefixLoose(self):
3113+
self.instance.config.MAILGW_SUBJECT_PREFIX_PARSING = 'loose'
3114+
self.instance.config.MAILGW_SUBJECT_SUFFIX_PARSING = 'loose'
3115+
nodeid = self._handle_mail('''Content-Type: text/plain;
3116+
charset="iso-8859-1"
3117+
From: Chef <[email protected]>
3118+
3119+
Subject: [frobulated] [frobulatedagain] testing stuff after double prefix
3120+
3121+
3122+
Message-Id: <dummy_test_message_id>
3123+
3124+
''')
3125+
assert not os.path.exists(SENDMAILDEBUG)
3126+
self.assertEqual(self.db.issue.get(nodeid, 'title'),
3127+
'[frobulated] [frobulatedagain] testing stuff after double prefix')
3128+
30943129
def testClassStrictInvalid(self):
30953130
self.instance.config.MAILGW_SUBJECT_PREFIX_PARSING = 'strict'
30963131
self.instance.config.MAILGW_DEFAULT_CLASS = ''

0 commit comments

Comments
 (0)