From 7e9ccead2e8561b5e319a791227f58df36812081 Mon Sep 17 00:00:00 2001 From: Yashashvi Dave Date: Mon, 30 Apr 2018 12:29:51 +0530 Subject: [PATCH] stream settings: Fix server error on long stream description. Add backend validations to check stream description length. --- zerver/models.py | 3 ++- zerver/tests/test_subs.py | 8 ++++++-- zerver/views/streams.py | 5 +++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/zerver/models.py b/zerver/models.py index 692e03baa0..074857fbd3 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -935,6 +935,7 @@ def generate_email_token_for_stream() -> str: class Stream(models.Model): MAX_NAME_LENGTH = 60 + MAX_DESCRIPTION_LENGTH = 1024 name = models.CharField(max_length=MAX_NAME_LENGTH, db_index=True) # type: Text realm = models.ForeignKey(Realm, db_index=True, on_delete=CASCADE) # type: Realm invite_only = models.NullBooleanField(default=False) # type: Optional[bool] @@ -955,7 +956,7 @@ class Stream(models.Model): # have plenty of room for the token. email_token = models.CharField( max_length=32, default=generate_email_token_for_stream) # type: str - description = models.CharField(max_length=1024, default=u'') # type: Text + description = models.CharField(max_length=MAX_DESCRIPTION_LENGTH, default=u'') # type: Text date_created = models.DateTimeField(default=timezone_now) # type: datetime.datetime deactivated = models.BooleanField(default=False) # type: bool diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index dda3eec7e7..d137a45509 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -610,12 +610,11 @@ class StreamAdminTest(ZulipTestCase): self.assert_json_error(result, "Invalid stream id") def test_change_stream_description(self) -> None: - user_profile = self.example_user('hamlet') + user_profile = self.example_user('iago') email = user_profile.email self.login(email) realm = user_profile.realm self.subscribe(user_profile, 'stream_name1') - do_change_is_admin(user_profile, True) events = [] # type: List[Mapping[str, Any]] with tornado_redirected_to_list(events): @@ -644,6 +643,11 @@ class StreamAdminTest(ZulipTestCase): self.assertEqual('Test description', stream.description) + result = self.client_patch('/json/streams/%d' % (stream_id,), + {'description': ujson.dumps('a' * 1025)}) + self.assert_json_error(result, "description is too long (limit: %s characters)." + % (Stream.MAX_DESCRIPTION_LENGTH)) + def test_change_stream_description_requires_realm_admin(self) -> None: user_profile = self.example_user('hamlet') email = user_profile.email diff --git a/zerver/views/streams.py b/zerver/views/streams.py index c3985d16e9..9e0721b833 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -27,7 +27,7 @@ from zerver.lib.streams import access_stream_by_id, access_stream_by_name, \ check_stream_name, check_stream_name_available, filter_stream_authorization, \ list_to_streams, access_stream_for_delete_or_update, access_default_stream_group_by_id from zerver.lib.validator import check_string, check_int, check_list, check_dict, \ - check_bool, check_variable_type + check_bool, check_variable_type, check_capped_string from zerver.models import UserProfile, Stream, Realm, Subscription, \ Recipient, get_recipient, get_stream, \ get_system_bot, get_user @@ -144,7 +144,8 @@ def remove_default_stream(request: HttpRequest, def update_stream_backend( request: HttpRequest, user_profile: UserProfile, stream_id: int, - description: Optional[str]=REQ(validator=check_string, default=None), + description: Optional[str]=REQ(validator=check_capped_string( + Stream.MAX_DESCRIPTION_LENGTH), default=None), is_private: Optional[bool]=REQ(validator=check_bool, default=None), history_public_to_subscribers: Optional[bool]=REQ(validator=check_bool, default=None), new_name: Optional[str]=REQ(validator=check_string, default=None),