cg-net: reap disconnected clients with a process sentinel

Finding 5 of the 2026-07-29 security review: nothing watched for a
connection ending, so players who left stayed on the client list
forever and the host kept trying to send to them.

cg-net--host-sentinel removes a closed connection from the list; it is
installed on every accepted connection.  This also tidies the drops
made by the Finding 4 bounds, which delete-process over-limit peers.

Seat numbers are still not reused after a departure -- deliberate, and
recorded in the sentinel docstring: a stale seat must not be inherited
by a stranger mid-game.  Named in the handback as remaining behavior.

Test cgt-net-reaps-disconnected fails against the previous code: after
the client disconnected the list still held it (= 0 1).
This commit is contained in:
Claude 2026-08-03 20:35:27 -05:00 committed by Corwin Brust
parent 640aaca1ae
commit ade2f38ee3
2 changed files with 37 additions and 0 deletions

View file

@ -214,6 +214,16 @@ The socket binds `cg-net-host-address' -- by default, this machine only."
(delete-process (cg-net-host-server cg-net--host))) (delete-process (cg-net-host-server cg-net--host)))
(setq cg-net--host nil))) (setq cg-net--host nil)))
(defun cg-net--host-sentinel (proc _event)
"Reap PROC from the client list when its connection has ended.
Without this, departed players stay listed forever and the host keeps
sending to them. Seat numbers are deliberately not reused: a stale
seat must not be inherited by a stranger mid-game."
(unless (process-live-p proc)
(when cg-net--host
(setf (cg-net-host-clients cg-net--host)
(delq proc (cg-net-host-clients cg-net--host))))))
(defun cg-net--host-accept (_server connection _message) (defun cg-net--host-accept (_server connection _message)
"Set up an accepted CONNECTION: assign a seat and send the current state. "Set up an accepted CONNECTION: assign a seat and send the current state.
A connection arriving past `cg-net-max-connections' is closed instead." A connection arriving past `cg-net-max-connections' is closed instead."
@ -229,6 +239,7 @@ A connection arriving past `cg-net-max-connections' is closed instead."
(push connection (cg-net-host-clients cg-net--host)) (push connection (cg-net-host-clients cg-net--host))
(process-put connection 'cg-net-seat seat) (process-put connection 'cg-net-seat seat)
(set-process-coding-system connection 'utf-8 'utf-8) (set-process-coding-system connection 'utf-8 'utf-8)
(set-process-sentinel connection #'cg-net--host-sentinel)
(set-process-filter connection (cg-net--filter #'cg-net--host-handle)) (set-process-filter connection (cg-net--filter #'cg-net--host-handle))
(cg-net--send connection (list :type 'welcome :seat seat)) (cg-net--send connection (list :type 'welcome :seat seat))
(cg-net--send connection (cg-net--send connection

View file

@ -181,6 +181,32 @@ Finding 4's second bound: the client list cannot grow without limit."
(dolist (p procs) (when (process-live-p p) (delete-process p))) (dolist (p procs) (when (process-live-p p) (delete-process p)))
(cg-net-host-stop))))) (cg-net-host-stop)))))
(ert-deftest cgt-net-reaps-disconnected ()
"A client that disconnects is removed from the host's client list.
Finding 5 of the 2026-07-29 review: nothing watched for a connection
ending, so departed players stayed listed forever and the host kept
sending to them. A process sentinel now reaps them."
(condition-case _
(delete-process
(make-network-process :name "cgt-probe7" :server t :service 0
:host "127.0.0.1" :family 'ipv4))
(error (ert-skip "TCP not available")))
(cl-flet ((pump () (dotimes (_ 12) (accept-process-output nil 0.05))))
(let* ((hgame (make-instance 'cgt-net-game :env (list :counter 0)))
(srv (cg-net-host-start hgame 0))
(port (process-contact srv :service))
(cgame (make-instance 'cgt-net-game :env (list :counter 0))))
(unwind-protect
(progn
(cg-net-connect "127.0.0.1" port "Test" cgame)
(pump)
(should (= 1 (length (cg-net-host-clients cg-net--host))))
(cg-net-disconnect)
(pump)
(should (= 0 (length (cg-net-host-clients cg-net--host)))))
(cg-net-disconnect)
(cg-net-host-stop)))))
;;;; Gaps ;;;; Gaps
(ert-deftest cgt-gaps-deal () (ert-deftest cgt-gaps-deal ()