openssl 3.0.0 / 1.1.1.e introduced a behavior change. When the peer s… #2138
Conversation
…imply closes the connection without notifying its client, a specific error is now generated by SSL. With this commit, ACE_SSL handles this error gracefully. Also, this patch includes some minor code cleanup such as more consistent error handling in the same file.
WalkthroughThe changes update the error handling and control flow within the Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Stream as ACE_SSL_SOCK_Stream::recv_i
participant OpenSSL as OpenSSL Lib
participant Error as Error Handler
App->>Stream: Call recv_i()
Stream->>OpenSSL: Perform SSL_read
OpenSSL-->>Stream: Result/Error Code
Stream->>Stream: Call SSL_get_error()
alt Error is SSL_ERROR_SSL with unexpected EOF
Stream->>Error: Process default error handling
else Error is SSL_ERROR_WANT_READ/WANT_WRITE
Stream->>Stream: Check for timeout
alt No timeout
Stream->>Error: Set errno to EWOULDBLOCK
end
else Other errors
Stream->>Error: Handle error normally
end
Stream-->>App: Return result or error
sequenceDiagram
participant App as Application
participant Stream as ACE_SSL_SOCK_Stream::close
participant OpenSSL as OpenSSL Lib
participant Error as Error Reporter
App->>Stream: Call close()
Stream->>OpenSSL: Retrieve error status via SSL_get_error
OpenSSL-->>Stream: Error status (status_2)
Stream->>Error: Call ACE_OS::set_errno_to_last_error()
Stream-->>App: Return close status
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ACE/ace/SSL/SSL_SOCK_Stream.inl (1)
191-232: Conditional logic for unexpected OpenSSL EOF looks correct; consider avoidinggoto.
Usinggoto default_is functional and follows ACE's style in some areas, yet it can reduce clarity. A structured approach or direct branching might be preferable in more modern C++.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ACE/ace/SSL/SSL_SOCK_Stream.inl(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (21)
- GitHub Check: ubuntu-20.04 g++-10 CodeQL
- GitHub Check: ubuntu-20.04 g++-10 ACE for TAO
- GitHub Check: ubuntu-20.04 g++-10 CORBA/e compact
- GitHub Check: ubuntu-22.04 clang++-16
- GitHub Check: ubuntu-22.04 clang++-15
- GitHub Check: ubuntu-22.04 clang++-14
- GitHub Check: ubuntu-20.04 clang++-13
- GitHub Check: ubuntu-20.04 clang++-12
- GitHub Check: ubuntu-20.04 clang++-11
- GitHub Check: ubuntu-20.04 clang++-10
- GitHub Check: ubuntu-20.04 clang++-9
- GitHub Check: ubuntu-20.04 clang++-8
- GitHub Check: ubuntu-20.04 clang++-7
- GitHub Check: ubuntu-20.04 clang++-6.0
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: windows-2019 vs2019
- GitHub Check: macos-14-C++
- GitHub Check: ubuntu-22.04 g++-12
- GitHub Check: macos-13-C++
- GitHub Check: ubuntu-20.04 g++-10
- GitHub Check: alpine-3.18
🔇 Additional comments (3)
ACE/ace/SSL/SSL_SOCK_Stream.inl (3)
2-3: Includes look appropriate.
These headers are correctly added for OpenSSL-specific functionality and refined error handling.
159-180: Ensure negative return fromACE::handle_ready()is handled.
WhenACE::handle_ready()returns-1, the code setsbytes_readto-1but doesn't explicitly seterrno. Verify thatACE::handle_ready()or subsequent code paths properly seterrnoso that upstream callers can accurately diagnose the failure.
346-376: Refined error handling inclose()is consistent and clear.
Storing the result ofSSL_get_error()in a separate variable and explicitly resetting the handle upon error helps maintain clarity and consistency. No issues found.
…imply closes the connection without notifying its client, a specific error is now generated by SSL. With this commit, ACE_SSL handles this error gracefully.
Also, this patch includes some minor code cleanup such as more consistent error handling in the same file.
Summary by CodeRabbit