fix(sm): an OnCER or OnDWR hook that closes the connection aborts processing

Merged
GO
5d ago
+638-43

Settings.OnCER promises that closing the connection from the hook aborts the handshake. The CER handler ignored that and carried on:

  • it published the peer's metadata;
  • it admitted the peer;
  • it called OnCEA;
  • it wrote the CEA to the closed connection and logged the failed write at Error.

OnDWR had the same gap for the DWA.

Change

  • diam.Conn.Closed() (new). It reports that Close has been called, or that the server has finished with the connection.
    • Close publishes it before closing the transport.
    • When the read loop ends, the server first waits for the handlers already running, so a CER or DWR that arrived before the peer's FIN is still answered (RFC 6733 §5.6 processes events in order).
    • Every server-side close goes through the same path, including Shutdown's forced close and the multistream error handler.
    • Closing Connection() directly is not observed, and the hook docs say to call c.Close().
  • Hooks. After OnCER or OnDWR returns, the state machine stops if the connection is closed. Then there are no metadata, no admission, no CEA or DWA, no OnCEA, OnHandshake or OnDWA, and nothing logged above Debug.

API change: implementations of diam.Conn outside this module must add Closed() bool. An optional interface found through ConnAs was considered and rejected: opaque wrappers that embed only the core methods would hide it, and the abort would then fail silently.

Tests

  • Hook abort: TCP and SCTP in every handler wiring and dispatch mode (30 SCTP subtests, none skipped), plus admission observed directly.
  • Half-close: a CER or DWR followed by the peer's FIN must be answered in every one of 50 runs, in sequential and concurrent dispatch. A first version of this change answered 7/50 CERs and 0/50 DWRs under concurrent dispatch. tshark captures confirm 50/50 CEAs and DWAs with matching identifiers.
  • Shutdown: a hook still running when Shutdown's deadline expires produces no answer and no Error log.
  • Multistream: a multistream read error publishes Closed before the transport closes.
  • Mutations caught:
    • no check after the hook;
    • the flag published before handlers finish;
    • the flag never published on read-loop exit;
    • Shutdown or the multistream handler bypassing the flag.

Validation

go build ./..., go test ./... -count=1, go test -race ./diam/... -count=1, go vet ./..., staticcheck ./... and golangci-lint run ./... all pass, and gofmt -l . is empty. The examples/middleware build, vet and test pass.

Fixes #70

Summary by cubic

Fixes #70. A Settings.OnCER or Settings.OnDWR hook that closes the connection now aborts processing: previously it continued publishing peer metadata, admitting the peer, calling OnCEA/OnDWA, writing the answer to the closed connection, and logging the failed write at Error.

  • Adds Closed() to diam.Conn; Close() publishes it before closing the transport, and read-loop exit waits for running handlers first so a CER or DWR received before the peer's FIN is still answered (RFC 6733 §5.6).
  • All server-side closes, including Shutdown's forced close and the multistream error handler, now publish Closed. Closing Connection() directly is not detected; the hook docs say to call c.Close().

Breaking change

  • External diam.Conn implementations must add Closed() bool.