mirror of https://github.com/zulip/zulip.git
narrow-parameter: Add validation for the narrow parameters.
This commit is contained in:
parent
def2d402f4
commit
5db24e002c
|
@ -20,6 +20,7 @@ from django.conf import settings
|
||||||
from django.core.exceptions import ValidationError
|
from django.core.exceptions import ValidationError
|
||||||
from django.db import connection
|
from django.db import connection
|
||||||
from django.utils.translation import gettext as _
|
from django.utils.translation import gettext as _
|
||||||
|
from pydantic import BaseModel, model_validator
|
||||||
from sqlalchemy.dialects import postgresql
|
from sqlalchemy.dialects import postgresql
|
||||||
from sqlalchemy.engine import Connection, Row
|
from sqlalchemy.engine import Connection, Row
|
||||||
from sqlalchemy.sql import (
|
from sqlalchemy.sql import (
|
||||||
|
@ -79,6 +80,68 @@ from zerver.models.users import (
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class NarrowParameter(BaseModel):
|
||||||
|
operator: str
|
||||||
|
operand: Any
|
||||||
|
negated: bool = False
|
||||||
|
|
||||||
|
@model_validator(mode="before")
|
||||||
|
@classmethod
|
||||||
|
def convert_term(cls, elem: Union[Dict[str, Any], List[str]]) -> Dict[str, Any]:
|
||||||
|
# We have to support a legacy tuple format.
|
||||||
|
if isinstance(elem, list):
|
||||||
|
if len(elem) != 2 or any(not isinstance(x, str) for x in elem):
|
||||||
|
raise ValueError("element is not a string pair")
|
||||||
|
return dict(operator=elem[0], operand=elem[1])
|
||||||
|
|
||||||
|
elif isinstance(elem, dict):
|
||||||
|
if "operand" not in elem or elem["operand"] is None:
|
||||||
|
raise ValueError("operand is missing")
|
||||||
|
|
||||||
|
if "operator" not in elem or elem["operator"] is None:
|
||||||
|
raise ValueError("operator is missing")
|
||||||
|
return elem
|
||||||
|
else:
|
||||||
|
raise ValueError("dict or list required")
|
||||||
|
|
||||||
|
@model_validator(mode="after")
|
||||||
|
def validate_terms(self) -> "NarrowParameter":
|
||||||
|
# Make sure to sync this list to frontend also when adding a new operator that
|
||||||
|
# supports integer IDs. Relevant code is located in web/src/message_fetch.js
|
||||||
|
# in handle_operators_supporting_id_based_api function where you will need to
|
||||||
|
# update operators_supporting_id, or operators_supporting_ids array.
|
||||||
|
operators_supporting_id = [
|
||||||
|
*channel_operators,
|
||||||
|
"id",
|
||||||
|
"sender",
|
||||||
|
"group-pm-with",
|
||||||
|
"dm-including",
|
||||||
|
]
|
||||||
|
operators_supporting_ids = ["pm-with", "dm"]
|
||||||
|
operators_non_empty_operand = {"search"}
|
||||||
|
|
||||||
|
operator = self.operator
|
||||||
|
if operator in operators_supporting_id:
|
||||||
|
operand_validator: Validator[object] = check_string_or_int
|
||||||
|
elif operator in operators_supporting_ids:
|
||||||
|
operand_validator = check_string_or_int_list
|
||||||
|
elif operator in operators_non_empty_operand:
|
||||||
|
operand_validator = check_required_string
|
||||||
|
else:
|
||||||
|
operand_validator = check_string
|
||||||
|
|
||||||
|
try:
|
||||||
|
self.operand = operand_validator("operand", self.operand)
|
||||||
|
self.operator = check_string("operator", self.operator)
|
||||||
|
if self.negated is not None:
|
||||||
|
self.negated = check_bool("negated", self.negated)
|
||||||
|
except ValidationError as error:
|
||||||
|
raise JsonableError(error.message)
|
||||||
|
|
||||||
|
# whitelist the fields we care about for now
|
||||||
|
return self
|
||||||
|
|
||||||
|
|
||||||
def is_spectator_compatible(narrow: Iterable[Dict[str, Any]]) -> bool:
|
def is_spectator_compatible(narrow: Iterable[Dict[str, Any]]) -> bool:
|
||||||
# This implementation should agree with is_spectator_compatible in hash_parser.ts.
|
# This implementation should agree with is_spectator_compatible in hash_parser.ts.
|
||||||
supported_operators = [
|
supported_operators = [
|
||||||
|
|
|
@ -395,10 +395,129 @@ class UnreadCountTests(ZulipTestCase):
|
||||||
.values_list("message_id", flat=True),
|
.values_list("message_id", flat=True),
|
||||||
message_ids[5::2],
|
message_ids[5::2],
|
||||||
)
|
)
|
||||||
|
response = self.assert_json_success(
|
||||||
|
self.client_post(
|
||||||
|
"/json/messages/flags/narrow",
|
||||||
|
{
|
||||||
|
"anchor": message_ids[3],
|
||||||
|
"include_anchor": "false",
|
||||||
|
"num_before": 0,
|
||||||
|
"num_after": 5,
|
||||||
|
"narrow": orjson.dumps([["stream", "Verona"], ["topic", "topic 1"]]).decode(),
|
||||||
|
"op": "add",
|
||||||
|
"flag": "starred",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
)
|
||||||
|
cordelia = self.example_user("cordelia")
|
||||||
|
response = self.assert_json_success(
|
||||||
|
self.client_post(
|
||||||
|
"/json/messages/flags/narrow",
|
||||||
|
{
|
||||||
|
"anchor": message_ids[3],
|
||||||
|
"include_anchor": "false",
|
||||||
|
"num_before": 0,
|
||||||
|
"num_after": 5,
|
||||||
|
"narrow": orjson.dumps([{"operator": "dm", "operand": [cordelia.id]}]).decode(),
|
||||||
|
"op": "add",
|
||||||
|
"flag": "starred",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
def test_update_flags_for_narrow_misuse(self) -> None:
|
def test_update_flags_for_narrow_misuse(self) -> None:
|
||||||
self.login("hamlet")
|
self.login("hamlet")
|
||||||
|
|
||||||
|
self.assert_json_error(
|
||||||
|
self.client_post(
|
||||||
|
"/json/messages/flags/narrow",
|
||||||
|
{
|
||||||
|
"anchor": "1",
|
||||||
|
"include_anchor": "false",
|
||||||
|
"num_before": "1",
|
||||||
|
"num_after": "1",
|
||||||
|
"narrow": "[[],[]]",
|
||||||
|
"op": "add",
|
||||||
|
"flag": "read",
|
||||||
|
},
|
||||||
|
),
|
||||||
|
"Invalid narrow[0]: Value error, element is not a string pair",
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assert_json_error(
|
||||||
|
self.client_post(
|
||||||
|
"/json/messages/flags/narrow",
|
||||||
|
{
|
||||||
|
"anchor": "1",
|
||||||
|
"include_anchor": "false",
|
||||||
|
"num_before": "1",
|
||||||
|
"num_after": "1",
|
||||||
|
"narrow": orjson.dumps(
|
||||||
|
[
|
||||||
|
{"operator": None, "operand": "Verona"},
|
||||||
|
{"operator": "topic", "operand": "topic 1"},
|
||||||
|
]
|
||||||
|
).decode(),
|
||||||
|
"op": "add",
|
||||||
|
"flag": "read",
|
||||||
|
},
|
||||||
|
),
|
||||||
|
"Invalid narrow[0]: Value error, operator is missing",
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assert_json_error(
|
||||||
|
self.client_post(
|
||||||
|
"/json/messages/flags/narrow",
|
||||||
|
{
|
||||||
|
"anchor": "1",
|
||||||
|
"include_anchor": "false",
|
||||||
|
"num_before": "1",
|
||||||
|
"num_after": "1",
|
||||||
|
"narrow": orjson.dumps(
|
||||||
|
[
|
||||||
|
{"operator": "stream", "operand": None},
|
||||||
|
{"operator": "topic", "operand": "topic 1"},
|
||||||
|
]
|
||||||
|
).decode(),
|
||||||
|
"op": "add",
|
||||||
|
"flag": "read",
|
||||||
|
},
|
||||||
|
),
|
||||||
|
"Invalid narrow[0]: Value error, operand is missing",
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assert_json_error(
|
||||||
|
self.client_post(
|
||||||
|
"/json/messages/flags/narrow",
|
||||||
|
{
|
||||||
|
"anchor": "1",
|
||||||
|
"include_anchor": "false",
|
||||||
|
"num_before": "1",
|
||||||
|
"num_after": "1",
|
||||||
|
"narrow": orjson.dumps(["asadasd"]).decode(),
|
||||||
|
"op": "add",
|
||||||
|
"flag": "read",
|
||||||
|
},
|
||||||
|
),
|
||||||
|
"Invalid narrow[0]: Value error, dict or list required",
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assert_json_error(
|
||||||
|
self.client_post(
|
||||||
|
"/json/messages/flags/narrow",
|
||||||
|
{
|
||||||
|
"anchor": "1",
|
||||||
|
"include_anchor": "false",
|
||||||
|
"num_before": "1",
|
||||||
|
"num_after": "1",
|
||||||
|
"narrow": orjson.dumps([{"operator": "search", "operand": ""}]).decode(),
|
||||||
|
"op": "add",
|
||||||
|
"flag": "read",
|
||||||
|
},
|
||||||
|
),
|
||||||
|
"operand cannot be blank.",
|
||||||
|
)
|
||||||
|
|
||||||
response = self.client_post(
|
response = self.client_post(
|
||||||
"/json/messages/flags/narrow",
|
"/json/messages/flags/narrow",
|
||||||
{
|
{
|
||||||
|
|
|
@ -11,7 +11,7 @@ from zerver.actions.message_flags import (
|
||||||
do_update_message_flags,
|
do_update_message_flags,
|
||||||
)
|
)
|
||||||
from zerver.lib.exceptions import JsonableError
|
from zerver.lib.exceptions import JsonableError
|
||||||
from zerver.lib.narrow import OptionalNarrowListT, fetch_messages, parse_anchor_value
|
from zerver.lib.narrow import NarrowParameter, fetch_messages, parse_anchor_value
|
||||||
from zerver.lib.request import RequestNotes
|
from zerver.lib.request import RequestNotes
|
||||||
from zerver.lib.response import json_success
|
from zerver.lib.response import json_success
|
||||||
from zerver.lib.streams import access_stream_by_id
|
from zerver.lib.streams import access_stream_by_id
|
||||||
|
@ -77,7 +77,7 @@ def update_message_flags_for_narrow(
|
||||||
include_anchor: Json[bool] = True,
|
include_anchor: Json[bool] = True,
|
||||||
num_before: Json[NonNegativeInt],
|
num_before: Json[NonNegativeInt],
|
||||||
num_after: Json[NonNegativeInt],
|
num_after: Json[NonNegativeInt],
|
||||||
narrow: Json[OptionalNarrowListT],
|
narrow: Json[Optional[List[NarrowParameter]]],
|
||||||
operation: Annotated[str, ApiParamConfig("op")],
|
operation: Annotated[str, ApiParamConfig("op")],
|
||||||
flag: str,
|
flag: str,
|
||||||
) -> HttpResponse:
|
) -> HttpResponse:
|
||||||
|
@ -92,8 +92,13 @@ def update_message_flags_for_narrow(
|
||||||
)
|
)
|
||||||
num_after = min(num_after, MAX_MESSAGES_PER_UPDATE - num_before)
|
num_after = min(num_after, MAX_MESSAGES_PER_UPDATE - num_before)
|
||||||
|
|
||||||
|
if narrow is not None and len(narrow) > 0:
|
||||||
|
narrow_dict = [x.model_dump() for x in narrow]
|
||||||
|
else:
|
||||||
|
narrow_dict = None
|
||||||
|
|
||||||
query_info = fetch_messages(
|
query_info = fetch_messages(
|
||||||
narrow=narrow,
|
narrow=narrow_dict,
|
||||||
user_profile=user_profile,
|
user_profile=user_profile,
|
||||||
realm=user_profile.realm,
|
realm=user_profile.realm,
|
||||||
is_web_public_query=False,
|
is_web_public_query=False,
|
||||||
|
|
Loading…
Reference in New Issue