Add explicit timeouts to subprocess and HTTP calls in git-gate paths #271

Merged
didericis merged 3 commits from add-subprocess-http-timeouts into main 2026-06-25 04:23:03 -04:00
2 changed files with 7 additions and 14 deletions
Showing only changes of commit c0d3f16519 - Show all commits
+4 -8
View File
@@ -16,18 +16,14 @@ from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
from pathlib import Path from pathlib import Path
from urllib.parse import urlsplit from urllib.parse import urlsplit
from .git_gate import GIT_GATE_DAEMON_TIMEOUT_SECS
Outdated
Review

Given we're using this for two different purposes now, probably makes sense to rename it to GIT_GATE_TIMEOUT_SECS. Can add a comment about where it's currently used/fact that it's used in a multiple places.

Given we're using this for two different purposes now, probably makes sense to rename it to `GIT_GATE_TIMEOUT_SECS`. Can add a comment about where it's currently used/fact that it's used in a multiple places.
DEFAULT_PORT = 9420 DEFAULT_PORT = 9420
# Bound memory use while still allowing ordinary git push packfiles. # Bound memory use while still allowing ordinary git push packfiles.
MAX_BODY_BYTES = 100 * 1024 * 1024 MAX_BODY_BYTES = 100 * 1024 * 1024
# Timeout for the access-hook subprocess and git http-backend CGI subprocess.
# Mirrors GIT_GATE_DAEMON_TIMEOUT_SECS so both HTTP and daemon paths share the
# same bound: a hung upstream fetch in the access-hook or a stalled CGI child
# cannot wedge the sidecar indefinitely.
GIT_HTTP_BACKEND_TIMEOUT_SECS = 30
class GitHttpHandler(BaseHTTPRequestHandler): class GitHttpHandler(BaseHTTPRequestHandler):
Outdated
Review

@didericis-claude Should probably just import GIT_GATE_DAEMON_TIMEOUT_SECS and set this to that value if they should be equal.

@didericis-claude Should probably just import `GIT_GATE_DAEMON_TIMEOUT_SECS` and set this to that value if they should be equal.
server_version = "bot-bottle-git-http/1" server_version = "bot-bottle-git-http/1"
@@ -53,7 +49,7 @@ class GitHttpHandler(BaseHTTPRequestHandler):
[hook_path, "upload-pack", str(repo_dir), peer, peer], [hook_path, "upload-pack", str(repo_dir), peer, peer],
capture_output=True, capture_output=True,
check=False, check=False,
timeout=GIT_HTTP_BACKEND_TIMEOUT_SECS, timeout=GIT_GATE_DAEMON_TIMEOUT_SECS,
) )
if hook.returncode != 0: if hook.returncode != 0:
detail = (hook.stderr or hook.stdout).decode( detail = (hook.stderr or hook.stdout).decode(
@@ -117,7 +113,7 @@ class GitHttpHandler(BaseHTTPRequestHandler):
env=env, env=env,
capture_output=True, capture_output=True,
check=False, check=False,
timeout=GIT_HTTP_BACKEND_TIMEOUT_SECS, timeout=GIT_GATE_DAEMON_TIMEOUT_SECS,
) )
self._write_cgi_response(proc.stdout) self._write_cgi_response(proc.stdout)
+3 -6
View File
@@ -9,11 +9,8 @@ import urllib.request
from pathlib import Path from pathlib import Path
from unittest import mock from unittest import mock
from bot_bottle.git_http_backend import ( from bot_bottle.git_gate import GIT_GATE_DAEMON_TIMEOUT_SECS
GIT_HTTP_BACKEND_TIMEOUT_SECS, from bot_bottle.git_http_backend import GitHttpHandler, MAX_BODY_BYTES
GitHttpHandler,
MAX_BODY_BYTES,
)
class TestGitHttpBackend(unittest.TestCase): class TestGitHttpBackend(unittest.TestCase):
@@ -204,7 +201,7 @@ class TestGitHttpBackend(unittest.TestCase):
for call in run.call_args_list: for call in run.call_args_list:
self.assertEqual( self.assertEqual(
GIT_HTTP_BACKEND_TIMEOUT_SECS, GIT_GATE_DAEMON_TIMEOUT_SECS,
call.kwargs.get("timeout"), call.kwargs.get("timeout"),
f"subprocess.run call missing timeout: {call}", f"subprocess.run call missing timeout: {call}",
) )