From b2dd15fd863e448fec3e57450888c9d24b3f8967 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Mon, 21 Jun 2021 12:11:32 -0700 Subject: [PATCH] message_edit: Reject buggy noop topic edit requests. A buggy client might send a message_edit request to change the topic field, sending the current topic as the new value. Previously, we would treat that as a normal request to edit the topic; now we act as though the API request had not requested a topic change. In the common case that only the topic was in the edit request, this now results in an error that should help client implementations identify their bug. This fixes a bad interaction with the "unresolve topic" logic, which assumed that upstream logic had verified that the topic was actually changing. --- zerver/lib/actions.py | 3 +++ zerver/tests/test_message_edit.py | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index fed1e2073f..c29b163768 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -2852,6 +2852,9 @@ def check_update_message( # use REQ_topic as well (or otherwise are guaranteed to strip input). if topic_name is not None: topic_name = topic_name.strip() + if topic_name == message.topic_name(): + topic_name = None + validate_message_edit_payload(message, stream_id, topic_name, propagate_mode, content) is_no_topic_msg = message.topic_name() == "(no topic)" diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index 326def25c4..268c378631 100644 --- a/zerver/tests/test_message_edit.py +++ b/zerver/tests/test_message_edit.py @@ -1968,6 +1968,18 @@ class EditMessageTest(EditMessageTestCase): ) id2 = self.send_stream_message(admin_user, "new", topic_name=original_topic) + # Check that we don't incorrectly send "unresolve topic" + # notifications when asking the preserve the current topic. + result = self.client_patch( + "/json/messages/" + str(id1), + { + "message_id": id1, + "topic": original_topic, + "propagate_mode": "change_all", + }, + ) + self.assert_json_error(result, "Nothing to change") + resolved_topic = RESOLVED_TOPIC_PREFIX + original_topic result = self.client_patch( "/json/messages/" + str(id1),