Project

General

Profile

Actions

Bug #11400

closed

IO.gets(/\x0d?\x0a\x0d?\x0a/, 4096) raises comparison of Fixnum with nil failed

Added by bararchy (Bar Hofesh) over 9 years ago. Updated over 9 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 2.2.1p85 (2015-02-26 revision 49769) [x86_64-linux]
[ruby-core:70149]

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

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

Actions #8

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 for the bug report
    and testing.
    [ruby-core:70149] [Bug #11400]
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0