Skip to content

tests: cover a channel-open rejection on a forwarding channel - #1205

Merged
philljj merged 1 commit into
wolfSSL:masterfrom
ejohnstown:ccb-phase1-1
Aug 30, 2026
Merged

tests: cover a channel-open rejection on a forwarding channel#1205
philljj merged 1 commit into
wolfSSL:masterfrom
ejohnstown:ccb-phase1-1

Conversation

@ejohnstown

@ejohnstown ejohnstown commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

No regress case registered a channelOpenCb and a fwdCb at the same time, which is why the v1.5.0 clobber -- fwdCb(LOCAL_SETUP) overwriting the open callback's rejection -- shipped unnoticed. The fix is on master already (0317c40, 616eb68); this is the missing gate.

  • TestDirectTcpipOpenCbRejectBeatsFwdCb: both callbacks registered and the open callback rejects, so the peer must get a channel-open failure and the forwarding hook must not run at all
  • TestDirectTcpipFwdCbRejectAfterOpenCbAccept: the other half, where the open callback accepts and the fwdCb's rejection has to reach the peer
  • TestDirectTcpipFwdCbRejectsChannelId: DoChannelOpen() consults the fwdCb twice, and only the setup rejection was covered

All three pin the recipient channel and the reason code, not just the message id. The call counter is file scope rather than reached through the callback ctx, so the zero the first test asserts cannot be a ctx that stopped being delivered; the third test reads 2 on the same counter as the positive control.

Tests only, no library change.

Copilot AI lite review requested due to automatic review settings August 27, 2026 23:12

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

Adds missing regression coverage for a forwarding-channel edge case where a channelOpenCb rejection could be overwritten by a forwarding callback result (a v1.5.0 behavior that shipped due to lack of combined-callback tests). This PR strengthens the test suite to ensure the peer receives the correct channel-open failure and the forwarding hooks are invoked (or not) as intended.

Changes:

  • Adds an “accept” channel-open callback and counting forwarding callbacks to exercise ordering/precedence.
  • Introduces three new direct-tcpip regression tests covering: open-callback rejection precedence, fwdCb rejection after accept, and rejection on the channel-id handoff path.
  • Registers the new tests in the WOLFSSH_FWD test block in main().

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/regress.c
No regress case registered a channelOpenCb and a fwdCb at the same time,
which is why the v1.5.0 clobber -- fwdCb(LOCAL_SETUP) overwriting the
open callback's rejection -- shipped unnoticed. The fix is on master
already (0317c40, 616eb68); this is the missing gate.

- TestDirectTcpipOpenCbRejectBeatsFwdCb: both callbacks registered and
  the open callback rejects, so the peer must get a channel-open failure
  and the forwarding hook must not run at all
- TestDirectTcpipFwdCbRejectAfterOpenCbAccept: the other half, where the
  open callback accepts and the fwdCb's rejection has to reach the peer
- TestDirectTcpipFwdCbRejectsChannelId: DoChannelOpen() consults the
  fwdCb twice, and only the setup rejection was covered

All three pin the recipient channel and the reason code, not just the
message id. The call counter is file scope rather than reached through
the callback ctx, so the zero the first test asserts cannot be a ctx that
stopped being delivered; the third test reads 2 on the same counter as
the positive control.

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #1205

Scan targets checked: wolfssh-bugs, wolfssh-src

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

@ejohnstown
ejohnstown requested a review from philljj August 28, 2026 22:31
@philljj philljj self-assigned this Aug 28, 2026
@philljj
philljj merged commit c88e2e7 into wolfSSL:master Aug 30, 2026
166 checks passed
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.

4 participants