Skip to content

Replace ASSERT(0) + "must be a bug" with proper error handling in EncryptedStreamSocket

Historical reference only: stale-v0.72a-experimental-clean and analysis\stale-v0.72a-experimental-clean are retired reference sources, not active branch targets or current baselines. Use them only as provenance or idea-extraction sources; landed status is determined against main. See Historical References.

Summary

EncryptedStreamSocket.cpp contains approximately 14 occurrences of ASSERT(0) paired with a // must be a bug comment. In release builds ASSERT compiles to nothing, so these conditions silently fail. The correct response in each case is to treat the condition as a protocol error, log it, and disconnect the peer gracefully via OnError().

Landed In Main

Implemented on 2026-04-18 in commit 93b3450.

Current main adds FailEncryptedStream(LPCTSTR) to EncryptedStreamSocket and routes the protocol/state-machine failure paths through explicit logging plus OnError(ERR_ENCRYPTION) instead of relying on assert-only behavior. The narrow internal invariants that are not protocol error paths remain plain asserts.

Location

srchybrid/EncryptedStreamSocket.cpp — 14 sites, pattern:

ASSERT(0);
// must be a bug

These occur in packet parsing and state machine transitions where an unexpected state is encountered (wrong negotiation phase, unexpected opcode, malformed length prefix, etc.).

Problem

  • Debug builds: ASSERT(0) pops a dialog and halts — useful during development but not in production
  • Release builds: the ASSERT expands to nothing; execution continues into undefined state, potentially corrupting the connection or the socket buffer
  • In both cases: the comment // must be a bug implies it is a protocol error from the remote peer, not an internal bug — the correct action is peer disconnect, not an assertion

Fix

Replace each occurrence with an explicit error path:

// Before:
ASSERT(0);
// must be a bug

// After:
TRACE(_T("EncryptedStreamSocket: unexpected state %d — disconnecting peer\n"), m_eEncryptionState);
OnError(WSAECONNABORTED);
return;

Exact message text and state variable name will vary by call site. Use the nearest available state variable or offset in the trace message.

Where OnError() is not appropriate (e.g., inside a Receive override that returns a length), return SOCKET_ERROR or -1 and set WSASetLastError.

Files to Modify

File Change
srchybrid/EncryptedStreamSocket.cpp Replace the protocol/state-machine ASSERT(0) sites with OnError() + return
srchybrid/EncryptedStreamSocket.h Add private FailEncryptedStream(LPCTSTR) helper declaration

Acceptance Criteria

  • [x] No assert-only protocol-failure path remains in EncryptedStreamSocket.cpp
  • [x] Former protocol/state-machine assert sites log and call OnError() or return an error value consistent with their call site
  • [x] In release builds, a misbehaving peer causes a clean disconnect, not silent undefined behavior
  • [x] Narrow internal invariants that are not remote/protocol error paths may remain plain asserts
  • [x] No changes to the happy-path encryption negotiation logic

Evidence

  • srchybrid/EncryptedStreamSocket.h declares CEncryptedStreamSocket::FailEncryptedStream(LPCTSTR).
  • srchybrid/EncryptedStreamSocket.cpp implements FailEncryptedStream() with a debug assertion, explicit DebugLogError(), and OnError(ERR_ENCRYPTION).
  • Static scan on current main shows former protocol/state-machine failure sites now call FailEncryptedStream() with contextual reason strings.
  • The remaining raw ASSERT(0) sites in EncryptedStreamSocket.cpp are narrow internal invariants: pending-send buffer state in Send() and the fallback after repeated random protocol-marker collisions.
  • Native coverage exists in repos/emulebb-build-tests/src/encrypted_stream_socket.tests.cpp and repos/emulebb-build-tests/src/encrypted_stream_flow.tests.cpp for the delayed server-send and handshake-failure policy seams.
  • Latest validation in this Beta 0.7.3 pass: python -m emule_workspace validate, python -m emule_workspace build app --config Debug --platform x64, and python -m emule_workspace test all --config Debug --platform x64 passed.

Experimental Reference Implementation

Status in stale-v0.72a-experimental-clean: Done. The fix introduced a FailEncryptedStream(LPCTSTR pszReason) private helper method that logs the error message and calls OnError(ERR_ENCRYPTION). All ASSERT(0); // must be a bug sites were replaced with FailEncryptedStream(_T("...descriptive message...")) — each with a unique context string. Sites that are inside Receive() overrides return SOCKET_ERROR after calling FailEncryptedStream. An EncryptedStreamSocketSeams.h header was also added for testability.

Porting note: The FailEncryptedStream helper is ~5 lines and can be copied verbatim. Then each of the 14 ASSERT sites is replaced with the appropriate FailEncryptedStream(...) call. The diff from experimental is clean and easy to port.