From 4ecb16b351f39acf8386cdca08be79ae860a7c3e Mon Sep 17 00:00:00 2001
From: Clemens Wolff <clemens@ascoderu.ca>
Date: Mon, 27 May 2019 01:04:20 -0400
Subject: [PATCH] Handle exceptions when inlining images (#198)

---
 opwen_email_server/actions.py                 |  5 +--
 opwen_email_server/utils/email_parser.py      | 13 +++++--
 .../utils/test_email_parser.py                | 38 +++++++++++++++----
 3 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/opwen_email_server/actions.py b/opwen_email_server/actions.py
index 2f763c2a..e6e8d676 100644
--- a/opwen_email_server/actions.py
+++ b/opwen_email_server/actions.py
@@ -97,11 +97,10 @@ def _store_inbound_email(self, email_id: str, email: dict):
             pending_storage = self._pending_factory(domain)
             pending_storage.store_text(email_id, 'pending')
 
-    @classmethod
-    def _parse_mime_email(cls, mime_email: str) -> dict:
+    def _parse_mime_email(self, mime_email: str) -> dict:
         email = parse_mime_email(mime_email)
         email = format_attachments(email)
-        email = format_inline_images(email)
+        email = format_inline_images(email, self.log_warning)
         return email
 
 
diff --git a/opwen_email_server/utils/email_parser.py b/opwen_email_server/utils/email_parser.py
index 2f26e0a3..8b3441df 100644
--- a/opwen_email_server/utils/email_parser.py
+++ b/opwen_email_server/utils/email_parser.py
@@ -6,6 +6,7 @@
 from io import BytesIO
 from itertools import chain
 from mimetypes import guess_type
+from typing import Callable
 from typing import Iterable
 from typing import List
 from typing import Optional
@@ -192,7 +193,7 @@ def _is_valid_url(url: Optional[str]) -> bool:
     return has_http_prefix or has_https_prefix
 
 
-def format_inline_images(email: dict) -> dict:
+def format_inline_images(email: dict, on_error: Callable) -> dict:
     email_body = email.get('body', '')
     if not email_body:
         return email
@@ -208,9 +209,13 @@ def format_inline_images(email: dict) -> dict:
         if not _is_valid_url(image_url):
             continue
 
-        encoded_image = _fetch_image_to_base64(image_url)
-        if encoded_image:
-            image_tag['src'] = encoded_image
+        try:
+            encoded_image = _fetch_image_to_base64(image_url)
+        except Exception as ex:
+            on_error('Unable to inline image %s: %s', image_url, ex)
+        else:
+            if encoded_image:
+                image_tag['src'] = encoded_image
 
     new_email = dict(email)
     new_email['body'] = str(soup)
diff --git a/tests/opwen_email_server/utils/test_email_parser.py b/tests/opwen_email_server/utils/test_email_parser.py
index c3b283ff..ff42292f 100644
--- a/tests/opwen_email_server/utils/test_email_parser.py
+++ b/tests/opwen_email_server/utils/test_email_parser.py
@@ -4,6 +4,7 @@
 from os.path import dirname
 from os.path import join
 from unittest import TestCase
+from unittest.mock import patch
 
 from responses import mock
 
@@ -124,21 +125,41 @@ def test_format_inline_images_with_img_tag(self):
         self.givenTestImage()
         input_email = {'body': '<div><h3>test image</h3><img src="http://test-url.png"/></div>'}
 
-        output_email = email_parser.format_inline_images(input_email)
+        output_email = email_parser.format_inline_images(input_email, self.fail_if_called)
 
         self.assertStartsWith(output_email['body'], '<div><h3>test image</h3><img src="data:image/png;')
 
+    @mock.activate
+    @patch.object(email_parser, 'Image')
+    def test_handles_exceptions_when_processing_image(self, mock_pil):
+        def throw():
+            raise IOError()
+
+        mock_pil.open.side_effect = throw
+        handled_errors = []
+
+        def on_error(*args):
+            handled_errors.append(args)
+
+        self.givenTestImage()
+        input_email = {'body': '<div><h3>test image</h3><img src="http://test-url.png"/></div>'}
+
+        output_email = email_parser.format_inline_images(input_email, on_error)
+
+        self.assertEqual(len(handled_errors), 1)
+        self.assertEqual(output_email, input_email)
+
     def test_format_inline_images_with_img_tag_without_src_attribute(self):
         input_email = {'body': '<div><img/></div>'}
 
-        output_email = email_parser.format_inline_images(input_email)
+        output_email = email_parser.format_inline_images(input_email, self.fail_if_called)
 
         self.assertEqual(output_email, input_email)
 
     def test_format_inline_images_with_img_tag_and_invalid_src_attribute(self):
         input_email = {'body': '<div><img src="foo:invalid"/></div>'}
 
-        output_email = email_parser.format_inline_images(input_email)
+        output_email = email_parser.format_inline_images(input_email, self.fail_if_called)
 
         self.assertEqual(output_email, input_email)
 
@@ -147,7 +168,7 @@ def test_format_inline_images_with_bad_request(self):
         self.givenTestImage(status=404)
         input_email = {'body': '<div><img src="http://test-url.png"/></div>'}
 
-        output_email = email_parser.format_inline_images(input_email)
+        output_email = email_parser.format_inline_images(input_email, self.fail_if_called)
 
         self.assertEqual(output_email, input_email)
 
@@ -156,14 +177,14 @@ def test_format_inline_images_with_many_img_tags(self):
         self.givenTestImage()
         input_email = {'body': '<div><img src="http://test-url.png"/><img src="http://test-url.png"/></div>'}
 
-        output_email = email_parser.format_inline_images(input_email)
+        output_email = email_parser.format_inline_images(input_email, self.fail_if_called)
 
         self.assertHasCount(output_email['body'], 'src="data:', 2)
 
     def test_format_inline_images_without_img_tags(self):
         input_email = {'body': '<div></div>'}
 
-        output_email = email_parser.format_inline_images(input_email)
+        output_email = email_parser.format_inline_images(input_email, self.fail_if_called)
 
         self.assertEqual(output_email, input_email)
 
@@ -172,7 +193,7 @@ def test_format_inline_images_without_content_type(self):
         self.givenTestImage(content_type='')
         input_email = {'body': '<div><img src="http://test-url.png"/></div>'}
 
-        output_email = email_parser.format_inline_images(input_email)
+        output_email = email_parser.format_inline_images(input_email, self.fail_if_called)
 
         self.assertStartsWith(output_email['body'], '<div><img src="data:image/png;')
 
@@ -194,6 +215,9 @@ def givenTestImage(cls, content_type='image/png', status=200):
                  body=image_bytes,
                  status=status)
 
+    def fail_if_called(self, message, *args):
+        self.fail(message % args)
+
 
 class FormatAttachedFilesTests(TestCase):
     def test_format_attachments_without_attachment(self):