Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

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

Open
  • Is something wrong?
#1,192 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 6 days

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
84/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
python
Domain
backend, networking

Research direction

Start with the five call sites listed in utility.py, mod_auth/controllers.py, mailer.py, and mod_upload/controllers.py, then compare them with the timeout constants in mod_ci/controllers.py and the existing call at mod_auth/controllers.py:185. Extend tests/test_utility.py and add assertions for the other affected calls; done means all five outbound requests pass the agreed timeout and the relevant tests pass.

Written by the indexing model from the issue text.

Description

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.

Dominant language
Python
Stars
30
Forks
87
Avg merge
5d 10h
Merged PRs (30d)
21

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from CCExtractor/sample-platform

All issues in CCExtractor/sample-platform

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.