The Django convention is for __repr__ to include the type and __str__
to omit it. In fact its default __repr__ implementation for models
automatically adds a type prefix to __str__, which has resulted in the
type being duplicated:
>>> UserProfile.objects.first()
<UserProfile: <UserProfile: emailgateway@zulip.com <Realm: zulipinternal 1>>>
Signed-off-by: Anders Kaseorg <anders@zulip.com>
In commit 8181ec4b56, we removed the `realm_str` as a parameter
for `send_message_backed`. This removes a missed test that included
this as a parameter for that endpoint/function.
This commit renames reset_emails_in_zulip_realm function to
reset_email_visibility_to_everyone_in_zulip_realm which makes
it more clear to understand what the function actually does.
This commit also adds a comment explaining what this function
does.
This already became useless in 6e11754642,
as detailed in the API changelog entry here. At this point, we should
eliminate this param and the weird code around it.
This commit also deletes the associated tests added in
6e11754642, since with realm_str removed,
they make no sense anymore (and actually fail with an OpenAPI error due
to using params not used in the API). Hypothetically they could be
translated to use the subdomain= kwarg, but that also doesn't make
sense, since at that point they'd be just testing the case of a user
making an API request on a different subdomain than their current one
and that's just redundant and already tested generally in
test_decorators.
In the case where a stream existed but had no subscribers, the error
message used to send to the owner always used `stream_name`, which
may have been None.
Switch to using `stream.name` rather than `stream_name` for this case.
Removes the initial check in `_internal_prep_message` of the length
of the message content because the `check_message` in the try block
will call `normalize_body` on the message content string, which
does a more robust check of the message content (empty string, null
bytes, length). If the message content length exceeds the value of
`settings.MAX_MESSAGE_LENGTH`, then it is truncated based on that
value. Updates associated backend test for these changes.
The removed length check would truncate the message content with a
hard coded value instead of using the value for
`settings.MAX_MESSAGE_LENGTH`.
Also, removes an extraneous comment about removing null bytes. If
there are null bytes in the message content, then `normalize_body`
will raise an error.
Note that the previous check had intentionally reduced any message over
the 10000 character limit to 3900 characters, with the code in
question dating to 2012's 100df7e349.
The 3900 character truncating rule was implemented for incoming emails
with the email gateway, and predated other features to help with
overly long messages (better stripping of email footers via Talon,
introduced in f1f48f305e, and
condensing, introduced in c92d664b44).
While we could preserve that logic if desired, it likely is no longer
a necessary or useful variation from our usual truncation rules.
This reverts commit 851d68e0fc.
That commit widened how long the transaction is open, which made it
much more likely that after the user was created in the transaction,
and the memcached caches were flushed, some other request will fill
the `get_realm_user_dicts` cache with data which did not include the
new user (because it had not been committed yet).
If a user creation request lost this race, the user would, upon first
request to `/`, get a blank page and a Javascript error:
Unknown user_id in get_by_user_id: 12345
...where 12345 was their own user-id. This error would persist until
the cache expired (in 7 days) or something else expunged it.
Reverting this does not prevent the race, as the post_save hook's call
to flush_user_profile is still in a transaction (and has been since
168f241ff0), and thus leaves the potential race window open.
However, it much shortens the potential window of opportunity, and is
a reasonable short-term stopgap.
Black 23 enforces some slightly more specific rules about empty line
counts and redundant parenthesis removal, but the result is still
compatible with Black 22.
(This does not actually upgrade our Python environment to Black 23
yet.)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We change the do_create_user function to use transaction.atomic
decorator instead of using with block. Due to this change, all
send_event calls are made inside transaction.on_commit.
Some other changes -
- Remove transaction.atomic decorator from send_inital_realm_messages
since it is now called inside a transaction.
- Made changes in tests which tests message events and notifications
to make sure on_commit callbacks are executed.
Previously, test cases or clients accessing /json/ views using HTTP
Basic Auth would be accepted, while we intended to only allow clients
authenticated with a session cookie to access these views.
This adds a check on the accessed path to avoid this possibility.
It seems unlikely that any API clients clients were taking advantage
of this unintended quirk; so we're not going to bother documenting
this bug fix as an API change. In any case, it should be trivial for
anyone affected to consult the documentation and then switch their
/json/foo URL to a correct /api/v1/foo URL.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This adds a helper based on testing patterns of using the "queries_captured"
context manager with "assert_length" to check the number of queries
executed for preventing performance regression.
It explains the rationale of checking the query count through an
"AssertionError" and prints the queries captured as assert_length does,
but with a format optimized for displaying the queries in a more
readable manner.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This commit adds the OPTIONAL .realm attribute to Message
(and ArchivedMessage), with the server changes for making new Messages
have this set. Old Messages still have to be migrated to backfill this,
before it can be non-nullable.
Appropriate test changes to correctly set .realm for Messages the tests
manually create are included here as well.
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>
Now that we can assume Python 3.6+, we can use the
email.headerregistry module to replace hacky manual email address
parsing.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit removes WILDCARD_MENTION_POLICY_STREAM_ADMINS
option of wildcard_mention_policy since we are not moving
forward with stream administrator concept and instead working
on new permssions model as per #19525.
We also add a migration to change wildcard_mention_policy of
existing realms to WILDCARD_MENTION_POLICY_ADMINS. This change
is fine since we were already treating both the setting values
as same as stream admin concept was not implemented completely.
Since `HttpResponse` is an inaccurate representation of the
monkey-patched response object returned by the Django test client, we
replace it with `_MonkeyPatchedWSGIResponse` as `TestHttpResponse`.
This replaces `HttpResponse` in zerver/tests, analytics/tests, coporate/tests,
zerver/lib/test_classes.py, and zerver/lib/test_helpers.py with
`TestHttpResponse`. Several files in zerver/tests are excluded
from this substitution.
This commit is auto-generated by a script, with manual adjustments on certain
files squashed into it.
This is a part of the django-stubs refactorings.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Add none-checks, rename variables (to avoid redefinition of
the same variable with different types error), add necessary
type annotations.
This is a part of #18777.
Signed-off-by: Zixuan James Li <359101898@qq.com>
In these tests, the code ends up with a logged in session when it's
undesired - later on these tests make requests to a different subdomain
- where a logged in session is not supposed to exist. This leads to an
unintended, strange situation where request.user is a user from the old
subdomain but the request itself is to a *different* subdomain. This
throws off get_realm_from_request, which will return the realm from
request.user.realm - which is not what these tests want and can lead to
these tests failing when some of the production code being tested
switches to using get_realm_from_request instead of
get_realm(get_subdomain).
Removes `client` parameter from backend tests using the
`POST /messages` endpoint when the test can use the default
`User-Agent` as the client, which is set to `ZulipMobile` for API
requests and a browser user agent string for web app requests.
Previously, we only checked mandatory_topics setting before
sending message in frontend and there was no restriction in
backend. This commit adds the check in backend also making
sure messages without topic cannot be sent through API as
well if mandatory_topics setting is set to True.
Under the unicodedata distributed with Python 3.6, some Emoji are
classified as `Cn`, and not `So`:
```
$ unicode 1f929 --long
U+1F929 GRINNING FACE WITH STAR EYES
UTF-8: f0 9f a4 a9 UTF-16BE: d83edd29 Decimal: 🤩 Octal: \0374451
🤩
Category: So (Symbol, Other); East Asian width: W (wide)
Unicode block: 1F900..1F9FF; Supplemental Symbols and Pictographs
Bidi: ON (Other Neutrals)
$ python3.6 -c 'import unicodedata; print(unicodedata.category("\U0001f929"))'
Cn
$ python3.7 -c 'import unicodedata; print(unicodedata.category("\U0001f929"))'
So
```
Drop `Cn` from the list of excluded Unicode character classes, and
replace it with an explicit list of the 66 non-characters, which are
invariant.
Co-authored-by: Shlok Patel <shlokcpatel2001@gmail.com>
An explanatory note on the changes in zulip.yaml and
curl_param_value_generators is warranted here. In our automated
tests for our curl examples, the test for the API endpoint that
changes the posting permissions of a stream comes before our
existing curl test for adding message reactions.
Since there is an extra notification message due to the change in
posting permissions, the message IDs used in tests that come after
need to be incremented by 1.
This is a part of #20289.
Special characters, including `\r`, `\n`, and more esoteric codepoints
like non-characters, can negatively affect rendering and UI behaviour.
Check for, and prevent making new messages with, characters in the
Unicode categories of `Cc` (control characters), `Cs`, (surrogates),
and `Cn` (unassigned, non-characters).
Fixes#20128.
The reason for this bug is because of different striping
processes in the backend and frontend, i.e The frontend
checks if the message's `raw_content` has changed to
decide if the `content` of the message should be sent in
the request to the backend, or not. So, it removes the
leading new line ('\n') from the message `raw_content`
when checking it, which is causing the "Error saving edit:
You don't have permission to edit this message" error.
This commit fixes it by removing the leading new line
when cleaning message content.
The bug was explained by @punchagan and its solution
by @timabbott.
This will offer users who are self-hosting to adjust
this value. Moreover, this will help to reduce the
overall time taken to test `test_markdown.py` (since
this can be now overridden with `override_settings`
Django decorator).
This is done as a prep commit for #18641.
This commit modifies the test_wildcard_mention_restrictions test
for checking that moderators are not allowed to send messages
with wildcard mention if wildcard_mention_policy is set to
WILDCARD_MENTION_POLICY_ADMINS. Previously, we were checking
for members, but it is better to check for moderators.
A bug in the implementation of the can_forge_sender permission
(previously is_api_super_user) resulted in users with this permission
being able to send messages appearing as if sent by a system bots,
including to other organizations hosted by the same Zulip installation.
- The send message API had a bug allowing an api super user to
use forging to send messages to other realms' streams, as a
cross-realm bot. We fix this most directly by eliminating the
realm_str parameter - it is not necessary for any valid current use
case. The email gateway doesn't use this API despite the comment in
that block suggesting otherwise.
- The conditionals inside access_stream_for_send_message are changed up
to improve security. They were generally not ordered very well,
allowing the function to successfully return due to very weak
acceptance conditions - skipping the higher importance checks that
should lead to raising an error.
- The query count in test_subs is decreased because
access_stream_for_send_message returns earlier when doing its check
for a cross-realm bot sender - some subscription checking queries are
skipped.
- A linkifier test in test_message_dict needs to be changed. It didn't
make much sense in the first place, because it was creating a message
by a normal user, to a stream outside of the user's realm. That
shouldn't even be allowed.
Anders Kaseorg
Lauryn Menard
Sahil Batra
Mateusz Mandera
Alex Vandiver
Zixuan James Li
Zixuan James Li
Lauryn Menard
Eeshan Garg
Shlok Patel
Riken Shah
akshatdalton
Abhijeet Prasad Bodas
sahil839