Skip to content

fix: store client TLS connect callback#448

Open
MarkAtwood wants to merge 1 commit into
wolfSSL:mainfrom
MarkAtwood:fix/client-tls-connect-cb
Open

fix: store client TLS connect callback#448
MarkAtwood wants to merge 1 commit into
wolfSSL:mainfrom
MarkAtwood:fix/client-tls-connect-cb

Conversation

@MarkAtwood

Copy link
Copy Markdown

Bug: posixTransportTls_InitConnect (client) never stores the connected-state callback into the context. After memset(ctx, 0, ...) it forwards connectcb/connectcb_arg only to the underlying TCP transport, leaving ctx->connectcb NULL. The existing NULL checks in this file then never fire on the client:

  • line ~186: if (ctx->connectcb != NULL) ctx->connectcb(..., WH_COMM_CONNECTED);
  • SendRequest ~line 276: if (ctx->connectcb != NULL) ctx->connectcb(..., WH_COMM_DISCONNECTED); on TLS handshake failure

So client-side connect/disconnect callbacks are silently never invoked. The server path (posixTransportTls_InitListen, lines 401-402) already stores these fields correctly; this brings the client into line.

Fix: store ctx->connectcb/ctx->connectcb_arg right after the memset, mirroring the server. Two lines, self-contained, no API/ABI change.

How build-verified: compiled the translation unit against a prebuilt wolfSSL install with -DWOLFHSM_CFG_TLS (crypto enabled); gcc exit 0, clean, and nm -g --defined-only confirms posixTransportTls_InitConnect is a live text symbol (code is compiled, not dead).

Reported by static analysis (Fenrir finding 771).

Copilot AI review requested due to automatic review settings July 9, 2026 23:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.

Fixes a client-side bug where the TLS transport never persisted the connected/disconnected callback in its client context, preventing client connect/disconnect notifications from ever firing.

Changes:

  • Store connectcb and connectcb_arg into posixTransportTlsClientContext immediately after the context memset, mirroring the server initialization path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants