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) almost 10 years ago. Updated almost 10 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) almost 10 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) almost 10 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) almost 10 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) almost 10 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) almost 10 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) almost 10 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) almost 10 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 almost 10 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