-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handling of errors - like unstable network - coming via SSL #89
base: master
Are you sure you want to change the base?
Conversation
In case closeConnection() is called due to SSL reported error - like session disconnection due to unstable Wifi - onClose() should be called to free up the recorded websocket handlers.
From updateBuffer() a session clean up is initiated via closeConnection() if error is detected when reading from SSL but the already removed session is not handled in other parts of the state machine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for providing the PR, I'll try to check it on a board asap.
However, during the code review I noticed this line (which has been there already before the PR):
size_t HTTPSConnection::readBytesToBuffer(byte* buffer, size_t length) {
Using size_t
instead of ssize_t
means that no underlying SSL error will ever be reported to the caller. That alone will cause trouble when using readBytesToBuffer()
. The same goes for passing the return value of recv()
in the basic HTTPConnection
variant.
In particular, this means that the else
branch in HTTPConnection.cpp:195..212
will never be reached with an int readReturnCode
being set based on a size_t
.
if (readReturnCode > 0) {
// ...
} else if (readReturnCode == 0) {
// ...
} else {
// dead code
}
So I would be curious if you ever saw that added SSL_error=...
message in your logs?
Co-authored-by: Frank Hessel <[email protected]>
Co-authored-by: Frank Hessel <[email protected]>
Merge fhessel#89 from fhessel/esp32_https_server
* Trigger wsHander onClose from closeConnection() In case closeConnection() is called due to SSL reported error - like session disconnection due to unstable Wifi - onClose() should be called to free up the recorded websocket handlers. * Handle error triggered by closed sessions From updateBuffer() a session clean up is initiated via closeConnection() if error is detected when reading from SSL but the already removed session is not handled in other parts of the state machine. * Get detailed SSL_read error * Update src/HTTPSConnection.cpp Co-authored-by: Frank Hessel <[email protected]> * Update src/HTTPConnection.cpp Co-authored-by: Frank Hessel <[email protected]> * correct the copy-paste error Co-authored-by: bligeti <[email protected]> Co-authored-by: Frank Hessel <[email protected]>
May help in crash fix for websockets.
Update as mentioned in issue fhessel#89 for possible crash fix on websockets.
Merge some upstream PRs - Handling of errors - like unstable network - coming via SSL fhessel#89 - WIP: Prevent crash on WebSocket request to non-WebSocket node. fhessel#106 - Fix infinite loop when the buffer ends with \r fhessel#123 - Fixed memory leak in the Websocket example fhessel#157
1. Merge some upstream PRs - Handling of errors - like unstable network - coming via SSL fhessel#89 - WIP: Prevent crash on WebSocket request to non-WebSocket node. fhessel#106 - Fix infinite loop when the buffer ends with \r fhessel#123 - Fixed memory leak in the Websocket example fhessel#157 2. Update examples and `README.md`
1. Merge some upstream PRs - Handling of errors - like unstable network - coming via SSL fhessel#89 - WIP: Prevent crash on WebSocket request to non-WebSocket node. fhessel#106 - Fix infinite loop when the buffer ends with \r fhessel#123 - Fixed memory leak in the Websocket example fhessel#157 2. Update examples and `README.md`
1. Merge some upstream PRs - Handling of errors - like unstable network - coming via SSL fhessel#89 - WIP: Prevent crash on WebSocket request to non-WebSocket node. fhessel#106 - Fix infinite loop when the buffer ends with \r fhessel#123 - Fixed memory leak in the Websocket example fhessel#157 2. Update examples and `README.md`
… request to non-WebSocket node.
…s (like curl) who ignores http 401 will connect to websocket. This fix checks if the handshake was really done (HTTP 101 was replied).
Possible fix for #85
The trigger of this problem can be network disconnections or similar errors coming via SSL.
In HTTPConnection::updateBuffer() closeConnection() is called when error is detected from SSL to clean up but the possibility of already cleaned up session is not handled in HTTPConnection::pendingBufferSize() and in HTTPConnection::loop().
Also in such case WSHandler::onClose() is not called therefore the recorded handlers (like in the websocket example) are not cleaned up.