kornelski / kornelski/rust-security-framework
write() should not return the error from write_func()
- Dominant language
- Rust
- Stars
- 304
- Forks
- 105
- PR merge metrics
- No merged PRs in 30d
Description
First of all thanks a lot for this lib, helps a lot writing stuff for apple ecosystem.
As far as I understand from apple documentation, SSLWrite() will call SSLWriteFunc(), but if there is any kind of error returned by SSLWriteFunc(), SSLWrite() itself will return an error of its own, ie there is no responsibility the coder has to "save" the error returned by SSLWriteFunc() and then check that after calling SSLWrite() etc.. In fact, doing that might be erroneous - consider this example.
1. We called SSLWrite(), SSLWriteFunc() might have returned errSslWouldBlock, but SSLWrite() itself succeeded because SSL lib is gonna buffer up the data we provide (and encrypt it and call SSLWriteFunc() on the encrypted data) - so the library has now "cached" an error errSslWouldBlock in conn.err
2. Later again we call SSLWrite(), this time just as an example say there was some error and the API returned errSSLPeerCertExpired
3. Now write() will go and check conn.err and will find errSslWouldBlock and that is what the caller to write() gets as an error, which is incorrect
So in summary, IMO there is no need to cache conn.err and return it - unless there was a reason for it, which ill be curious to know
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by tracing the write() path and its interaction with SSLWrite() and SSLWriteFunc(). Verify how callback errors are stored and returned, then confirm that write() reports the SSLWrite() result rather than an earlier cached callback error; the issue's three-step example describes the behavior to test.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- macos, rust
- Domain
- security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100