Hacktoberfest 2026: những issue maintainer đã đánh dấu cho tháng Mười, đang mở và phù hợp người mới. Xem issue Hacktoberfest

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

Đang mở
  • Có gì không ổn?
#1,192 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức phù hợp với người mới
84/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
python
Lĩnh vực
backend, networking

Hướng nghiên cứu

Bắt đầu với năm vị trí gọi được liệt kê trong utility.py, mod_auth/controllers.py, mailer.py và mod_upload/controllers.py, sau đó so sánh chúng với các hằng số timeout trong mod_ci/controllers.py và lời gọi hiện có tại mod_auth/controllers.py:185. Mở rộng tests/test_utility.py và thêm assertions cho các lời gọi bị ảnh hưởng khác; hoàn tất khi cả năm yêu cầu outbound đều sử dụng timeout đã thống nhất và các bài kiểm thử liên quan đều đạt.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

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.

Ngôn ngữ chính
Python
Star
30
Fork
85
Merge trung bình
5 ngày 16 giờ
Pull request đã merge (30 ngày)
20

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của CCExtractor/sample-platform

Tất cả issue của CCExtractor/sample-platform

Issue tương tự

Thêm issue về Python

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.