Handling of errors - like unstable network - coming via SSL - #89
Handling of errors - like unstable network - coming via SSL#89bligeti wants to merge 25 commits into
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.
fhessel
left a comment
There was a problem hiding this comment.
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) {
// ...
} elseif (readReturnCode == 0) {
// ...
} else {
// dead code
}So I would be curious if you ever saw that added SSL_error=... message in your logs?
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Frank Hessel <frnkhessel@googlemail.com>
Co-authored-by: Frank Hessel <frnkhessel@googlemail.com>
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 <frnkhessel@googlemail.com> * Update src/HTTPConnection.cpp Co-authored-by: Frank Hessel <frnkhessel@googlemail.com> * correct the copy-paste error Co-authored-by: bligeti <64783210+bligeti@users.noreply.github.com> Co-authored-by: Frank Hessel <frnkhessel@googlemail.com>
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.