outgoing_webhook: Treat "" json in response as response_not_required.

b7b1ec0aeb made our checks of the response
format stronger, to enforce that the json translates to a valid dict.
However, old client code (zulip_botserver) was using "" as equivalent to
response_not_required - so we need to keep backward-compatibility to not
break things built on it.
This commit is contained in:
Mateusz Mandera
2021-05-13 15:09:58 +02:00
committed by Tim Abbott
parent 41a477bbfb
commit de6bd22ee9
2 changed files with 38 additions and 0 deletions

View File

@@ -292,6 +292,12 @@ def process_success_response(
except json.JSONDecodeError: except json.JSONDecodeError:
raise JsonableError(_("Invalid JSON in response")) raise JsonableError(_("Invalid JSON in response"))
if response_json == "":
# Versions of zulip_botserver before 2021-05 used
# json.dumps("") as their "no response required" success
# response; handle that for backwards-compatibility.
return
if not isinstance(response_json, dict): if not isinstance(response_json, dict):
raise JsonableError(_("Invalid response format")) raise JsonableError(_("Invalid response format"))

View File

@@ -510,3 +510,35 @@ class TestOutgoingWebhookMessaging(ZulipTestCase):
self.assertEqual(last_message.topic_name(), "bar") self.assertEqual(last_message.topic_name(), "bar")
display_recipient = get_display_recipient(last_message.recipient) display_recipient = get_display_recipient(last_message.recipient)
self.assertEqual(display_recipient, "Denmark") self.assertEqual(display_recipient, "Denmark")
@responses.activate
def test_empty_string_json_as_response_to_outgoing_webhook_request(self) -> None:
"""
Verifies that if the response to the request triggered by mentioning the bot
is the json representation of the empty string, the outcome is the same
as {"response_not_required": True} - since this behavior is kept for
backwards-compatibility.
"""
bot_owner = self.example_user("othello")
bot = self.create_outgoing_bot(bot_owner)
responses.add(
responses.POST,
"https://bot.example.com/",
json="",
)
with self.assertLogs(level="INFO") as logs:
stream_message_id = self.send_stream_message(
bot_owner, "Denmark", content=f"@**{bot.full_name}** foo", topic_name="bar"
)
self.assertEqual(len(responses.calls), 1)
self.assertEqual(len(logs.output), 1)
self.assertIn(f"Outgoing webhook request from {bot.id}@zulip took ", logs.output[0])
# We verify that no new message was sent, since that's the behavior implied
# by the response_not_required option.
last_message = self.get_last_message()
self.assertEqual(last_message.id, stream_message_id)