From dbbc64dbfee29bab9f5debf9da357a9f2ac6194c Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Thu, 22 Sep 2016 09:41:10 -0700 Subject: [PATCH] bug: Fix code that mis-identifies missed message formats. In our email mirror, we have a special format for missed message emails that uses a 32-bit randomly generated token that we put into redis that is then prefixed with "mm" for a total of 34 characters. We had a bug where we would mis-classify emails like mmcfoo@example.com as being these system-generated emails that were part of the redis setup. It's actually a little unclear how the bug in the library function would have manifested from the user's point of view, but it was definitely buggy code, and it's possibly related in a subtle way to an error report we got from a customer where only one of their users, who happened to have a name like mmcfoo, was having problems with the mirror. --- zerver/lib/email_mirror.py | 2 +- zerver/tests/test_email_mirror.py | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/zerver/lib/email_mirror.py b/zerver/lib/email_mirror.py index 63c0c1842b..8ccd36d6b7 100644 --- a/zerver/lib/email_mirror.py +++ b/zerver/lib/email_mirror.py @@ -90,7 +90,7 @@ def get_missed_message_token_from_address(address): # type: (text_type) -> text_type msg_string = get_email_gateway_message_string_from_address(address) - if not msg_string.startswith('mm') and len(msg_string) != 34: + if not is_mm_32_format(msg_string): raise ZulipEmailForwardError('Could not parse missed message address') # strip off the 'mm' before returning the redis key diff --git a/zerver/tests/test_email_mirror.py b/zerver/tests/test_email_mirror.py index eedbc04a46..b91413fb74 100644 --- a/zerver/tests/test_email_mirror.py +++ b/zerver/tests/test_email_mirror.py @@ -20,6 +20,7 @@ from zerver.lib.actions import ( from zerver.lib.email_mirror import ( process_message, process_stream_message, ZulipEmailForwardError, create_missed_message_address, + get_missed_message_token_from_address, ) from zerver.lib.digest import handle_digest_email @@ -38,11 +39,31 @@ import mock import os import sys from os.path import dirname, abspath +from six import text_type from six.moves import cStringIO as StringIO from django.conf import settings from typing import Any, Callable, Mapping, Union +class TestEmailMirrorLibrary(ZulipTestCase): + def test_get_missed_message_token(self): + # type: () -> None + + def get_token(address): + # type: (text_type) -> text_type + with self.settings(EMAIL_GATEWAY_PATTERN="%s@example.com"): + return get_missed_message_token_from_address(address) + + address = 'mm' + ('x' * 32) + '@example.com' + token = get_token(address) + self.assertEqual(token, 'x' * 32) + + # This next section was a bug at one point--we'd treat ordinary + # user addresses that happened to begin with "mm" as being + # the special mm+32chars tokens. + address = 'mmathers@example.com' + with self.assertRaises(ZulipEmailForwardError): + get_token(address) class TestStreamEmailMessagesSuccess(ZulipTestCase): def test_receive_stream_email_messages_success(self):