Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

[BUG] Missing timeouts on 5 outbound HTTP calls outside mod_ci (follow-up to #943)

Aperta
#1,192 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
84/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
python

Direzione di ricerca

Inizia con i cinque punti di chiamata elencati in utility.py, mod_auth/controllers.py, mailer.py e mod_upload/controllers.py, quindi confrontali con le costanti di timeout in mod_ci/controllers.py e con la chiamata esistente in mod_auth/controllers.py:185. Estendi tests/test_utility.py e aggiungi assertions per le altre chiamate interessate; il lavoro è completato quando tutte e cinque le richieste in uscita usano il timeout concordato e i test pertinenti hanno esito positivo.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Sample platform commit: fe2b4a4

In raising this issue, I confirm the following (please check boxes, eg [X]):

  • I have read and understood the contributors guide.
  • I have checked that the bug-fix I am reporting can be replicated, or that the feature I am suggesting isn't already present.
  • I have checked that the issue I'm posting isn't already reported.
  • I have checked that the issue I'm posting isn't already solved and no duplicates exist in closed issues and in opened issues
  • I have checked the pull requests tab for existing solutions/implementations to my issue/suggestion.

My familiarity with the project is as follows (check one, eg [X]):

  • I have never visited/used the platform.
  • I have used the platform just a couple of times.
  • I have used the platform extensively, but have not contributed previously.
  • I am an active contributor to the platform.

Summary

#942 identified "GitHub API calls without timeouts" as a critical reliability gap, and #943 fixed it — but only within mod_ci/controllers.py. The audit table in #942 only enumerated call sites in that one file.

Five outbound HTTP calls in utility.py, mod_auth/controllers.py, mailer.py and mod_upload/controllers.py still have no timeout=. Since requests has no default timeout, each of these blocks its worker indefinitely if the remote end accepts the TCP connection but never responds.

Affected call sites

# File Call Reached from
1 utility.py:140 requests.get('https://api.github.com/meta', ...) unauthenticated POST /start-ci
2 mod_auth/controllers.py:212 requests.post(...) OAuth token exchange GET /account/github_callback
3 mod_auth/controllers.py:142 session.post(...) token validity check github_token_validity()
4 mailer.py:45 requests.post(...) Mailgun send signup, password reset
5 mod_upload/controllers.py:106 session.post(...) create GitHub issue upload flow

Why site 1 is the most serious

The outbound call happens before any authentication, because fetching the IP allowlist is the first check:

POST /start-ci                          <- no auth required to send this
  └─ @request_from_github()             utility.py:61
      └─ is_github_web_hook_ip()        utility.py:110   <- FIRST check
          └─ get_cached_web_hook_blocks()   utility.py:126
              └─ requests.get(...)      utility.py:140   <- no timeout

The IP allowlist cannot gate this call, because the call is what retrieves the allowlist.

There is also a cache-invalidation flaw that amplifies it. In get_cached_web_hook_blocks(), the KeyError branch logs the failure but never updates cached_load_time, and cached_web_hook_blocks stays empty. Since the guard is:

if len(cached_web_hook_blocks) == 0 or cache_has_expired(cached_load_time):

...an unexpected payload from the GitHub meta API means the cache never populates and every subsequent webhook request re-fetches. Combined with the missing timeout, that turns one hung call into one hung worker per inbound request.

I'd rate the overall severity as moderate rather than critical: it requires GitHub's API to be hanging rather than merely down (a refused connection fails fast). It is primarily a resilience-under-upstream-failure problem, which is exactly the class #942 was opened to address.

Reproduction

A listener that accepts the connection and never replies reproduces the hang. This is the specific failure mode that a ConnectionError handler does not catch:

python -c "import socket; s=socket.socket(); s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1); s.bind(('127.0.0.1', 9999)); s.listen(1); input()"

Point any of the five call sites at http://127.0.0.1:9999 and the request never returns.

The convention already exists in-tree

This is an inconsistency rather than an unknown. mod_ci/controllers.py:55-63 defines named constants (GITHUB_API_TIMEOUT, GCP_API_TIMEOUT, ARTIFACT_DOWNLOAD_TIMEOUT), and mod_auth/controllers.py:185 already does it correctly:

response = session.get(url, timeout=(3.05, 10))   # line 185 - has a timeout

...43 lines below a call in the same file that does not:

response = session.post(url, json={"access_token": token})   # line 142 - no timeout

Proposed fix

Add timeout=(3.05, 10) at the five sites, matching the existing (connect, read) tuple form at mod_auth/controllers.py:185. No signatures, return types, or call sites change.

Worth noting explicitly: sites 1, 2, 3 and 5 have no exception handler, so a timeout converts a hang into a 500. That is still a strict improvement (a fast, logged failure beats a wedged worker), but adding handlers could be a sensible follow-up if you'd prefer that in the same PR.

Tests would assert the kwarg is actually passed, so the regression can't silently return:

mock_get.assert_called_once_with(
    'https://api.github.com/meta', auth=mock.ANY, timeout=(3.05, 10))

tests/test_utility.py already patches requests.get, so this extends cleanly.

Out of scope, happy to file separately

While tracing the above I found that is_valid_signature() (utility.py:152) raises unhandled exceptions on a malformed X-Hub-Signature header, producing a 500 instead of a clean 418:

'garbage'      -> ValueError: not enough values to unpack (expected 2, got 1)
'bogus=abc'    -> TypeError: Missing required parameter 'digestmod'
'__name__=abc' -> ValueError: unsupported hash type hashlib

hashlib.__dict__.get(hash_algorithm) performs an attribute lookup on a caller-supplied string. Real-world impact is low, since it is only reachable after the GitHub IP check passes and GitHub always sends well-formed signatures. Let me know if you'd like that as its own issue.


I'd be glad to take this one if it's not already spoken for.

Lingua principale
Python
Stelle
30
Fork
85
Merge medio
6g 8h
PR unite (30g)
22

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di CCExtractor/sample-platform

Tutte le issue di CCExtractor/sample-platform

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.