Conversation
📝 WalkthroughWalkthroughCentralized TLS hostname verification and added TLS-method compatibility and shutdown-error normalization for OpenSSL; introduced small helper abstractions for WolfSSL zero-return/shutdown checks; swapped synchronization primitive from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
An automated preview of the documentation is available at https://115.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-02-10 00:41:15 UTC |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #115 +/- ##
===========================================
+ Coverage 79.70% 79.75% +0.05%
===========================================
Files 65 65
Lines 5661 5680 +19
===========================================
+ Hits 4512 4530 +18
- Misses 1149 1150 +1
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
GCOVR code coverage report https://115.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-02-10 00:47:55 UTC |
…austion Without this, the full benchmark suite exhausts ephemeral ports before reaching fan_out, causing WSAEADDRINUSE (10048) on connect.
Add backend-specific TLS compatibility shims and correct TLS target file lists, then harden IOCP truncation test behavior so OpenSSL/WolfSSL suites complete reliably.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/wolfssl/src/wolfssl_stream.cpp`:
- Around line 101-113: The fallback in has_peer_shutdown() can misclassify a
locally-sent close_notify as peer shutdown because it treats any non-zero
wolfSSL_get_shutdown() as "received"; change the behavior to avoid silently
weakening security by (A) replacing the else-branch with a conservative default
that does NOT assume peer shutdown (e.g., return false) and add a clear comment
in do_read_some referencing has_peer_shutdown() about the limitation when flag
macros are unavailable, and/or (B) fail fast at compile-time when neither
WOLFSSL_RECEIVED_SHUTDOWN nor SSL_RECEIVED_SHUTDOWN is defined (use `#error` or a
static_assert-style preprocessor check) so builds must explicitly handle the
missing macros; refer to has_peer_shutdown and do_read_some when making the
change.
| inline bool | ||
| has_peer_shutdown(WOLFSSL* ssl) noexcept | ||
| { | ||
| int const shutdown = wolfSSL_get_shutdown(ssl); | ||
| #if defined(WOLFSSL_RECEIVED_SHUTDOWN) | ||
| return (shutdown & WOLFSSL_RECEIVED_SHUTDOWN) != 0; | ||
| #elif defined(SSL_RECEIVED_SHUTDOWN) | ||
| return (shutdown & SSL_RECEIVED_SHUTDOWN) != 0; | ||
| #else | ||
| // Some WolfSSL builds expose only wolfSSL_get_shutdown(), not flag macros. | ||
| return shutdown != 0; | ||
| #endif | ||
| } |
There was a problem hiding this comment.
has_peer_shutdown fallback may misclassify our own close_notify as peer shutdown.
The else branch (Line 111) treats any non-zero wolfSSL_get_shutdown() value as peer shutdown. However, the return value is a bitmask that can include a "sent" flag (we initiated close_notify) without a "received" flag. At Line 530 in do_read_some, this distinction determines whether the caller sees eof vs stream_truncated — reporting eof when only our own close_notify was sent could mask a truncation attack.
If the flag macros are truly unavailable, there's no clean way to distinguish, but the current fallback silently weakens the security guarantee. Consider at minimum adding a comment at the call site (Line 530) noting this known limitation, or — if feasible — a build-time #error or static_assert when neither macro is defined, to catch this at compile time rather than silently degrading.
🤖 Prompt for AI Agents
In `@src/wolfssl/src/wolfssl_stream.cpp` around lines 101 - 113, The fallback in
has_peer_shutdown() can misclassify a locally-sent close_notify as peer shutdown
because it treats any non-zero wolfSSL_get_shutdown() as "received"; change the
behavior to avoid silently weakening security by (A) replacing the else-branch
with a conservative default that does NOT assume peer shutdown (e.g., return
false) and add a clear comment in do_read_some referencing has_peer_shutdown()
about the limitation when flag macros are unavailable, and/or (B) fail fast at
compile-time when neither WOLFSSL_RECEIVED_SHUTDOWN nor SSL_RECEIVED_SHUTDOWN is
defined (use `#error` or a static_assert-style preprocessor check) so builds must
explicitly handle the missing macros; refer to has_peer_shutdown and
do_read_some when making the change.
Keep OpenSSL and WolfSSL error-domain checks fully separate and normalize shutdown read errors with backend-local logic to avoid cross-backend constant mixing.
2 participants
Summary by CodeRabbit
Refactor
Bug Fixes
Chores