messages: Check for empty recipient lists in Addressee.

The check for empty recipient lists (the "message_to" argument)
does not belong in check_message. As we implement support for
sending messages by user IDs (see #9474), we will be extending
much of the existing code in Addressee.legacy_build that validates
recipient lists. Therefore, Addressee.legacy_build is a much more
apt area to check for empty recipient lists.

Also, Addressee.for_private and Addressee.for_user_ids also need
to do their own validation, since not everything goes through
Addressee.legacy_build. It is okay to simply throw a 500 in these
cases because we expect that callers will be doing their own
validation for calls that don't go through Addressee.legacy_build().

This commit is a part of our efforts surrounding #9474.
This commit is contained in:
Eeshan Garg
2018-08-24 19:54:46 -02:30
parent 2f634f8c06
commit b3e8e36a8d
2 changed files with 5 additions and 4 deletions

View File

@@ -2167,10 +2167,6 @@ def check_message(sender: UserProfile, client: Client, addressee: Addressee,
elif addressee.is_private(): elif addressee.is_private():
user_profiles = addressee.user_profiles() user_profiles = addressee.user_profiles()
if user_profiles is None or len(user_profiles) == 0:
raise JsonableError(_("Message must have recipients"))
mirror_message = client and client.name in ["zephyr_mirror", "irc_mirror", mirror_message = client and client.name in ["zephyr_mirror", "irc_mirror",
"jabber_mirror", "JabberMirror"] "jabber_mirror", "JabberMirror"]
not_forged_mirror_message = mirror_message and not forged not_forged_mirror_message = mirror_message and not forged

View File

@@ -115,6 +115,9 @@ class Addressee:
return Addressee.for_stream(stream_name, topic_name) return Addressee.for_stream(stream_name, topic_name)
elif message_type_name == 'private': elif message_type_name == 'private':
if not message_to:
raise JsonableError(_("Message must have recipients"))
emails = message_to emails = message_to
return Addressee.for_private(emails, realm) return Addressee.for_private(emails, realm)
else: else:
@@ -135,6 +138,7 @@ class Addressee:
@staticmethod @staticmethod
def for_private(emails: Sequence[str], realm: Realm) -> 'Addressee': def for_private(emails: Sequence[str], realm: Realm) -> 'Addressee':
assert len(emails) > 0
user_profiles = get_user_profiles(emails, realm) user_profiles = get_user_profiles(emails, realm)
return Addressee( return Addressee(
msg_type='private', msg_type='private',
@@ -143,6 +147,7 @@ class Addressee:
@staticmethod @staticmethod
def for_user_ids(user_ids: Sequence[int], realm: Realm) -> 'Addressee': def for_user_ids(user_ids: Sequence[int], realm: Realm) -> 'Addressee':
assert len(user_ids) > 0
user_profiles = get_user_profiles_by_ids(user_ids, realm) user_profiles = get_user_profiles_by_ids(user_ids, realm)
return Addressee( return Addressee(
msg_type='private', msg_type='private',