streams: Consider stream name validation logic.

This commit is contained in:
Tim Abbott
2017-01-29 22:01:19 -08:00
parent d14037c82e
commit 1bbf0f9a98
5 changed files with 18 additions and 33 deletions

View File

@@ -26,7 +26,7 @@ from zerver.lib.message import (
render_markdown,
)
from zerver.models import Realm, RealmEmoji, Stream, UserProfile, UserActivity, RealmAlias, \
Subscription, Recipient, Message, Attachment, UserMessage, valid_stream_name, \
Subscription, Recipient, Message, Attachment, UserMessage, \
Client, DefaultStream, UserPresence, Referral, PushDeviceToken, MAX_SUBJECT_LENGTH, \
MAX_MESSAGE_LENGTH, get_client, get_stream, get_recipient, get_huddle, \
get_user_profile_by_id, PreregistrationUser, get_display_recipient, \
@@ -1186,12 +1186,10 @@ def check_send_message(sender, client, message_type_name, message_to,
def check_stream_name(stream_name):
# type: (Text) -> None
if stream_name == "":
raise JsonableError(_("Stream can't be empty"))
if stream_name.strip() == "":
raise JsonableError(_("Invalid stream name '%s'" % (stream_name)))
if len(stream_name) > Stream.MAX_NAME_LENGTH:
raise JsonableError(_("Stream name too long"))
if not valid_stream_name(stream_name):
raise JsonableError(_("Invalid stream name"))
raise JsonableError(_("Stream name too long (limit: %s characters)" % (Stream.MAX_NAME_LENGTH)))
def send_pm_if_empty_stream(sender, stream, stream_name, realm):
# type: (UserProfile, Stream, Text, Realm) -> None
@@ -1274,9 +1272,6 @@ def check_message(sender, client, message_type_name, message_to,
if subject == "":
raise JsonableError(_("Topic can't be empty"))
subject = truncate_topic(subject)
## FIXME: Commented out temporarily while we figure out what we want
# if not valid_stream_name(subject):
# return json_error(_("Invalid subject name"))
stream = get_stream(stream_name, realm)

View File

@@ -9,7 +9,7 @@ from zerver.lib.actions import check_stream_name, create_streams_if_needed
from zerver.lib.request import JsonableError
from zerver.models import UserProfile, Stream, Subscription, \
Realm, Recipient, bulk_get_recipients, get_recipient, get_stream, \
bulk_get_streams, valid_stream_name
bulk_get_streams
def access_stream_common(user_profile, stream, error):
# type: (UserProfile, Stream, Text) -> Tuple[Recipient, Subscription]
@@ -101,9 +101,7 @@ def list_to_streams(streams_raw, user_profile, autocreate=False):
"""Converts list of dicts to a list of Streams, validating input in the process
For each stream name, we validate it to ensure it meets our
requirements for a proper stream name: that is, that it is shorter
than Stream.MAX_NAME_LENGTH characters and passes
valid_stream_name.
requirements for a proper stream name using check_stream_name.
This function in autocreate mode should be atomic: either an exception will be raised
during a precheck, or all the streams specified will have been created if applicable.
@@ -121,10 +119,7 @@ def list_to_streams(streams_raw, user_profile, autocreate=False):
# Stream names should already have been stripped by the
# caller, but it makes sense to verify anyway.
assert stream_name == stream_name.strip()
if len(stream_name) > Stream.MAX_NAME_LENGTH:
raise JsonableError(_("Stream name (%s) too long.") % (stream_name,))
if not valid_stream_name(stream_name):
raise JsonableError(_("Invalid stream name (%s).") % (stream_name,))
check_stream_name(stream_name)
existing_streams = [] # type: List[Stream]
missing_stream_dicts = [] # type: List[Mapping[str, Any]]

View File

@@ -725,10 +725,6 @@ class Stream(ModelReprMixin, models.Model):
post_save.connect(flush_stream, sender=Stream)
post_delete.connect(flush_stream, sender=Stream)
def valid_stream_name(name):
# type: (Text) -> bool
return name != ""
# The Recipient table is used to map Messages to the set of users who
# received the message. It is implemented as a set of triples (id,
# type_id, type). We have 3 types of recipients: Huddles (for group

View File

@@ -43,7 +43,7 @@ from zerver.lib.actions import (
do_create_realm, do_remove_default_stream, do_set_realm_create_stream_by_admins_only,
gather_subscriptions_helper, bulk_add_subscriptions, bulk_remove_subscriptions,
gather_subscriptions, get_default_streams_for_realm, get_realm, get_stream,
get_user_profile_by_email, set_default_streams,
get_user_profile_by_email, set_default_streams, check_stream_name,
create_stream_if_needed, create_streams_if_needed, active_user_ids
)
@@ -1078,7 +1078,7 @@ class SubscriptionRestApiTest(ZulipTestCase):
**self.api_auth(email)
)
self.assert_json_error(result,
"Invalid stream name (%s)." % (invalid_stream_name,))
"Invalid stream name '%s'" % (invalid_stream_name,))
def test_stream_name_too_long(self):
# type: () -> None
@@ -1095,7 +1095,7 @@ class SubscriptionRestApiTest(ZulipTestCase):
**self.api_auth(email)
)
self.assert_json_error(result,
"Stream name (%s) too long." % (long_stream_name,))
"Stream name too long (limit: 60 characters)")
def test_compose_views_rollback(self):
# type: () -> None
@@ -1403,7 +1403,7 @@ class SubscriptionAPITest(ZulipTestCase):
long_stream_name = "a" * 61
result = self.common_subscribe_to_streams(self.test_email, [long_stream_name])
self.assert_json_error(result,
"Stream name (%s) too long." % (long_stream_name,))
"Stream name too long (limit: 60 characters)")
def test_user_settings_for_adding_streams(self):
# type: () -> None
@@ -1431,7 +1431,7 @@ class SubscriptionAPITest(ZulipTestCase):
invalid_stream_name = ""
result = self.common_subscribe_to_streams(self.test_email, [invalid_stream_name])
self.assert_json_error(result,
"Invalid stream name (%s)." % (invalid_stream_name,))
"Invalid stream name '%s'" % (invalid_stream_name,))
def assert_adding_subscriptions_for_principal(self, invitee, streams, invite_only=False):
# type: (Text, List[Text], bool) -> None
@@ -1935,7 +1935,7 @@ class SubscriptionAPITest(ZulipTestCase):
invalid_stream_name = ""
result = self.client_post("/json/subscriptions/exists",
{"stream": invalid_stream_name})
self.assert_json_error(result, "Invalid characters in stream name")
self.assert_json_error(result, "Invalid stream name ''")
def test_existing_subscriptions_autosubscription(self):
# type: () -> None

View File

@@ -19,12 +19,12 @@ from zerver.lib.actions import bulk_remove_subscriptions, \
do_remove_default_stream, get_topic_history_for_stream
from zerver.lib.response import json_success, json_error, json_response
from zerver.lib.streams import access_stream_by_id, access_stream_by_name, \
check_stream_name_available, filter_stream_authorization, list_to_streams
check_stream_name, check_stream_name_available, filter_stream_authorization, \
list_to_streams
from zerver.lib.validator import check_string, check_list, check_dict, \
check_bool, check_variable_type
from zerver.models import UserProfile, Stream, Realm, Subscription, \
Recipient, get_recipient, get_stream, valid_stream_name, \
get_active_user_dicts_in_realm
Recipient, get_recipient, get_stream, get_active_user_dicts_in_realm
from collections import defaultdict
import ujson
@@ -210,7 +210,7 @@ def add_subscriptions_backend(request, user_profile,
stream_dicts.append(stream_dict_copy)
# Validation of the streams arguments, including enforcement of
# can_create_streams policy and valid_stream_name policy is inside
# can_create_streams policy and check_stream_name policy is inside
# list_to_streams.
existing_streams, created_streams = \
list_to_streams(stream_dicts, user_profile, autocreate=True)
@@ -353,8 +353,7 @@ def get_topics_backend(request, user_profile,
def json_stream_exists(request, user_profile, stream_name=REQ("stream"),
autosubscribe=REQ(validator=check_bool, default=False)):
# type: (HttpRequest, UserProfile, Text, bool) -> HttpResponse
if not valid_stream_name(stream_name):
return json_error(_("Invalid characters in stream name"))
check_stream_name(stream_name)
try:
(stream, recipient, sub) = access_stream_by_name(user_profile, stream_name)