Fixed incorrect header comparisons in compareMessages. It iterated

over every header in the new message and didn't detect when the new
message was missing a header that was in the reference message.

I have fixed that by retrieving all header names from the new and
reference (old) messages, lower casing the list and comparing them
(with a fixup for the x-roundup-version header). If the list of
headers isn't identical, I throw an error.

Also the Content-Type mime values are now being compared as well to
make sure the new message being generates matches the reference mime
content type.

Of course this broke some tests. A number of them were using
compareMessages when they should have been using assertEqual. That was
an easy fix.

Also some messages when retrieved from the content property are
missing trailing newlines. I think this is part of the signature
removal algorithm. This may be a bug or intended. In any case I am
fixing the tests to remove trailing newlines.

However I have a handful of tests that are not that obvious.
Two of are

   test_mailgw.py:testMultipartCharsetLatin1AttachFile
   test_mailgw.py:testMultipartCharsetUTF8AttachFile

I removed a:

   Content-Transfer-Encoding: quoted-printable

message header from the reference message in both cases. The message
content-type was multipart/mixed and it had no content that was
outside of a mime part. Sending a message like this using mailx and
nmh resulted in an email being delivered that was missing a
Content-Transfer-Encoding, so I think I am ok here.

The last broken test also started as a missing
Content-Transfer-Encoding header. But there was a twist:

  test-mailgw.py:testMultipartRFC822

had a reference copy with a mime type of text/plain which requires a
Content-Transfer-Encoding. However I never looked at the mime type of
the mail being generated. The generated mail was of type
multipart/mixed.  So I changed the reference copy to remove the
Content-Transfer-Encoding and changed the reference mime type to
multipart/mixed.

Now all the mailgw tests are passing. With luck I haven't encoded an
invalid test.

Also did one minor spelling correction.

Author: rouilj

test/test_mailgw.py

@@ -64,6 +64,8 @@ class DiffHelper:
     def compareMessages(self, new, old):
         """Compare messages for semantic equivalence.
 
+        Only use this for full rfc 2822/822/whatever messages with headers.
+
         Will raise an AssertionError with a diff for inequality.
 
         Note that header fieldnames are case-insensitive.
@@ -85,6 +87,28 @@ def compareMessages(self, new, old):
             res = []
 
             replace = {}
+
+            # make sure that all headers are the same between the new
+            # and old (reference) messages. Once we have done this we
+            # can iterate over all the headers in the new message and
+            # compare contents. If we don't do this, we don't know
+            # when the new message has dropped a header that should be
+            # present.
+            # Headers are case insensitive, so smash to lower case
+            new_headers=[x.lower() for x in new.keys()]
+            old_headers=[x.lower() for x in old.keys()]
+
+            if "x-roundup-version" not in old_headers:
+                # add it. it is skipped in most cases and missing from
+                # the test cases in old.
+                old_headers.append("x-roundup-version")
+                
+            new_headers.sort() # sort, make comparison easier in error message.
+            old_headers.sort()
+
+            if new_headers != old_headers:
+                res.append('headers differ new vs. reference: %r != %r'%(new_headers, old_headers))
+                
             for key in new.keys():
                 if key.startswith('from '):
                     # skip the unix from line
@@ -96,10 +120,16 @@ def compareMessages(self, new, old):
                             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 + '--'
+                    newmimeboundary = new[key].split('=',1)[-1].strip('"')
+                    oldmimeboundary = old.get(key, '').split('=',1)[-1].strip('"')
+                    # mime types are not case sensitive rfc 2045
+                    newmimetype = new[key].split(';',1)[0].strip('"').lower()
+                    oldmimetype = old.get(key, '').split(';',1)[0].strip('"').lower()
+                    # throw an error if we have differeing content types
+                    if not newmimetype == oldmimetype:
+                        res.append('content-type mime type headers differ new vs. reference: %r != %r'%(newmimetype, oldmimetype))
+                    replace ['--' + newmimeboundary] = '--' + oldmimeboundary
+                    replace ['--' + newmimeboundary + '--'] = '--' + oldmimeboundary + '--'
                 elif new.get_all(key, '') != old.get_all(key, ''):
                     # check that all other headers are identical, including
                     # headers that appear more than once.
@@ -911,7 +941,6 @@ def testMultipartCharsetUTF8AttachFile(self):
 X-Roundup-Loop: hello
 X-Roundup-Issue-Status: chatting
 X-Roundup-Issue-Files: unnamed
-Content-Transfer-Encoding: quoted-printable
 
 
 --utf-8
@@ -975,7 +1004,6 @@ def testMultipartCharsetLatin1AttachFile(self):
 X-Roundup-Loop: hello
 X-Roundup-Issue-Status: chatting
 X-Roundup-Issue-Files: unnamed
-Content-Transfer-Encoding: quoted-printable
 
 
 --utf-8
@@ -1022,7 +1050,7 @@ def testMultipartRFC822(self):
         self.compareMessages(self._get_mail(),
 '''FROM: [email protected]
 TO: [email protected], [email protected]
-Content-Type: text/plain; charset="utf-8"
+Content-Type: multipart/mixed; charset="utf-8"
 Subject: [issue1] Testing...
 To: [email protected], [email protected]
 From: "Contrary, Mary" <[email protected]>
@@ -1035,7 +1063,6 @@ def testMultipartRFC822(self):
 X-Roundup-Loop: hello
 X-Roundup-Issue-Status: chatting
 X-Roundup-Issue-Files: Fwd: Original email subject.eml
-Content-Transfer-Encoding: quoted-printable
 
 
 --utf-8
@@ -2120,14 +2147,14 @@ def testNewUserAuthor(self):
         try:
             self._handle_mail(message)
         except Unauthorized, value:
-            body_diff = self.compareMessages(str(value), """
+            body_diff = self.assertEqual(str(value), """
 You are not a registered user.
 
 Unknown address: [email protected]
 """)
             assert not body_diff, body_diff
         else:
