From b998138d3ab8ca0ea212c00469042349f8c3fe53 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Sun, 25 Apr 2021 20:54:32 +0200 Subject: [PATCH] outgoing_webhook: Handle valid, but unexpected json in response. Responses such as "null" or "true" are valid json, but json.loads returns different objects than dicts that the codepath expects. Fixes #18223. --- zerver/lib/outgoing_webhook.py | 3 +++ zerver/tests/test_outgoing_webhook_system.py | 27 ++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/zerver/lib/outgoing_webhook.py b/zerver/lib/outgoing_webhook.py index 32a1cc0be6..7fde13c373 100644 --- a/zerver/lib/outgoing_webhook.py +++ b/zerver/lib/outgoing_webhook.py @@ -291,6 +291,9 @@ def process_success_response( fail_with_message(event, "Invalid JSON in response") return + if not isinstance(response_json, dict): + raise JsonableError(_("Invalid response format")) + success_data = service_handler.process_success(response_json) if success_data is None: diff --git a/zerver/tests/test_outgoing_webhook_system.py b/zerver/tests/test_outgoing_webhook_system.py index 904889b746..505d439f06 100644 --- a/zerver/tests/test_outgoing_webhook_system.py +++ b/zerver/tests/test_outgoing_webhook_system.py @@ -261,6 +261,33 @@ The outgoing webhook server attempted to send a message in Zulip, but that reque assert bot_user.bot_owner is not None self.assertEqual(bot_owner_notification.recipient_id, bot_user.bot_owner.recipient_id) + def test_invalid_response_format(self) -> None: + bot_user = self.example_user("outgoing_webhook_bot") + mock_event = self.mock_event(bot_user) + service_handler = GenericOutgoingWebhookService("token", bot_user, "service") + + expect_logging_info = self.assertLogs(level="INFO") + expect_fail = mock.patch("zerver.lib.outgoing_webhook.fail_with_message") + + with responses.RequestsMock(assert_all_requests_are_fired=True) as requests_mock: + # We mock the endpoint to return response with valid json which doesn't + # translate to a dict like is expected, + requests_mock.add( + requests_mock.POST, "https://example.zulip.com", status=200, json=True + ) + with expect_logging_info, expect_fail as mock_fail: + do_rest_call("https://example.zulip.com", mock_event, service_handler) + self.assertTrue(mock_fail.called) + bot_owner_notification = self.get_last_message() + self.assertEqual( + bot_owner_notification.content, + """[A message](http://zulip.testserver/#narrow/stream/999-Verona/topic/Foo/near/) to your bot @_**Outgoing Webhook** triggered an outgoing webhook. +The outgoing webhook server attempted to send a message in Zulip, but that request resulted in the following error: +> Invalid response format""", + ) + assert bot_user.bot_owner is not None + self.assertEqual(bot_owner_notification.recipient_id, bot_user.bot_owner.recipient_id) + class TestOutgoingWebhookMessaging(ZulipTestCase): def create_outgoing_bot(self, bot_owner: UserProfile) -> UserProfile: