tests: Consistently JSON-encode ‘to’ parameter

Although our POST /messages handler accepts the ‘to’ parameter with or
without JSON encoding, there are two problems with passing it as an
unencoded string.

Firstly, you’d fail to send a message to a stream named ‘true’ or
‘false’ or ‘null’ or ‘2022’, as the JSON interpretation is prioritized
over the plain string interpretation.

Secondly, and more importantly for our tests, it violates our OpenAPI
schema, which requires the parameter to be JSON-encoded.  This is
because OpenAPI has no concept of a parameter that’s “optionally
JSON-encoded”, nor should it: such a parameter cannot be unambiguously
decoded for the reason above.

Our version of openapi-core doesn’t currently detect this schema
violation, but after the next upgrade it will.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit is contained in:
Anders Kaseorg 2022-09-12 23:39:44 -07:00 committed by Tim Abbott
parent a1de8d95a8
commit bd9a1dc971
10 changed files with 71 additions and 47 deletions

View File

@ -15,7 +15,7 @@
curl -X POST {{ api_url }}/v1/messages \
-u BOT_EMAIL_ADDRESS:BOT_API_KEY \
--data-urlencode type=stream \
--data-urlencode to=Denmark \
--data-urlencode 'to="Denmark"' \
--data-urlencode topic=Castle \
--data-urlencode 'content=I come not, friends, to steal away your hearts.'

View File

@ -1401,7 +1401,7 @@ class TestIncomingWebhookBot(ZulipTestCase):
payload = dict(
type="private",
content="Test message",
to=othello.email,
to=orjson.dumps([othello.email]).decode(),
)
result = self.api_post(webhook_bot, "/api/v1/messages", payload)

View File

@ -1028,7 +1028,7 @@ class TestMissedMessageEmailMessages(ZulipTestCase):
"type": "stream",
"topic": "test topic",
"content": "test_receive_missed_stream_message_email_messages",
"to": "Denmark",
"to": orjson.dumps("Denmark").decode(),
},
)
self.assert_json_success(result)
@ -1067,7 +1067,7 @@ class TestMissedMessageEmailMessages(ZulipTestCase):
"type": "stream",
"topic": "test topic",
"content": "test_receive_email_response_for_auth_failures",
"to": "announce",
"to": orjson.dumps("announce").decode(),
},
)
self.assert_json_success(result)
@ -1111,7 +1111,7 @@ class TestMissedMessageEmailMessages(ZulipTestCase):
"type": "stream",
"topic": "test topic",
"content": "test_receive_missed_stream_message_email_messages",
"to": "Denmark",
"to": orjson.dumps("Denmark").decode(),
},
)
self.assert_json_success(result)
@ -1156,7 +1156,7 @@ class TestMissedMessageEmailMessages(ZulipTestCase):
"type": "stream",
"topic": "test topic",
"content": "test_receive_missed_stream_message_email_messages",
"to": "Denmark",
"to": orjson.dumps("Denmark").decode(),
},
)
self.assert_json_success(result)
@ -1192,7 +1192,7 @@ class TestMissedMessageEmailMessages(ZulipTestCase):
"type": "stream",
"topic": "test topic",
"content": "test_receive_missed_stream_message_email_messages",
"to": "Denmark",
"to": orjson.dumps("Denmark").decode(),
},
)
self.assert_json_success(result)
@ -1229,7 +1229,7 @@ class TestMissedMessageEmailMessages(ZulipTestCase):
"type": "stream",
"topic": "test topic",
"content": "test_receive_missed_stream_message_email_messages",
"to": "Denmark",
"to": orjson.dumps("Denmark").decode(),
},
)
self.assert_json_success(result)

View File

@ -385,7 +385,7 @@ class TestMissedMessages(ZulipTestCase):
{
"type": "private",
"content": "Test message",
"to": hamlet.email,
"to": orjson.dumps([hamlet.email]).decode(),
},
)
self.assert_json_success(result)

View File

@ -117,7 +117,7 @@ class RateLimitTests(ZulipTestCase):
"/api/v1/messages",
{
"type": "stream",
"to": "Verona",
"to": orjson.dumps("Verona").decode(),
"content": content,
"topic": "whatever",
},

View File

@ -1,3 +1,5 @@
import orjson
from zerver.lib.test_classes import ZulipTestCase
@ -7,7 +9,7 @@ class LegacySubjectTest(ZulipTestCase):
payload = dict(
type="stream",
to="Verona",
to=orjson.dumps("Verona").decode(),
content="Test message",
)

View File

@ -1,7 +1,7 @@
import datetime
import sys
from email.headerregistry import Address
from typing import TYPE_CHECKING, Any, List, Mapping, Optional, Set
from typing import TYPE_CHECKING, Any, List, Mapping, Optional, Set, Union
from unittest import mock
import orjson
@ -97,7 +97,7 @@ class MessagePOSTTest(ZulipTestCase):
"/json/messages",
{
"type": "stream",
"to": "Verona",
"to": orjson.dumps("Verona").decode(),
"content": "Test message",
"topic": "Test topic",
},
@ -114,7 +114,7 @@ class MessagePOSTTest(ZulipTestCase):
"/api/v1/messages",
{
"type": "stream",
"to": "Verona",
"to": orjson.dumps("Verona").decode(),
"content": "Test message",
"topic": "Test topic",
},
@ -519,7 +519,7 @@ class MessagePOSTTest(ZulipTestCase):
{
"type": "private",
"content": "Test message",
"to": othello.email,
"to": orjson.dumps([othello.email]).decode(),
},
)
self.assert_json_success(result)
@ -538,7 +538,7 @@ class MessagePOSTTest(ZulipTestCase):
{
"type": "private",
"content": "Test message",
"to": user_profile.email,
"to": orjson.dumps([user_profile.email]).decode(),
},
)
self.assert_json_success(result)
@ -814,7 +814,7 @@ class MessagePOSTTest(ZulipTestCase):
"sender": self.mit_email("sipbtest"),
"content": "Test message",
"client": "zephyr_mirror",
"to": self.mit_email("starnine"),
"to": orjson.dumps([self.mit_email("starnine")]).decode(),
},
subdomain="zephyr",
)
@ -912,7 +912,7 @@ class MessagePOSTTest(ZulipTestCase):
self.login("hamlet")
post_data = {
"type": "stream",
"to": "Verona",
"to": orjson.dumps("Verona").decode(),
"content": " I like whitespace at the end! \n\n \n",
"topic": "Test topic",
}
@ -924,7 +924,7 @@ class MessagePOSTTest(ZulipTestCase):
# Test if it removes the new line from the beginning of the message.
post_data = {
"type": "stream",
"to": "Verona",
"to": orjson.dumps("Verona").decode(),
"content": "\nAvoid the new line at the beginning of the message.",
"topic": "Test topic",
}
@ -946,7 +946,7 @@ class MessagePOSTTest(ZulipTestCase):
long_message = "A" * (MAX_MESSAGE_LENGTH + 1)
post_data = {
"type": "stream",
"to": "Verona",
"to": orjson.dumps("Verona").decode(),
"content": long_message,
"topic": "Test topic",
}
@ -967,7 +967,7 @@ class MessagePOSTTest(ZulipTestCase):
long_topic = "A" * (MAX_TOPIC_NAME_LENGTH + 1)
post_data = {
"type": "stream",
"to": "Verona",
"to": orjson.dumps("Verona").decode(),
"content": "test content",
"topic": long_topic,
}
@ -1189,7 +1189,7 @@ class MessagePOSTTest(ZulipTestCase):
"content": "Test message",
"client": "irc_mirror",
"topic": "from irc",
"to": "IRCLand",
"to": orjson.dumps("IRCLand").decode(),
},
)
self.assert_json_success(result)
@ -1212,7 +1212,7 @@ class MessagePOSTTest(ZulipTestCase):
"content": "Test message",
"client": "irc_mirror",
"topic": "from irc",
"to": "IRCLand",
"to": orjson.dumps("IRCLand").decode(),
},
)
self.assert_json_success(result)
@ -1236,7 +1236,7 @@ class MessagePOSTTest(ZulipTestCase):
def test_with(sender_email: str, client: str, forged: bool) -> None:
payload = dict(
type="stream",
to=stream_name,
to=orjson.dumps(stream_name).decode(),
client=client,
topic="whatever",
content="whatever",
@ -1283,7 +1283,7 @@ class MessagePOSTTest(ZulipTestCase):
payload = dict(
type="stream",
to=stream_name,
to=orjson.dumps(stream_name).decode(),
topic="whatever",
content="whatever",
)
@ -1309,7 +1309,7 @@ class MessagePOSTTest(ZulipTestCase):
"/api/v1/messages",
{
"type": "stream",
"to": "notify_channel",
"to": orjson.dumps("notify_channel").decode(),
"content": "Test message",
"topic": "Test topic",
},
@ -1330,7 +1330,7 @@ class MessagePOSTTest(ZulipTestCase):
self.make_stream(stream_name, invite_only=False)
payload = dict(
type="stream",
to=stream_name,
to=orjson.dumps(stream_name).decode(),
topic="whatever",
content="whatever",
)
@ -1352,7 +1352,7 @@ class ScheduledMessageTest(ZulipTestCase):
def do_schedule_message(
self,
msg_type: str,
to: str,
to: Union[str, int, List[str], List[int]],
msg: str,
defer_until: str = "",
tz_guess: str = "",
@ -1367,7 +1367,7 @@ class ScheduledMessageTest(ZulipTestCase):
payload = {
"type": msg_type,
"to": to,
"to": orjson.dumps(to).decode(),
"content": msg,
"topic": topic_name,
"realm_str": realm_str,
@ -1407,7 +1407,9 @@ class ScheduledMessageTest(ZulipTestCase):
# Scheduling a private message is successful.
othello = self.example_user("othello")
hamlet = self.example_user("hamlet")
result = self.do_schedule_message("private", othello.email, content + " 3", defer_until_str)
result = self.do_schedule_message(
"private", [othello.email], content + " 3", defer_until_str
)
message = self.last_scheduled_message()
self.assert_json_success(result)
self.assertEqual(message.content, "Test message 3")
@ -1416,14 +1418,14 @@ class ScheduledMessageTest(ZulipTestCase):
# Setting a reminder in PM's to other users causes a error.
result = self.do_schedule_message(
"private", othello.email, content + " 4", defer_until_str, delivery_type="remind"
"private", [othello.email], content + " 4", defer_until_str, delivery_type="remind"
)
self.assert_json_error(result, "Reminders can only be set for streams.")
# Setting a reminder in PM's to ourself is successful.
# Required by reminders from message actions popover caret feature.
result = self.do_schedule_message(
"private", hamlet.email, content + " 5", defer_until_str, delivery_type="remind"
"private", [hamlet.email], content + " 5", defer_until_str, delivery_type="remind"
)
message = self.last_scheduled_message()
self.assert_json_success(result)
@ -1891,7 +1893,7 @@ class StreamMessagesTest(ZulipTestCase):
"/api/v1/messages",
{
"type": "stream",
"to": "Verona",
"to": orjson.dumps("Verona").decode(),
"sender": self.mit_email("sipbtest"),
"client": "zephyr_mirror",
"topic": "announcement",

View File

@ -281,7 +281,11 @@ class ReactionMessageIDTest(ZulipTestCase):
result = self.api_post(
pm_sender,
"/api/v1/messages",
{"type": "private", "content": "Test message", "to": pm_recipient.email},
{
"type": "private",
"content": "Test message",
"to": orjson.dumps([pm_recipient.email]).decode(),
},
)
pm_id = self.assert_json_success(result)["id"]
reaction_info = {
@ -306,7 +310,11 @@ class ReactionTest(ZulipTestCase):
pm = self.api_post(
pm_sender,
"/api/v1/messages",
{"type": "private", "content": "Test message", "to": pm_recipient.email},
{
"type": "private",
"content": "Test message",
"to": orjson.dumps([pm_recipient.email]).decode(),
},
)
self.assert_json_success(pm)
content = orjson.loads(pm.content)
@ -336,7 +344,11 @@ class ReactionTest(ZulipTestCase):
pm = self.api_post(
pm_sender,
"/api/v1/messages",
{"type": "private", "content": "Test message", "to": pm_recipient.email},
{
"type": "private",
"content": "Test message",
"to": orjson.dumps([pm_recipient.email]).decode(),
},
)
self.assert_json_success(pm)
@ -417,7 +429,11 @@ class ReactionEventTest(ZulipTestCase):
result = self.api_post(
pm_sender,
"/api/v1/messages",
{"type": "private", "content": "Test message", "to": pm_recipient.email},
{
"type": "private",
"content": "Test message",
"to": orjson.dumps([pm_recipient.email]).decode(),
},
)
pm_id = self.assert_json_success(result)["id"]
@ -456,7 +472,11 @@ class ReactionEventTest(ZulipTestCase):
result = self.api_post(
pm_sender,
"/api/v1/messages",
{"type": "private", "content": "Test message", "to": pm_recipient.email},
{
"type": "private",
"content": "Test message",
"to": orjson.dumps([pm_recipient.email]).decode(),
},
)
content = self.assert_json_success(result)
pm_id = content["id"]

View File

@ -418,7 +418,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
payload = dict(
type="stream",
to=stream_name,
to=orjson.dumps(stream_name).decode(),
topic="whatever",
content=content_with_group_mention,
)

View File

@ -66,7 +66,7 @@ class WidgetContentTestCase(ZulipTestCase):
payload = dict(
type="stream",
to=stream_name,
to=orjson.dumps(stream_name).decode(),
topic="whatever",
content="whatever",
)
@ -122,7 +122,7 @@ class WidgetContentTestCase(ZulipTestCase):
payload = dict(
type="stream",
to=stream_name,
to=orjson.dumps(stream_name).decode(),
topic="whatever",
content=content,
widget_content=orjson.dumps(widget_content).decode(),
@ -152,7 +152,7 @@ class WidgetContentTestCase(ZulipTestCase):
payload = dict(
type="stream",
to=stream_name,
to=orjson.dumps(stream_name).decode(),
topic="whatever",
content=content,
)
@ -180,7 +180,7 @@ class WidgetContentTestCase(ZulipTestCase):
payload = dict(
type="stream",
to=stream_name,
to=orjson.dumps(stream_name).decode(),
topic="whatever",
content=content,
)
@ -231,7 +231,7 @@ class WidgetContentTestCase(ZulipTestCase):
payload = dict(
type="stream",
to=stream_name,
to=orjson.dumps(stream_name).decode(),
topic="whatever",
content=content,
)
@ -259,7 +259,7 @@ class WidgetContentTestCase(ZulipTestCase):
payload = dict(
type="stream",
to=stream_name,
to=orjson.dumps(stream_name).decode(),
topic="whatever",
content=content,
)
@ -317,7 +317,7 @@ class WidgetContentTestCase(ZulipTestCase):
payload = dict(
type="stream",
to=stream_name,
to=orjson.dumps(stream_name).decode(),
topic="whatever",
content=content,
)
@ -374,7 +374,7 @@ class WidgetContentTestCase(ZulipTestCase):
payload = dict(
type="stream",
to=stream_name,
to=orjson.dumps(stream_name).decode(),
topic="whatever",
content=content,
)