refactor(http): route every reject-before-read through one arming helper
Replaces the inline `handler.close_connection = True` at each reject-before-read site with api.helpers.arm_connection_close(), the shared mechanism the review asked for. Behaviour is unchanged at every real handler; the helper adds the same stub tolerance the read side just got, so a handler that refuses the attribute can never turn a 4xx into a 500. Sites: _check_csrf (origin + token), the sidecar provenance 403 (body-reading calls only), the rate-limited CSP report, /api/health/restart, the deprecated process-complete-ack 410, and all four upload handlers' pre-read length/boundary rejections.
This commit is contained in:
1 parent
9a181c0026
commit
a93e8ba890
2 files changed
+17
-16
No files matched your search
+7
-6
@@ -2974,6 +2974,7 @@ from api.helpers import (
|
||||
require,
|
||||
bad,
|
||||
safe_resolve,
|
||||
arm_connection_close,
|
||||
j,
|
||||
t,
|
||||
read_body,
|
||||
@@ -5873,7 +5874,7 @@ def _check_csrf(handler) -> bool:
|
||||
if not _check_same_origin_browser_request(handler):
|
||||
# CSRF checks run before read_body(). Close rather than reusing an
|
||||
# HTTP/1.1 connection whose unread body would corrupt the next request.
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
return False
|
||||
if not _is_browser_unsafe_request(handler):
|
||||
return True # non-browser clients (curl, MCP, agent) have no Origin/Referer
|
||||
@@ -5886,7 +5887,7 @@ def _check_csrf(handler) -> bool:
|
||||
submitted = handler.headers.get(CSRF_HEADER_NAME) or handler.headers.get("X-CSRF-Token")
|
||||
if verify_csrf_token(cookie_val or "", submitted or ""):
|
||||
return True
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
return _set_csrf_failure_reason(handler, "token_mismatch")
|
||||
|
||||
|
||||
@@ -6071,7 +6072,7 @@ def _handle_extension_sidecar_proxy(
|
||||
# unread body would corrupt the next request (same class as _check_csrf).
|
||||
# GET (read_request_body=False) has nothing unread — keep-alive survives.
|
||||
if read_request_body:
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
return j(handler, {"error": _csrf_rejection_error(handler)}, status=403)
|
||||
try:
|
||||
request_body = _read_body_bytes(handler) if read_request_body else None
|
||||
@@ -6516,7 +6517,7 @@ def _handle_csp_report(handler) -> bool:
|
||||
)
|
||||
# Rate-limit rejection runs before the body is read; close-and-advertise
|
||||
# so the unread report can't corrupt the next pooled request.
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
return _send_no_content(handler)
|
||||
|
||||
payload = _read_csp_report_payload(handler)
|
||||
@@ -12910,7 +12911,7 @@ def _handle_health_restart(handler) -> bool:
|
||||
"""Restart the Hermes messaging gateway service."""
|
||||
# This endpoint never consumes its request body on any outcome; close so a
|
||||
# supplied body can't corrupt the next pooled request.
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
outcome = restart_active_profile_gateway()
|
||||
|
||||
if outcome.get("status") == "completed":
|
||||
@@ -15236,7 +15237,7 @@ def handle_post(handler, parsed) -> bool:
|
||||
# The 410 runs before the stale tab's JSON body is read;
|
||||
# close-and-advertise so those unread bytes can't corrupt the next
|
||||
# pooled request.
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
j(
|
||||
handler,
|
||||
{
|
||||
|
||||
+10
-10
@@ -8,7 +8,7 @@ import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
from api.config import MAX_UPLOAD_BYTES, STATE_DIR
|
||||
from api.helpers import j
|
||||
from api.helpers import arm_connection_close, j
|
||||
from api.models import get_session
|
||||
from api.profiles import _profiles_match, get_active_profile_name as _get_active_profile_name
|
||||
from api.workspace import (
|
||||
@@ -47,7 +47,7 @@ _MAX_EXTRACTED_BYTES = 10 * MAX_UPLOAD_BYTES
|
||||
|
||||
def _reject_before_read(handler, message, status):
|
||||
"""Reject without reading the body; close so unread bytes can't poison HTTP/1.1 reuse."""
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
return j(handler, {'error': message}, status=status)
|
||||
|
||||
|
||||
@@ -217,7 +217,7 @@ def handle_upload(handler):
|
||||
content_length = int(handler.headers.get('Content-Length', 0) or 0)
|
||||
except (TypeError, ValueError):
|
||||
# A non-numeric Content-Length is rejected before the body is read.
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
raise ValueError('Invalid Content-Length') from None
|
||||
if content_length > MAX_UPLOAD_BYTES:
|
||||
return _reject_before_read(handler, f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)', 413)
|
||||
@@ -226,7 +226,7 @@ def handle_upload(handler):
|
||||
except ValueError:
|
||||
# parse_multipart validates Content-Length/boundary before reading;
|
||||
# a ValueError here means the body was never consumed.
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
raise
|
||||
session_id = fields.get('session_id', '')
|
||||
if 'file' not in files:
|
||||
@@ -418,7 +418,7 @@ def handle_upload_extract(handler):
|
||||
content_length = int(handler.headers.get('Content-Length', 0) or 0)
|
||||
except (TypeError, ValueError):
|
||||
# A non-numeric Content-Length is rejected before the body is read.
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
raise ValueError('Invalid Content-Length') from None
|
||||
if content_length > MAX_UPLOAD_BYTES:
|
||||
return _reject_before_read(handler, f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)', 413)
|
||||
@@ -427,7 +427,7 @@ def handle_upload_extract(handler):
|
||||
except ValueError:
|
||||
# parse_multipart validates Content-Length/boundary before reading;
|
||||
# a ValueError here means the body was never consumed.
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
raise
|
||||
session_id = fields.get('session_id', '')
|
||||
if 'file' not in files:
|
||||
@@ -461,7 +461,7 @@ def handle_transcribe(handler):
|
||||
content_length = int(handler.headers.get('Content-Length', 0) or 0)
|
||||
except (TypeError, ValueError):
|
||||
# A non-numeric Content-Length is rejected before the body is read.
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
raise ValueError('Invalid Content-Length') from None
|
||||
if content_length > MAX_UPLOAD_BYTES:
|
||||
return _reject_before_read(handler, f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)', 413)
|
||||
@@ -470,7 +470,7 @@ def handle_transcribe(handler):
|
||||
except ValueError:
|
||||
# parse_multipart validates Content-Length/boundary before reading;
|
||||
# a ValueError here means the body was never consumed.
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
raise
|
||||
if 'file' not in files:
|
||||
return j(handler, {'error': 'No file field in request'}, status=400)
|
||||
@@ -656,7 +656,7 @@ def handle_workspace_upload(handler):
|
||||
content_length = int(handler.headers.get('Content-Length', 0) or 0)
|
||||
except (TypeError, ValueError):
|
||||
# A non-numeric Content-Length is rejected before the body is read.
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
raise ValueError('Invalid Content-Length') from None
|
||||
if content_length > MAX_UPLOAD_BYTES:
|
||||
return _reject_before_read(handler, f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)', 413)
|
||||
@@ -666,7 +666,7 @@ def handle_workspace_upload(handler):
|
||||
except ValueError:
|
||||
# parse_multipart validates Content-Length/boundary before reading;
|
||||
# a ValueError here means the body was never consumed.
|
||||
handler.close_connection = True
|
||||
arm_connection_close(handler)
|
||||
raise
|
||||
session_id = fields.get('session_id', '')
|
||||
subpath = fields.get('path', '')
|
||||
|
||||
Reference in new issue
Block a user