Skip to content

Commit

Permalink
Replace requests with httpx
Browse files Browse the repository at this point in the history
Fixes a security issue with requests, and makes sure we're using only
one HTTP library.

Untested (except for mypy and tests), will do that later when all the
package updates have been merged.

Signed-off-by: Tim Weber <scy@scy.name>
  • Loading branch information
scy committed Dec 19, 2024
1 parent ef58acf commit c69d30a
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 83 deletions.
38 changes: 22 additions & 16 deletions server/dearmep/http_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from typing import Literal, Optional, Union

import backoff
import requests
import httpx
from ratelimit import RateLimitException, limits # type: ignore[import]

from . import __version__
Expand All @@ -23,29 +23,31 @@
DEFAULT_MASS_DOWNLOAD_JOBS = 3


def new_session() -> requests.Session:
s = requests.Session()
s.headers.update(
{
def new_session() -> httpx.Client:
return httpx.Client(
headers={
"User-Agent": f"{APP_NAME} {__version__}",
}
)
return s


def session_or_new(session: Optional[requests.Session]) -> requests.Session:
def session_or_new(session: Optional[httpx.Client]) -> httpx.Client:
return session or new_session()


def _permanent_download_error(e: Exception) -> bool:
"""Whether a HTTP error is considered permanent & retrying should stop."""
if isinstance(e, RateLimitException):
return False
if not isinstance(e, requests.exceptions.HTTPError):
if not isinstance(e, httpx.HTTPError):
return True
req: Optional[requests.Request] = e.request
res: Optional[requests.Response] = e.response
url = req.url if req else "[unknown URL]"
req = (
e.request
if isinstance(e, (httpx.RequestError, httpx.HTTPStatusError))
else None
)
res = e.response if isinstance(e, httpx.HTTPStatusError) else None
url = str(req.url) if req else "[unknown URL]"
if res is None:
_logger.warning(
f"downloading {url} failed without a response, will retry"
Expand All @@ -65,7 +67,7 @@ def __init__( # noqa: PLR0913
self,
*,
jobs: int = DEFAULT_MASS_DOWNLOAD_JOBS,
session: Optional[requests.Session] = None,
session: Optional[httpx.Client] = None,
task: Optional[BaseTask] = None,
overwrite: bool = False,
skip_existing: bool = False,
Expand All @@ -88,11 +90,11 @@ def __init__( # noqa: PLR0913
self._mgmt_thread: Optional[Thread] = None
self._should_run: bool = False
self._abort: bool = False
self.errors: list[tuple[str, requests.Response]] = []
self.errors: list[tuple[str, httpx.Response]] = []

@backoff.on_exception(
backoff.expo,
(requests.exceptions.HTTPError, RateLimitException),
(httpx.HTTPError, RateLimitException),
giveup=_permanent_download_error,
max_time=120,
)
Expand Down Expand Up @@ -137,8 +139,8 @@ def _queue_worker(self) -> None:
continue
try:
self._download(url, dest)
except requests.exceptions.HTTPError as e:
if e.response is not None and (
except httpx.HTTPStatusError as e:
if (
self._ignore_codes is True
or e.response.status_code in self._ignore_codes
):
Expand All @@ -147,6 +149,10 @@ def _queue_worker(self) -> None:
self._abort = True
self._should_run = False
raise
except httpx.HTTPError:
self._abort = True
self._should_run = False
raise
except BaseException:
self._abort = True
self._should_run = False
Expand Down
10 changes: 5 additions & 5 deletions server/dearmep/phone/elks/elks.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from datetime import datetime, timedelta, timezone
from typing import Any, Literal, Optional, Union

import requests
import httpx
from fastapi import (
APIRouter,
Depends,
Expand Down Expand Up @@ -67,7 +67,7 @@ def send_sms(
provider_cfg.username,
provider_cfg.password,
)
response = requests.post(
response = httpx.post(
url="https://api.46elks.com/a1/sms",
timeout=10,
auth=auth,
Expand All @@ -77,7 +77,7 @@ def send_sms(
"message": message,
},
)
if not response.ok:
if not response.is_success:
_logger.critical(
f"46elks request to send sms failed: {response.status_code}"
)
Expand Down Expand Up @@ -123,7 +123,7 @@ def start_elks_call(
phone_numbers=phone_numbers,
)

response = requests.post(
response = httpx.post(
url="https://api.46elks.com/a1/calls",
timeout=10,
auth=auth,
Expand All @@ -136,7 +136,7 @@ def start_elks_call(
},
)

if not response.ok:
if not response.is_success:
_logger.critical(
f"46elks request to start call failed: {response.status_code}"
)
Expand Down
4 changes: 2 additions & 2 deletions server/dearmep/phone/elks/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import logging
from random import choice

import requests
import httpx

from ...config import Language
from .models import Number
Expand Down Expand Up @@ -49,7 +49,7 @@ def get_numbers(
Fetches all available numbers of an account at 46elks.
"""

response = requests.get(
response = httpx.get(
url="https://api.46elks.com/a1/numbers", timeout=10, auth=auth
)
if response.status_code != 200: # noqa: PLR2004
Expand Down
2 changes: 0 additions & 2 deletions server/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ dependencies = [
"pytz~=2023.3.post1",
"pyyaml~=6.0",
"ratelimit~=2.2.1",
"requests~=2.31",
"rich~=13.4.2",
"sqlmodel<0.0.11",
"starlette-exporter~=0.15.1",
Expand All @@ -59,7 +58,6 @@ dev = [
"types-pillow~=9.5.0.5",
"types-pytz~=2023.3.1.1",
"types-pyyaml~=6.0.12.2",
"types-requests~=2.31.0.1",
]

[project.scripts]
Expand Down
58 changes: 0 additions & 58 deletions server/uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit c69d30a

Please sign in to comment.