Merge lp:~mattpalmer/dkimpy/dkimpy into lp:dkimpy

[Matthew Palmer]

Resolve canonicalization of empty bodies in relaxed mode - Bug #1727319

[Scott Kitterman (kitterman)]

I agree with your analysis of the issue. I'd be glad to merge this, but unfortunately the tests fail in python3.4 (only python3 I tested, but I suspect it wouldn't matter). This needs to work with python3, so please update and resubmit. I suspect it's just test definition errors, but I haven't had time to check.

review: Needs Fixing

[Scott Kitterman (kitterman)]

Fix turned out to be simple (b"" vice "" in correct_empty_body).

=== modified file 'dkim/canonicalization.py'
--- dkim/canonicalization.py	2011-06-04 05:51:14 +0000
+++ dkim/canonicalization.py	2017-10-25 13:33:59 +0000
@@ -48,6 +48,13 @@
     return re.sub(b"\r\n", b"", content)
 
 
+def correct_empty_body(content):
+    if content == b"\r\n":
+        return ""
+    else:
+        return content
+
+
 class Simple:
     """Class that represents the "simple" canonicalization algorithm."""
 
@@ -85,8 +92,8 @@
         # Remove all trailing WSP at end of lines.
         # Compress non-line-ending WSP to single space.
         # Ignore all empty lines at the end of the message body.
-        return strip_trailing_lines(
-            compress_whitespace(strip_trailing_whitespace(body)))
+        return correct_empty_body(strip_trailing_lines(
+            compress_whitespace(strip_trailing_whitespace(body))))
 
 
 class CanonicalizationPolicy:

=== modified file 'dkim/tests/test_canonicalization.py'
--- dkim/tests/test_canonicalization.py	2011-06-04 05:51:14 +0000
+++ dkim/tests/test_canonicalization.py	2017-10-25 13:33:59 +0000
@@ -53,6 +53,26 @@
             b'Foo  \tbar    \r\n',
             b'Foo  \tbar    \r\n\r\n')
 
+    def test_adds_crlf(self):
+        self.assertCanonicalForm(
+            b'Foo bar\r\n',
+            b'Foo bar')
+
+    def test_empty_body(self):
+        self.assertCanonicalForm(
+            b'\r\n',
+            b'')
+
+    def test_single_crlf_body(self):
+        self.assertCanonicalForm(
+            b'\r\n',
+            b'\r\n')
+
+    def test_multiple_crlf_body(self):
+        self.assertCanonicalForm(
+            b'\r\n',
+            b'\r\n\r\n')
+
 
 class TestRelaxedAlgorithmHeaders(BaseCanonicalizationTest):
 
@@ -98,6 +118,26 @@
             b'Foo\r\nbar\r\n',
             b'Foo\r\nbar\r\n\r\n\r\n')
 
+    def test_adds_crlf(self):
+        self.assertCanonicalForm(
+            b'Foo bar\r\n',
+            b'Foo bar')
+
+    def test_empty_body(self):
+        self.assertCanonicalForm(
+            b'',
+            b'')
+
+    def test_single_crlf_body(self):
+        self.assertCanonicalForm(
+            b'',
+            b'\r\n')
+
+    def test_multiple_crlf_body(self):
+        self.assertCanonicalForm(
+            b'',
+            b'\r\n\r\n')
+
 
 class TestCanonicalizationPolicyFromCValue(unittest.TestCase):