fix: an answer whose client went away ends quietly, and a shell whose client left before it opened is stopped.
Release / release (push) Successful in 15s
Release / release (push) Successful in 15s
This commit is contained in:
1 parent
dd489e65aa
commit
1e0e375d60
7 files changed
+66
-1
No files matched your search
@@ -1342,3 +1342,28 @@ method.
|
|||||||
Consequences: A change to one family of paths touches one module and the routing in `handler`.
|
Consequences: A change to one family of paths touches one module and the routing in `handler`.
|
||||||
State shared between requests, such as the open shells, lives in the module that owns it; other
|
State shared between requests, such as the open shells, lives in the module that owns it; other
|
||||||
modules change it and never rebind it.
|
modules change it and never rebind it.
|
||||||
|
|
||||||
|
## ADR-48: Exceptions are handled; no traceback reaches the logs
|
||||||
|
|
||||||
|
Status: Accepted
|
||||||
|
|
||||||
|
Context: The services log to the journal, which administrators read through the Logs page and
|
||||||
|
`journalctl`. Python prints a traceback for an exception nothing handles: `socketserver` for a
|
||||||
|
failed request, `threading` for a failed thread and the interpreter for the process. A client
|
||||||
|
that closes its connection before the answer is written, as a page does when it is left or
|
||||||
|
reloaded, raises `BrokenPipeError` or `ConnectionResetError` in the handler. A traceback is many
|
||||||
|
lines of internals for one event and hides the lines around it.
|
||||||
|
|
||||||
|
Decision: The code handles the exceptions it can expect where they occur, and answers or logs
|
||||||
|
them with their reason in one line. An exception left unhandled is a defect to be fixed. As the
|
||||||
|
last line of defence, every server replaces the default reports:
|
||||||
|
|
||||||
|
- `vpnrouter_serve.serve`, which the web service and the metrics service run, sets the servers'
|
||||||
|
`handle_error`, `threading.excepthook` and `sys.excepthook`.
|
||||||
|
- The bootstrap server sets its server's `handle_error` and `sys.excepthook`.
|
||||||
|
|
||||||
|
A `ConnectionError` of a client that went away is not logged; any other exception is logged in
|
||||||
|
one line with its type and message.
|
||||||
|
|
||||||
|
Consequences: The logs hold one line per failure, without its stack. A failure that reaches the
|
||||||
|
last line of defence is found by its message and fixed where it occurs, with its own handling.
|
||||||
@@ -1,3 +1,15 @@
|
|||||||
|
vpn-router (0.37.3-1) unstable; urgency=medium
|
||||||
|
|
||||||
|
fix: an answer whose client went away ends quietly, and a shell whose client left before it opened is stopped.
|
||||||
|
|
||||||
|
* The web service, the metrics service and the bootstrap server stop
|
||||||
|
writing an answer when its client closes the connection, and a site
|
||||||
|
file whose download is left no longer gets a second answer.
|
||||||
|
* A shell whose client closed the connection before it opened is
|
||||||
|
stopped and its end is logged, instead of running on.
|
||||||
|
|
||||||
|
-- Slawomir Koszewski <slawek@koszewscy.waw.pl> Fri, 30 Oct 2026 02:03:26 +0100
|
||||||
|
|
||||||
vpn-router (0.37.2-1) unstable; urgency=medium
|
vpn-router (0.37.2-1) unstable; urgency=medium
|
||||||
|
|
||||||
fix: the web service, the metrics service and the bootstrap server log a failure in one line instead of a traceback, and nothing when a client goes away.
|
fix: the web service, the metrics service and the bootstrap server log a failure in one line instead of a traceback, and nothing when a client goes away.
|
||||||
|
|||||||
@@ -43,6 +43,7 @@ def report():
|
|||||||
class Handler(BaseHTTPRequestHandler):
|
class Handler(BaseHTTPRequestHandler):
|
||||||
def do_GET(self):
|
def do_GET(self):
|
||||||
body = report().encode()
|
body = report().encode()
|
||||||
|
try:
|
||||||
self.send_response(200)
|
self.send_response(200)
|
||||||
self.send_header('Content-Type', 'text/plain; charset=utf-8')
|
self.send_header('Content-Type', 'text/plain; charset=utf-8')
|
||||||
self.send_header('Content-Length', str(len(body)))
|
self.send_header('Content-Length', str(len(body)))
|
||||||
@@ -50,6 +51,9 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
self.send_header('Refresh', '10')
|
self.send_header('Refresh', '10')
|
||||||
self.end_headers()
|
self.end_headers()
|
||||||
self.wfile.write(body)
|
self.wfile.write(body)
|
||||||
|
except ConnectionError:
|
||||||
|
# The client went away before the answer was written.
|
||||||
|
self.close_connection = True
|
||||||
|
|
||||||
|
|
||||||
def failed(request, client_address):
|
def failed(request, client_address):
|
||||||
|
|||||||
@@ -405,6 +405,7 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
registry = CollectorRegistry()
|
registry = CollectorRegistry()
|
||||||
registry.register(Sample(current))
|
registry.register(Sample(current))
|
||||||
body, kind, code = generate_latest(registry), CONTENT_TYPE_LATEST, 200
|
body, kind, code = generate_latest(registry), CONTENT_TYPE_LATEST, 200
|
||||||
|
try:
|
||||||
self.send_response(code)
|
self.send_response(code)
|
||||||
self.send_header('Content-Type', kind)
|
self.send_header('Content-Type', kind)
|
||||||
self.send_header('Content-Length', str(len(body)))
|
self.send_header('Content-Length', str(len(body)))
|
||||||
@@ -414,6 +415,9 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
self.end_headers()
|
self.end_headers()
|
||||||
if send_body:
|
if send_body:
|
||||||
self.wfile.write(body)
|
self.wfile.write(body)
|
||||||
|
except ConnectionError:
|
||||||
|
# The client went away before the answer was written.
|
||||||
|
self.close_connection = True
|
||||||
|
|
||||||
def do_GET(self):
|
def do_GET(self):
|
||||||
self.answer(True)
|
self.answer(True)
|
||||||
|
|||||||
@@ -188,6 +188,7 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
code = 200
|
code = 200
|
||||||
else:
|
else:
|
||||||
body, kind, code = b'Not found\n', 'text/plain; charset=utf-8', 404
|
body, kind, code = b'Not found\n', 'text/plain; charset=utf-8', 404
|
||||||
|
try:
|
||||||
self.send_response(code)
|
self.send_response(code)
|
||||||
self.send_header('Content-Type', kind)
|
self.send_header('Content-Type', kind)
|
||||||
self.send_header('Content-Length', str(len(body)))
|
self.send_header('Content-Length', str(len(body)))
|
||||||
@@ -197,6 +198,9 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
self.end_headers()
|
self.end_headers()
|
||||||
if send_body:
|
if send_body:
|
||||||
self.wfile.write(body)
|
self.wfile.write(body)
|
||||||
|
except ConnectionError:
|
||||||
|
# The client went away before the answer was written.
|
||||||
|
self.close_connection = True
|
||||||
|
|
||||||
def raw_body(self):
|
def raw_body(self):
|
||||||
"""The request body's bytes, b'' without one or over BODY_LIMIT."""
|
"""The request body's bytes, b'' without one or over BODY_LIMIT."""
|
||||||
|
|||||||
@@ -367,11 +367,14 @@ def shell(handler, name):
|
|||||||
def refuse(code, message):
|
def refuse(code, message):
|
||||||
log(f'shell of {name} from {handler.client()} refused: {message}')
|
log(f'shell of {name} from {handler.client()} refused: {message}')
|
||||||
body = json.dumps({'error': message}).encode() + b'\n'
|
body = json.dumps({'error': message}).encode() + b'\n'
|
||||||
|
try:
|
||||||
handler.send_response(code)
|
handler.send_response(code)
|
||||||
handler.send_header('Content-Type', 'application/json')
|
handler.send_header('Content-Type', 'application/json')
|
||||||
handler.send_header('Content-Length', str(len(body)))
|
handler.send_header('Content-Length', str(len(body)))
|
||||||
handler.end_headers()
|
handler.end_headers()
|
||||||
handler.wfile.write(body)
|
handler.wfile.write(body)
|
||||||
|
except ConnectionError:
|
||||||
|
handler.close_connection = True
|
||||||
|
|
||||||
key = handler.headers.get('Sec-WebSocket-Key', '').strip()
|
key = handler.headers.get('Sec-WebSocket-Key', '').strip()
|
||||||
if handler.headers.get('Upgrade', '').lower() != 'websocket' or 'upgrade' not in handler.headers.get('Connection', '').lower() \
|
if handler.headers.get('Upgrade', '').lower() != 'websocket' or 'upgrade' not in handler.headers.get('Connection', '').lower() \
|
||||||
@@ -408,12 +411,21 @@ def shell(handler, name):
|
|||||||
finally:
|
finally:
|
||||||
os.close(terminal)
|
os.close(terminal)
|
||||||
accept = base64.b64encode(hashlib.sha1((key + WEBSOCKET_GUID).encode()).digest()).decode()
|
accept = base64.b64encode(hashlib.sha1((key + WEBSOCKET_GUID).encode()).digest()).decode()
|
||||||
|
handler.close_connection = True
|
||||||
|
try:
|
||||||
handler.send_response(101, 'Switching Protocols')
|
handler.send_response(101, 'Switching Protocols')
|
||||||
handler.send_header('Upgrade', 'websocket')
|
handler.send_header('Upgrade', 'websocket')
|
||||||
handler.send_header('Connection', 'Upgrade')
|
handler.send_header('Connection', 'Upgrade')
|
||||||
handler.send_header('Sec-WebSocket-Accept', accept)
|
handler.send_header('Sec-WebSocket-Accept', accept)
|
||||||
handler.end_headers()
|
handler.end_headers()
|
||||||
handler.close_connection = True
|
except ConnectionError:
|
||||||
|
# The client went away before the shell opened; the shell started for it is stopped.
|
||||||
|
run('systemctl', 'stop', unit)
|
||||||
|
os.close(master)
|
||||||
|
process.kill()
|
||||||
|
process.wait()
|
||||||
|
log(f'shell of {name} from {handler.client()} ended before it opened: the client closed the connection')
|
||||||
|
return
|
||||||
client, started, idle = handler.client(), time.monotonic(), options['shell_idle_minutes'] * 60
|
client, started, idle = handler.client(), time.monotonic(), options['shell_idle_minutes'] * 60
|
||||||
log(f'shell of {name} from {client} opened as {account.pw_name} in {unit}')
|
log(f'shell of {name} from {client} opened as {account.pw_name} in {unit}')
|
||||||
|
|
||||||
|
|||||||
@@ -112,6 +112,7 @@ def site_files(handler, name, relative, query, payload, send_body):
|
|||||||
return 404, {'error': f'{relative} does not exist'}
|
return 404, {'error': f'{relative} does not exist'}
|
||||||
with open(target, 'rb') as handle:
|
with open(target, 'rb') as handle:
|
||||||
size = os.fstat(handle.fileno()).st_size
|
size = os.fstat(handle.fileno()).st_size
|
||||||
|
try:
|
||||||
handler.send_response(200)
|
handler.send_response(200)
|
||||||
handler.send_header('Content-Type', mimetypes.guess_type(target.name)[0] or 'application/octet-stream')
|
handler.send_header('Content-Type', mimetypes.guess_type(target.name)[0] or 'application/octet-stream')
|
||||||
handler.send_header('Content-Length', str(size))
|
handler.send_header('Content-Length', str(size))
|
||||||
@@ -121,6 +122,9 @@ def site_files(handler, name, relative, query, payload, send_body):
|
|||||||
handler.end_headers()
|
handler.end_headers()
|
||||||
if send_body:
|
if send_body:
|
||||||
shutil.copyfileobj(handle, handler.wfile, vpnrouter_sites.CHUNK)
|
shutil.copyfileobj(handle, handler.wfile, vpnrouter_sites.CHUNK)
|
||||||
|
except ConnectionError:
|
||||||
|
# The client went away before the file was sent.
|
||||||
|
handler.close_connection = True
|
||||||
return None
|
return None
|
||||||
if handler.command == 'PUT':
|
if handler.command == 'PUT':
|
||||||
length = handler.headers.get('Content-Length')
|
length = handler.headers.get('Content-Length')
|
||||||
|
|||||||
Reference in new issue
Block a user