request: Store client information using ZulipRequestNotes.

This concludes the HttpRequest migration to eliminate arbitrary
attributes (except private ones that are belong to django) attached
to the request object during runtime and migrated them to a
separate data structure dedicated for the purpose of adding
information (so called notes) to a HttpRequest.

zerver/context_processors.py

@@ -170,7 +170,7 @@ def zulip_default_context(request: HttpRequest) -> Dict[str, Any]:
         "settings_path": settings_path,
         "secrets_path": secrets_path,
         "settings_comments_path": settings_comments_path,
-        "platform": request.client_name,
+        "platform": get_request_notes(request).client_name,
         "allow_search_engine_indexing": allow_search_engine_indexing,
         "landing_page_navbar_message": settings.LANDING_PAGE_NAVBAR_MESSAGE,
         "default_page_params": default_page_params,

zerver/decorator.py

@@ -92,11 +92,12 @@ def update_user_activity(
     else:
         query = request.META["PATH_INFO"]
 
+    assert request_notes.client is not None
     event = {
         "query": query,
         "user_profile_id": user_profile.id,
         "time": datetime_to_timestamp(timezone_now()),
-        "client_id": request.client.id,
+        "client_id": request_notes.client.id,
     }
     queue_json_publish("user_activity", event, lambda event: None)
 
@@ -181,8 +182,11 @@ def process_client(
     skip_update_user_activity: bool = False,
     query: Optional[str] = None,
 ) -> None:
+    request_notes = get_request_notes(request)
     if client_name is None:
-        client_name = request.client_name
+        client_name = request_notes.client_name
+
+    assert client_name is not None
 
     # We could check for a browser's name being "Mozilla", but
     # e.g. Opera and MobileSafari don't set that, and it seems
@@ -192,7 +196,7 @@ def process_client(
         # the Zulip desktop apps be themselves.
         client_name = "website"
 
-    request.client = get_client(client_name)
+    request_notes.client = get_client(client_name)
     if not skip_update_user_activity and user_profile.is_authenticated:
         update_user_activity(request, user_profile, query)
 
@@ -788,7 +792,8 @@ def client_is_exempt_from_rate_limiting(request: HttpRequest) -> bool:
 
     # Don't rate limit requests from Django that come from our own servers,
     # and don't rate-limit dev instances
-    return (request.client and request.client.name.lower() == "internal") and (
+    client = get_request_notes(request).client
+    return (client is not None and client.name.lower() == "internal") and (
         is_local_addr(request.META["REMOTE_ADDR"]) or settings.DEBUG_RATE_LIMITING
     )
 

zerver/lib/home.py

@@ -16,6 +16,7 @@ from zerver.lib.i18n import (
     get_language_list,
     get_language_translation_data,
 )
+from zerver.lib.request import get_request_notes
 from zerver.models import Message, Realm, Stream, UserProfile
 from zerver.views.message_flags import get_latest_update_message_flag_activity
 
@@ -138,9 +139,11 @@ def build_page_params_for_home_page_load(
     }
 
     if user_profile is not None:
+        client = get_request_notes(request).client
+        assert client is not None
         register_ret = do_events_register(
             user_profile,
-            request.client,
+            client,
             apply_markdown=True,
             client_gravatar=True,
             slim_presence=True,

zerver/lib/logging_util.py

@@ -276,9 +276,13 @@ class ZulipWebhookFormatter(ZulipFormatter):
                 )
 
         header_message = header_text if header_text else None
+        from zerver.lib.request import get_request_notes
+
+        client = get_request_notes(request).client
+        assert client is not None
 
         setattr(record, "user", f"{request.user.delivery_email} ({request.user.realm.string_id})")
-        setattr(record, "client", request.client.name)
+        setattr(record, "client", client.name)
         setattr(record, "url", request.META.get("PATH_INFO", None))
         setattr(record, "content_type", request.content_type)
         setattr(record, "custom_headers", header_message)

zerver/lib/test_helpers.py

@@ -300,8 +300,6 @@ class HostRequestMock(HttpRequest):
         self.host = host
         self.GET = QueryDict(mutable=True)
         self.method = ""
-        if client_name is not None:
-            self.client = get_client(client_name)
 
         # Convert any integer parameters passed into strings, even
         # though of course the HTTP API would do so.  Ideally, we'd
@@ -322,11 +320,12 @@ class HostRequestMock(HttpRequest):
         self.user = user_profile
         self._body = b""
         self.content_type = ""
-        self.client_name = ""
 
         request_notes_map[self] = ZulipRequestNotes(
+            client_name="",
             log_data={},
             tornado_handler=tornado_handler,
+            client=get_client(client_name) if client_name is not None else None,
         )
 
     @property

zerver/lib/webhooks/common.py

@@ -14,7 +14,7 @@ from zerver.lib.actions import (
     send_rate_limited_pm_notification_to_bot_owner,
 )
 from zerver.lib.exceptions import ErrorCode, JsonableError, StreamDoesNotExistError
-from zerver.lib.request import REQ, has_request_variables
+from zerver.lib.request import REQ, get_request_notes, has_request_variables
 from zerver.lib.send_email import FromAddress
 from zerver.lib.timestamp import timestamp_to_datetime
 from zerver.lib.validator import check_list, check_string
@@ -107,9 +107,11 @@ def check_send_webhook_message(
         ):
             return
 
+    client = get_request_notes(request).client
+    assert client is not None
     if stream is None:
         assert user_profile.bot_owner is not None
-        check_send_private_message(user_profile, request.client, user_profile.bot_owner, body)
+        check_send_private_message(user_profile, client, user_profile.bot_owner, body)
     else:
         # Some third-party websites (such as Atlassian's Jira), tend to
         # double escape their URLs in a manner that escaped space characters
@@ -125,11 +127,9 @@ def check_send_webhook_message(
 
         try:
             if stream.isdecimal():
-                check_send_stream_message_by_id(
-                    user_profile, request.client, int(stream), topic, body
-                )
+                check_send_stream_message_by_id(user_profile, client, int(stream), topic, body)
             else:
-                check_send_stream_message(user_profile, request.client, stream, topic, body)
+                check_send_stream_message(user_profile, client, stream, topic, body)
         except StreamDoesNotExistError:
             # A PM will be sent to the bot_owner by check_message, notifying
             # that the webhook bot just tried to send a message to a non-existent

zerver/middleware.py

@@ -351,7 +351,7 @@ class LogRequests(MiddlewareMixin):
             # Avoid re-initializing request_notes.log_data if it's already there.
             return
 
-        request.client_name, request.client_version = parse_client(request)
+        request_notes.client_name, request_notes.client_version = parse_client(request)
         request_notes.log_data = {}
         record_request_start_data(request_notes.log_data)
 
@@ -401,10 +401,6 @@ class LogRequests(MiddlewareMixin):
                 requestor_for_logs = request.user.format_requestor_for_logs()
             else:
                 requestor_for_logs = "unauth@{}".format(get_subdomain(request) or "root")
-        try:
-            client = request.client.name
-        except Exception:
-            client = request.client_name
 
         if response.streaming:
             content_iter = response.streaming_content
@@ -413,15 +409,15 @@ class LogRequests(MiddlewareMixin):
             content = response.content
             content_iter = None
 
-        assert request_notes.log_data is not None
+        assert request_notes.client_name is not None and request_notes.log_data is not None
         write_log_line(
             request_notes.log_data,
             request.path,
             request.method,
             remote_ip,
             requestor_for_logs,
-            client,
-            client_version=request.client_version,
+            request_notes.client_name,
+            client_version=request_notes.client_version,
             status_code=response.status_code,
             error_content=content,
             error_content_iter=content_iter,

zerver/tornado/handlers.py

@@ -243,9 +243,9 @@ class AsyncDjangoHandler(tornado.web.RequestHandler, base.BaseHandler):
         if request_notes.requestor_for_logs is not None:
             request_notes.requestor_for_logs = old_request_notes.requestor_for_logs
         request.user = old_request.user
-        request.client = old_request.client
-        request.client_name = old_request.client_name
-        request.client_version = old_request.client_version
+        request_notes.client = old_request_notes.client
+        request_notes.client_name = old_request_notes.client_name
+        request_notes.client_version = old_request_notes.client_version
 
         # The saved_response attribute, if present, causes
         # rest_dispatch to return the response immediately before

zerver/tornado/views.py

@@ -114,7 +114,8 @@ def get_events_backend(
     handler: AsyncDjangoHandler = tornado_handler
 
     if user_client is None:
-        valid_user_client = request.client
+        valid_user_client = get_request_notes(request).client
+        assert valid_user_client is not None
     else:
         valid_user_client = user_client
 

zerver/views/events_register.py

@@ -5,7 +5,7 @@ from django.utils.translation import gettext as _
 
 from zerver.lib.events import do_events_register
 from zerver.lib.exceptions import JsonableError
-from zerver.lib.request import REQ, has_request_variables
+from zerver.lib.request import REQ, get_request_notes, has_request_variables
 from zerver.lib.response import json_success
 from zerver.lib.validator import check_bool, check_dict, check_list, check_string
 from zerver.models import Stream, UserProfile
@@ -79,9 +79,12 @@ def events_register_backend(
     if client_capabilities is None:
         client_capabilities = {}
 
+    client = get_request_notes(request).client
+    assert client is not None
+
     ret = do_events_register(
         user_profile,
-        request.client,
+        client,
         apply_markdown,
         client_gravatar,
         slim_presence,

zerver/views/message_flags.py

@@ -37,9 +37,10 @@ def update_message_flags(
     flag: str = REQ(),
 ) -> HttpResponse:
     request_notes = get_request_notes(request)
+    assert request_notes.client is not None
     assert request_notes.log_data is not None
 
-    count = do_update_message_flags(user_profile, request.client, operation, flag, messages)
+    count = do_update_message_flags(user_profile, request_notes.client, operation, flag, messages)
 
     target_count_str = str(len(messages))
     log_data_str = f"[{operation} {flag}/{target_count_str}] actually {count}"
@@ -50,8 +51,9 @@ def update_message_flags(
 
 @has_request_variables
 def mark_all_as_read(request: HttpRequest, user_profile: UserProfile) -> HttpResponse:
-    count = do_mark_all_as_read(user_profile, request.client)
     request_notes = get_request_notes(request)
+    assert request_notes.client is not None
+    count = do_mark_all_as_read(user_profile, request_notes.client)
 
     log_data_str = f"[{count} updated]"
     assert request_notes.log_data is not None

zerver/views/message_send.py

@@ -20,6 +20,7 @@ from zerver.lib.actions import (
 )
 from zerver.lib.exceptions import JsonableError
 from zerver.lib.message import render_markdown
+from zerver.lib.request import get_request_notes
 from zerver.lib.response import json_success
 from zerver.lib.timestamp import convert_to_UTC
 from zerver.lib.topic import REQ_topic
@@ -52,13 +53,16 @@ def create_mirrored_message_users(
         for email in recipients:
             referenced_users.add(email.lower())
 
-    if request.client.name == "zephyr_mirror":
+    client = get_request_notes(request).client
+    assert client is not None
+
+    if client.name == "zephyr_mirror":
         user_check = same_realm_zephyr_user
         fullname_function = compute_mit_user_fullname
-    elif request.client.name == "irc_mirror":
+    elif client.name == "irc_mirror":
         user_check = same_realm_irc_user
         fullname_function = compute_irc_user_fullname
-    elif request.client.name in ("jabber_mirror", "JabberMirror"):
+    elif client.name in ("jabber_mirror", "JabberMirror"):
         user_check = same_realm_jabber_user
         fullname_function = compute_jabber_user_fullname
     else:
@@ -221,7 +225,8 @@ def send_message_backend(
     # `yes` to accepting `true` like all of our normal booleans.
     forged = forged_str is not None and forged_str in ["yes", "true"]
 
-    client = request.client
+    client = get_request_notes(request).client
+    assert client is not None
     can_forge_sender = request.user.can_forge_sender
     if forged and not can_forge_sender:
         raise JsonableError(_("User not authorized for this query"))
@@ -324,7 +329,9 @@ def render_message_backend(
     message = Message()
     message.sender = user_profile
     message.content = content
-    message.sending_client = request.client
+    client = get_request_notes(request).client
+    assert client is not None
+    message.sending_client = client
 
     rendering_result = render_markdown(message, content, realm=user_profile.realm)
     return json_success({"rendered": rendering_result.rendered_content})

zerver/views/presence.py

@@ -9,7 +9,7 @@ from django.utils.translation import gettext as _
 from zerver.decorator import human_users_only
 from zerver.lib.actions import do_update_user_status, update_user_presence
 from zerver.lib.presence import get_presence_for_user, get_presence_response
-from zerver.lib.request import REQ, JsonableError, has_request_variables
+from zerver.lib.request import REQ, JsonableError, get_request_notes, has_request_variables
 from zerver.lib.response import json_success
 from zerver.lib.timestamp import datetime_to_timestamp
 from zerver.lib.validator import check_bool, check_capped_string
@@ -75,11 +75,13 @@ def update_user_status_backend(
     if (away is None) and (status_text is None):
         raise JsonableError(_("Client did not pass any new values."))
 
+    client = get_request_notes(request).client
+    assert client is not None
     do_update_user_status(
         user_profile=user_profile,
         away=away,
         status_text=status_text,
-        client_id=request.client.id,
+        client_id=client.id,
     )
 
     return json_success()
@@ -99,9 +101,9 @@ def update_active_status_backend(
     if status_val is None:
         raise JsonableError(_("Invalid status: {}").format(status))
     elif user_profile.presence_enabled:
-        update_user_presence(
-            user_profile, request.client, timezone_now(), status_val, new_user_input
-        )
+        client = get_request_notes(request).client
+        assert client is not None
+        update_user_presence(user_profile, client, timezone_now(), status_val, new_user_input)
 
     if ping_only:
         ret: Dict[str, Any] = {}

zerver/views/streams.py

@@ -50,7 +50,7 @@ from zerver.lib.exceptions import (
     OrganizationOwnerRequired,
     ResourceNotFoundError,
 )
-from zerver.lib.request import REQ, has_request_variables
+from zerver.lib.request import REQ, get_request_notes, has_request_variables
 from zerver.lib.response import json_success
 from zerver.lib.retention import parse_message_retention_days
 from zerver.lib.streams import (
@@ -404,8 +404,10 @@ def remove_subscriptions_backend(
         people_to_unsub = {user_profile}
 
     result: Dict[str, List[str]] = dict(removed=[], not_removed=[])
+    client = get_request_notes(request).client
+    assert client is not None
     (removed, not_subscribed) = bulk_remove_subscriptions(
-        people_to_unsub, streams, request.client, acting_user=user_profile
+        people_to_unsub, streams, client, acting_user=user_profile
     )
 
     for (subscriber, removed_stream) in removed:

zerver/webhooks/dialogflow/view.py

@@ -5,7 +5,7 @@ from django.http import HttpRequest, HttpResponse
 
 from zerver.decorator import webhook_view
 from zerver.lib.actions import check_send_private_message
-from zerver.lib.request import REQ, has_request_variables
+from zerver.lib.request import REQ, get_request_notes, has_request_variables
 from zerver.lib.response import json_success
 from zerver.models import UserProfile, get_user
 
@@ -35,5 +35,7 @@ def api_dialogflow_webhook(
         body = f"{status} - {error_status}"
 
     receiving_user = get_user(email, user_profile.realm)
-    check_send_private_message(user_profile, request.client, receiving_user, body)
+    client = get_request_notes(request).client
+    assert client is not None
+    check_send_private_message(user_profile, client, receiving_user, body)
     return json_success()

zerver/webhooks/slack/view.py

@@ -4,7 +4,7 @@ from django.utils.translation import gettext as _
 from zerver.decorator import webhook_view
 from zerver.lib.actions import check_send_stream_message
 from zerver.lib.exceptions import JsonableError
-from zerver.lib.request import REQ, has_request_variables
+from zerver.lib.request import REQ, get_request_notes, has_request_variables
 from zerver.lib.response import json_success
 from zerver.models import UserProfile
 
@@ -34,5 +34,7 @@ def api_slack_webhook(
         subject = _("Message from Slack")
 
     content = ZULIP_MESSAGE_TEMPLATE.format(message_sender=user_name, text=text)
-    check_send_stream_message(user_profile, request.client, stream, subject, content)
+    client = get_request_notes(request).client
+    assert client is not None
+    check_send_stream_message(user_profile, client, stream, subject, content)
     return json_success()

zerver/webhooks/teamcity/view.py

@@ -10,7 +10,7 @@ from zerver.lib.actions import (
     check_send_private_message,
     send_rate_limited_pm_notification_to_bot_owner,
 )
-from zerver.lib.request import REQ, has_request_variables
+from zerver.lib.request import REQ, get_request_notes, has_request_variables
 from zerver.lib.response import json_success
 from zerver.lib.send_email import FromAddress
 from zerver.lib.webhooks.common import check_send_webhook_message
@@ -137,7 +137,9 @@ def api_teamcity_webhook(
             return json_success()
 
         body = f"Your personal build for {body}"
-        check_send_private_message(user_profile, request.client, teamcity_user, body)
+        client = get_request_notes(request).client
+        assert client is not None
+        check_send_private_message(user_profile, client, teamcity_user, body)
 
         return json_success()
 

zerver/webhooks/yo/view.py

@@ -5,7 +5,7 @@ from django.http import HttpRequest, HttpResponse
 
 from zerver.decorator import webhook_view
 from zerver.lib.actions import check_send_private_message
-from zerver.lib.request import REQ, has_request_variables
+from zerver.lib.request import REQ, get_request_notes, has_request_variables
 from zerver.lib.response import json_success
 from zerver.models import UserProfile, get_user
 
@@ -22,5 +22,7 @@ def api_yo_app_webhook(
 ) -> HttpResponse:
     body = f"Yo from {username}"
     receiving_user = get_user(email, user_profile.realm)
-    check_send_private_message(user_profile, request.client, receiving_user, body)
+    client = get_request_notes(request).client
+    assert client is not None
+    check_send_private_message(user_profile, client, receiving_user, body)
     return json_success()

zproject/backends.py

@@ -278,7 +278,9 @@ def rate_limit_auth(auth_func: AuthFuncT, *args: Any, **kwargs: Any) -> Optional
 
     request = args[1]
     username = kwargs["username"]
-    if not hasattr(request, "client") or not client_is_exempt_from_rate_limiting(request):
+    if get_request_notes(request).client is None or not client_is_exempt_from_rate_limiting(
+        request
+    ):
         # Django cycles through enabled authentication backends until one succeeds,
         # or all of them fail. If multiple backends are tried like this, we only want
         # to execute rate_limit_authentication_* once, on the first attempt:

zproject/sentry.py

@@ -52,8 +52,8 @@ def add_context(event: "Event", hint: "Hint") -> Optional["Event"]:
         request = get_current_request()
         if request:
             request_notes = get_request_notes(request)
-            if hasattr(request, "client"):
-                event["tags"]["client"] = request.client.name
+            if request_notes.client is not None:
+                event["tags"]["client"] = request_notes.client.name
             if request_notes.realm is not None:
                 event["tags"].setdefault("realm", request_notes.realm.string_id)
     return event