Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Found by: pym67 and PeGaSuS
Patch by: michaelortmann with help from the whole eggheads crew
Fixes: #1496
One-line summary:
When there are more than 511 bytes available to
SSL_read()
from an ssl sock, eggdrop would notSSL_read()
the next bytes, until more data arrives for this sock.Additional description (if needed):
For ssl connections, eggdrop grabs max 511 bytes via
SSL_read()
. If there where more bytes, the next call toselect()
insockread()
does not fire for such sock. Afterselect()
there is a call toSSL_pending()
, but thats not reached, because select doesnt fire for the sock, but only for the "secondly"select()
timeout (heartbeat) und thus returns0
. Only, when new data arrives, like in our example, when an IRC server sends a "Registration Timeout" message (because eggy didnt read and respond to the PING), only then eggy reads again, and again only up to those 511 bytes. Now it will read the PING the server was waiting for us to respond to, but its too late and the server already disconnects us.In out testcase the server (UnrealIRCd-6.1.1.1) sends more than 511 bytes because of a long "CAP 302 multiline reply".
This patch checks for
SSL_pending()
before theselect()
and then treats the sock as ifselect()
would have fired for it.The bug was probably effecting many ssl conections, but its hard to notice, if the other end of the ssl connection constantly sends traffic triggering new
SSL_read()
s. In such case i would expect only a delay here and there.This patch is likely to fix #1496, because the commit f9d8af4 referenced there introduced a change to not handle
SSL_pending()
for whenselect()
doesnt fire for the sock:f9d8af4#diff-e83dab64ed96e8a50f94260f6a3dea5712ef4150a7c2aa3c1c282d4227c93b66R923-R924
So, the bug was introduced with eggdrop 1.9.3rc1
Now, that the bug got fixed, if we look at the size of the buffer that we read into in
sockread()
: Some time ago the buffer size was upped because of tags and is currently above 8k.SSL_read()
will return max 16K, see man page. So, this PR now also upps the "grab" value from 511 toRECVLINEMAX-1
. This is more efficient forSSL_read()
,read()
and eggdrops IO buffer handling.Please, can anyone check, that #1496 is indeed fixed by this PR?
Test cases demonstrating functionality (if applicable):
For the following tests, finegrained timestamps, see #1087 and #1441 and additional debug was added.
Before:
https://pastebin.com/A7HFC9jV
After:
https://pastebin.com/0aCMzgY4