-            raise AssertionError, "Unathorized not raised when handling mail"
+            raise AssertionError, "Unauthorized not raised when handling mail"
 
         # Add Web Access role to anonymous, and try again to make sure
         # we get a "please register at:" message this time.
@@ -2139,7 +2166,7 @@ def testNewUserAuthor(self):
         try:
             self._handle_mail(message)
         except Unauthorized, value:
-            body_diff = self.compareMessages(str(value), """
+            body_diff = self.assertEqual(str(value), """
 You are not a registered user. Please register at:
 
 http://tracker.example/cgi-bin/roundup.cgi/bugs/user?@template=register
@@ -2544,8 +2571,10 @@ def testFollowupStupidQuoting(self):
 
     def testEmailQuoting(self):
         self.instance.config.EMAIL_KEEP_QUOTED_TEXT = 'no'
-        self.innerTestQuoting(self.firstquotingtest, '''This is a followup
-''', 'This is a followup')
+        # FIXME possible bug. Messages retreived from content are missing
+        # trailing newlines. Probably due to signature stripping.
+        # so nuke all trailing newlines.
+        self.innerTestQuoting(self.firstquotingtest, '''This is a followup''', 'This is a followup')
 
     def testEmailQuotingNewIsNew(self):
         self.instance.config.EMAIL_KEEP_QUOTED_TEXT = 'new'
@@ -2618,13 +2647,15 @@ def testEmailReplaceBodyNewIsFollowup(self):
 
     def testEmailQuotingRemove(self):
         self.instance.config.EMAIL_KEEP_QUOTED_TEXT = 'yes'
+        # FIXME possible bug. Messages retreived from content are missing
+        # trailing newlines. Probably due to signature stripping.
+        # so nuke all trailing newlines.
         self.innerTestQuoting(self.firstquotingtest, '''Blah blah wrote:
 > Blah bklaskdfj sdf asdf jlaskdf skj sdkfjl asdf
 >  skdjlkjsdfalsdkfjasdlfkj dlfksdfalksd fj
 >
 
-This is a followup
-''', 'This is a followup')
+This is a followup''', 'This is a followup')
 
     secondquotingtest = '''Content-Type: text/plain;
   charset="iso-8859-1"
@@ -2674,6 +2705,9 @@ def testEmailQuotingRemove(self):
 '''
     def testEmailQuoting2(self):
         self.instance.config.EMAIL_KEEP_QUOTED_TEXT = 'no'
+        # FIXME possible bug. Messages retreived from content are missing
+        # trailing newlines. Probably due to signature stripping.
+        # so nuke all trailing newlines.
         self.innerTestQuoting(self.secondquotingtest, '''AA:
 
  AA
@@ -2689,13 +2723,16 @@ def testEmailQuoting2(self):
 BB
 BB
 
-CC
-''', 'AA:')
+CC''', 'AA:')
 
     def testEmailQuotingRemove2(self):
         self.instance.config.EMAIL_KEEP_QUOTED_TEXT = 'yes'
+        # FIXME possible bug. Messages retreived from content are missing
+        # trailing newlines. Probably due to signature stripping.
+        # so nuke all trailing newlines. That's what the trailing
+        # [:-1] is doing on the '\n'.join(....)[8:-3]
         self.innerTestQuoting(self.secondquotingtest,
-            '\n'.join(self.secondquotingtest.split('\n')[8:-3]), 'AA:')
+            '\n'.join(self.secondquotingtest.split('\n')[8:-3][:-1]), 'AA:')
 
     thirdquotingtest = '''Content-Type: text/plain;
   charset="iso-8859-1"
@@ -2769,7 +2806,7 @@ def innerTestQuoting(self, msgtext, expect, summary=None):
             newmessages.remove(msg)
         messageid = newmessages[0]
 
-        self.compareMessages(self.db.msg.get(messageid, 'content'), expect)
+        self.assertEqual(self.db.msg.get(messageid, 'content'), expect)
         if summary:
             self.assertEqual (summary, self.db.msg.get(messageid, 'summary'))