outgoing_http: Use OutgoingSession subclasses in more places.

This adds the X-Smokescreen-Role header to proxy connections, to track
usage from various codepaths, and enforces a timeout.  Timeouts were
kept consistent with their previous values, or set to 5s if they had
none previously.

tools/fetch-contributor-data

@@ -28,6 +28,7 @@ from typing_extensions import TypedDict
 django.setup()
 
 from zerver.lib.avatar_hash import gravatar_hash
+from zerver.lib.github import GithubSession
 from zproject.config import get_secret
 
 duplicate_commits_file = os.path.join(os.path.dirname(__file__), "duplicate_commits.json")
@@ -69,9 +70,13 @@ def fetch_contributors(repo_name: str, max_retries: int) -> List[Contributor]:
     if personal_access_token is not None:
         headers = {"Authorization": f"token {personal_access_token}"}
 
+    session = GithubSession()
     while True:
-        response: requests.Response = requests.get(
+        response: requests.Response = session.get(
-            api_link, {**api_data, "page": f"{page_index}"}, verify=certificates, headers=headers
+            api_link,
+            params={**api_data, "page": f"{page_index}"},
+            verify=certificates,
+            headers=headers,
         )
         if response.status_code == 200:
             data = response.json()

zerver/lib/github.py

@@ -4,14 +4,20 @@ import logging
 import requests
 
 from zerver.lib.cache import cache_with_key
+from zerver.lib.outgoing_http import OutgoingSession
 
 logger = logging.getLogger(__name__)
 
 
+class GithubSession(OutgoingSession):
+    def __init__(self) -> None:
+        super().__init__(role="github", timeout=5)
+
+
 def get_latest_github_release_version_for_repo(repo: str) -> str:
     api_url = f"https://api.github.com/repos/zulip/{repo}/releases/latest"
     try:
-        return requests.get(api_url).json()["tag_name"]
+        return GithubSession().get(api_url).json()["tag_name"]
     except (requests.RequestException, json.JSONDecodeError, KeyError):
         logger.exception(
             "Unable to fetch the latest release version from GitHub %s", api_url, stack_info=True
@@ -21,7 +27,7 @@ def get_latest_github_release_version_for_repo(repo: str) -> str:
 
 def verify_release_download_link(link: str) -> bool:
     try:
-        requests.head(link).raise_for_status()
+        GithubSession().head(link).raise_for_status()
         return True
     except requests.RequestException:
         logger.error("App download link is broken %s", link)

zerver/lib/markdown/__init__.py

@@ -53,6 +53,7 @@ from zerver.lib.exceptions import MarkdownRenderingException
 from zerver.lib.markdown import fenced_code
 from zerver.lib.markdown.fenced_code import FENCE_RE
 from zerver.lib.mention import MentionData, get_stream_name_info
+from zerver.lib.outgoing_http import OutgoingSession
 from zerver.lib.subdomains import is_static_or_current_realm_url
 from zerver.lib.tex import render_tex
 from zerver.lib.thumbnail import user_uploads_or_external
@@ -468,12 +469,17 @@ def fetch_tweet_data(tweet_id: str) -> Optional[Dict[str, Any]]:
     return res
 
 
+class OpenGraphSession(OutgoingSession):
+    def __init__(self) -> None:
+        super().__init__(role="markdown", timeout=1)
+
+
 def fetch_open_graph_image(url: str) -> Optional[Dict[str, Any]]:
     og = {"image": None, "title": None, "desc": None}
 
     try:
-        with requests.get(
+        with OpenGraphSession().get(
-            url, headers={"Accept": "text/html,application/xhtml+xml"}, stream=True, timeout=1
+            url, headers={"Accept": "text/html,application/xhtml+xml"}, stream=True
         ) as res:
             if res.status_code != requests.codes.ok:
                 return None

zerver/lib/remote_server.py

@@ -12,9 +12,15 @@ from analytics.models import InstallationCount, RealmCount
 from version import ZULIP_VERSION
 from zerver.lib.exceptions import JsonableError
 from zerver.lib.export import floatify_datetime_fields
+from zerver.lib.outgoing_http import OutgoingSession
 from zerver.models import RealmAuditLog
 
 
+class PushBouncerSession(OutgoingSession):
+    def __init__(self) -> None:
+        super().__init__(role="push_bouncer", timeout=30)
+
+
 class PushNotificationBouncerException(Exception):
     pass
 
@@ -55,8 +61,8 @@ def send_to_push_bouncer(
     headers.update(extra_headers)
 
     try:
-        res = requests.request(
+        res = PushBouncerSession().request(
-            method, url, data=post_data, auth=api_auth, timeout=30, verify=True, headers=headers
+            method, url, data=post_data, auth=api_auth, verify=True, headers=headers
         )
     except (
         requests.exceptions.Timeout,

zerver/lib/url_preview/oembed.py

@@ -1,6 +1,7 @@
 import json
 from typing import Any, Dict, Optional
 
+import requests
 from pyoembed import PyOembedException, oEmbed
 
 
@@ -9,7 +10,7 @@ def get_oembed_data(
 ) -> Optional[Dict[str, Any]]:
     try:
         data = oEmbed(url, maxwidth=maxwidth, maxheight=maxheight)
-    except (PyOembedException, json.decoder.JSONDecodeError):
+    except (PyOembedException, json.decoder.JSONDecodeError, requests.exceptions.ConnectionError):
         return None
 
     oembed_resource_type = data.get("type", "")

zerver/lib/url_preview/preview.py

@@ -8,6 +8,7 @@ from django.utils.encoding import smart_str
 
 from version import ZULIP_VERSION
 from zerver.lib.cache import cache_with_key, get_cache_with_key, preview_url_cache_key
+from zerver.lib.outgoing_http import OutgoingSession
 from zerver.lib.pysa import mark_sanitized
 from zerver.lib.url_preview.oembed import get_oembed_data
 from zerver.lib.url_preview.parsers import GenericParser, OpenGraphParser
@@ -36,6 +37,11 @@ HEADERS = {"User-Agent": ZULIP_URL_PREVIEW_USER_AGENT}
 TIMEOUT = 15
 
 
+class PreviewSession(OutgoingSession):
+    def __init__(self) -> None:
+        super().__init__(role="preview", timeout=TIMEOUT, headers=HEADERS)
+
+
 def is_link(url: str) -> Optional[Match[str]]:
     return link_regex.match(smart_str(url))
 
@@ -51,7 +57,7 @@ def guess_mimetype_from_content(response: requests.Response) -> str:
 
 def valid_content_type(url: str) -> bool:
     try:
-        response = requests.get(url, stream=True, headers=HEADERS, timeout=TIMEOUT)
+        response = PreviewSession().get(url, stream=True)
     except requests.RequestException:
         return False
 
@@ -94,7 +100,7 @@ def get_link_embed_data(
     if data.get("oembed"):
         return data
 
-    response = requests.get(mark_sanitized(url), stream=True, headers=HEADERS, timeout=TIMEOUT)
+    response = PreviewSession().get(mark_sanitized(url), stream=True)
     if response.ok:
         og_data = OpenGraphParser(
             response.content, response.headers.get("Content-Type")

zerver/management/commands/register_server.py

@@ -8,6 +8,7 @@ from django.core.management.base import CommandError
 from django.utils.crypto import get_random_string
 
 from zerver.lib.management import ZulipBaseCommand, check_config
+from zerver.lib.remote_server import PushBouncerSession
 
 if settings.DEVELOPMENT:
     SECRETS_FILENAME = "zproject/dev-secrets.conf"
@@ -82,8 +83,9 @@ class Command(ZulipBaseCommand):
         registration_url = (
             settings.PUSH_NOTIFICATION_BOUNCER_URL + "/api/v1/remotes/server/register"
         )
+        session = PushBouncerSession()
         try:
-            response = requests.post(registration_url, params=request)
+            response = session.post(registration_url, params=request)
         except requests.RequestException:
             raise CommandError(
                 "Network error connecting to push notifications service "

zerver/tests/test_link_embed.py

@@ -95,6 +95,7 @@ class OembedTestCase(ZulipTestCase):
             "width": 658,
             "height": 400,
         }
+        # pyoembed.providers.imgur only works with http:// URLs, not https:// (!)
         url = "http://imgur.com/photo/158727223"
         reconstructed_url = reconstruct_url(url)
         responses.add(
@@ -140,13 +141,29 @@ class OembedTestCase(ZulipTestCase):
         self.assertEqual(data["title"], response_data["title"])
 
     @responses.activate
-    def test_error_request(self) -> None:
+    def test_connect_error_request(self) -> None:
+        url = "http://instagram.com/p/BLtI2WdAymy"
+        reconstructed_url = reconstruct_url(url)
+        responses.add(responses.GET, reconstructed_url, body=ConnectionError())
+        data = get_oembed_data(url)
+        self.assertIsNone(data)
+
+    @responses.activate
+    def test_400_error_request(self) -> None:
         url = "http://instagram.com/p/BLtI2WdAymy"
         reconstructed_url = reconstruct_url(url)
         responses.add(responses.GET, reconstructed_url, status=400)
         data = get_oembed_data(url)
         self.assertIsNone(data)
 
+    @responses.activate
+    def test_500_error_request(self) -> None:
+        url = "http://instagram.com/p/BLtI2WdAymy"
+        reconstructed_url = reconstruct_url(url)
+        responses.add(responses.GET, reconstructed_url, status=500)
+        data = get_oembed_data(url)
+        self.assertIsNone(data)
+
     @responses.activate
     def test_invalid_json_in_response(self) -> None:
         url = "http://instagram.com/p/BLtI2WdAymy"

zerver/views/video_calls.py

@@ -24,6 +24,7 @@ from requests_oauthlib import OAuth2Session
 from zerver.decorator import zulip_login_required
 from zerver.lib.actions import do_set_zoom_token
 from zerver.lib.exceptions import ErrorCode, JsonableError
+from zerver.lib.outgoing_http import OutgoingSession
 from zerver.lib.pysa import mark_sanitized
 from zerver.lib.request import REQ, has_request_variables
 from zerver.lib.response import json_success
@@ -33,6 +34,11 @@ from zerver.lib.validator import check_dict, check_string
 from zerver.models import UserProfile, get_realm
 
 
+class VideoCallSession(OutgoingSession):
+    def __init__(self) -> None:
+        super().__init__(role="video_calls", timeout=5)
+
+
 class InvalidZoomTokenError(JsonableError):
     code = ErrorCode.INVALID_ZOOM_TOKEN
 
@@ -218,7 +224,7 @@ def join_bigbluebutton(
         raise JsonableError(_("BigBlueButton is not configured."))
     else:
         try:
-            response = requests.get(
+            response = VideoCallSession().get(
                 add_query_to_redirect_url(
                     settings.BIG_BLUE_BUTTON_URL + "api/create",
                     urlencode(