Bug #11400
closedIO.gets(/\x0d?\x0a\x0d?\x0a/, 4096) raises comparison of Fixnum with nil failed
Description
This is my code, "socket" is a SSLSocket this is why it allows a Regexp in gets
begin http_response[:http_headers] = WEBrick::Utils.timeout(3){ socket.gets(/\x0d?\x0a\x0d?\x0a/, 4096) #\r?\n\r?\n } rescue Errno::ECONNRESET return nil rescue Timeout::Error LOGGER_SYSLOG_SYSLOG.syslog_error('e', 'Timeout when reading aware') rescue Exception => e LOGGER_SYSLOG_SYSLOG.syslog_error('e', "Error Reading Aware: #{e}") retry unless socket.eof? || socket.closed? end
The error reads like this: "Error Reading Aware: comparison of Fixnum with nil failed"
Which means it comes from this begin\rescue block, but there is nothing inside it which would compare a Fixnum with nil, so, I suspect that gets does it ?
Updated by bararchy (Bar Hofesh) over 9 years ago
- Subject changed from IO.gets("\r\n", 4096) returns comparison of Fixnum with nil failed to IO.gets(/\x0d?\x0a\x0d?\x0a/, 4096) raises comparison of Fixnum with nil failed
Updated by bararchy (Bar Hofesh) over 9 years ago
Further Testing show that this happens (less so) when Regexp is not used, as in IO.gets("\r\n\r\n")
Updated by normalperson (Eric Wong) over 9 years ago
Since you're using SSLSocket, I recommend dumping a backtrace when you
rescue since gets is implemented in Ruby in the OpenSSL::Buffering module
(ext/openssl/lib/openssl/buffering.rb in the Ruby source)
Updated by bararchy (Bar Hofesh) over 9 years ago
Eric Wong wrote:
Since you're using SSLSocket, I recommend dumping a backtrace when you
rescue since gets is implemented in Ruby in the OpenSSL::Buffering module
(ext/openssl/lib/openssl/buffering.rb in the Ruby source)
Error Reading Aware: comparison of Fixnum with nil failed
Error Reading Aware: ["/usr/local/rvm/rubies/ruby-2.2.1/lib/ruby/2.2.0/openssl/buffering.rb:217:in each'", "/usr/local/rvm/rubies/ruby-2.2.1/lib/ruby/2.2.0/openssl/buffering.rb:217:in
min'", "/usr/local/rvm/rubies/ruby-2.2.1/lib/ruby/2.2.0/openssl/buffering.rb:217:in gets'", "/opt/Zakif/Core/Core_Http_Aware_Read.rb:10:in
block in read'", "/usr/local/rvm/gems/ruby-2.2.1/gems/webrick-1.3.1/lib/webrick/utils.rb:234:in timeout'", "/opt/Zakif/Core/Core_Http_Aware_Read.rb:9:in
read'", "/opt/Zakif/Proxies/Proxies_Ldap.rb:169:in block (2 levels) in tcp_proxy'", "/opt/Zakif/Proxies/Proxies_Ldap.rb:135:in
each'", "/opt/Zakif/Proxies/Proxies_Ldap.rb:135:in block in tcp_proxy'", "/opt/Zakif/Proxies/Proxies_Ldap.rb:130:in
loop'", "/opt/Zakif/Proxies/Proxies_Ldap.rb:130:in tcp_proxy'", "/opt/Zakif/Proxies/Proxies_Ldap.rb:410:in
first_data_exchange_after_login'", "/opt/Zakif/Proxies/Proxies_Ldap.rb:234:in http_login'", "/opt/Zakif/Proxies/Proxies_Ldap.rb:96:in
block (2 levels) in main_loop_device'"]
Does this help ?
Updated by bararchy (Bar Hofesh) over 9 years ago
Looking at the code it's clear the offending line is https://github.com/ruby/ruby/blob/trunk/ext/openssl/lib/openssl/buffering.rb#L216
Where sometimes size would be nil, and because of "size = [size, limit].min" or more specifically "size = [nil, 4096].min" in my case
So:
[nil, 4096].min ArgumentError: comparison of Fixnum with nil failed from (pry):1:in
each'`
You should use size.to_i or something :)
Updated by normalperson (Eric Wong) over 9 years ago
bar.hofesh@safe-t.com wrote:
Looking at the code it's clear the offending line is https://github.com/ruby/ruby/blob/trunk/ext/openssl/lib/openssl/buffering.rb#L216
Where sometimes size would be nil, and because of "size = [size, limit].min" or more specifically
So:
[nil, 4096].min ArgumentError: comparison of Fixnum with nil failed from (pry):1:in
each'`
Thanks.
You should use size.to_i or something :)
size.to_i seems wrong since it would give zero. consume_rbuf
can handle size=nil, so I think we should continue passing nil
to consume_rbuff in this case.
Totally untested patch below:
--- a/ext/openssl/lib/openssl/buffering.rb
+++ b/ext/openssl/lib/openssl/buffering.rb
@@ -213,7 +213,7 @@ module OpenSSL::Buffering
else
size = idx ? idx+eol.size : nil
end
- if limit and limit >= 0
- if size && limit && limit >= 0
size = [size, limit].min
end
consume_rbuff(size)
Care to test? Thanks!
Would also be helpful to write a standalone test case so we can commit
it to our test suite.
Updated by bararchy (Bar Hofesh) over 9 years ago
Small and elegant fix, after a few hours of testings (stress and SSL errors), it seems that your patch is working and isn't breaking anything I can see.
Thanks
Updated by Anonymous over 9 years ago
- Status changed from Open to Closed
Applied in changeset r51466.
openssl/buffering: fix gets on EOF with limit
- ext/openssl/lib/openssl/buffering.rb (gets):
avoid comparing fixnum with nil - test/openssl/test_pair.rb: test gets with limit when EOF is hit
Thanks to Bar Hofesh bar.hofesh@safe-t.com for the bug report
and testing.
[ruby-core:70149] [Bug #11400]