Bug #4070: possible XMLRPC connectino leak

• Status: Rejected
• Assignee: nahi (Hiroshi Nakamura)
• Target version: 1.9.3
• ruby -v: ruby 1.9.3dev (2010-11-16 trunk 29806) [x86_64-linux]

[shyouhei (Shyouhei Urabe)]

=begin
Hi all.

We got a pull request that claims xmlrpc to have a connection leak.

http://github.com/ruby/ruby/pull/1

But I'm not sure if the code posted there is a "proper" use of XMLRPC module. XMLRPC::Client instance seems to keep a TCP sessions and can issue many RPC calls from it, no need to recreate instances every time. It seems to me that the reporter's code unnecessarily opens too many TCP sessions at once and acts like a kind of DoS attack to a server. Proposed fix ensures this usage and disables TCP session pooling.

Is this a right fix that I can pull into? or should we have a better way?
=end

[tarui (Masaya Tarui)]

=begin
Hi,

It is not client side's problem. it's server side's one.
now, XMLRPC::Server.new() uses Webrick with maxConnections=4.

It's very poor, so server should not allow keep-alive.
I think that below patch is better. Even if it is not best.

Index: lib/xmlrpc/server.rb

--- lib/xmlrpc/server.rb (revision 29847)
+++ lib/xmlrpc/server.rb (working copy)
@@ -635,6 +635,7 @@
super(*a)
require 'webrick'
@server = WEBrick::HTTPServer.new(:Port => port, :BindAddress => host,
:MaxClients => maxConnections,

•                                  :HTTPVersion    =>
  

HTTPVersion.new("1.0"),
:Logger => WEBrick::Log.new(stdlog))
@server.mount("/", self)
end

--
Masaya TARUI
No Tool,No Life.

Hi,

It is not client side's problem. it's server side's one.
now, XMLRPC::Server.new() uses Webrick with maxConnections=4.

It's very poor, so server should not allow keep-alive.

I think that below patch is better. Even if it is not best.

Index: lib/xmlrpc/server.rb
===================================================================
--- lib/xmlrpc/server.rb        (revision 29847)

+++ lib/xmlrpc/server.rb        (working copy)
@@ -635,6 +635,7 @@
     super(*a)
     require 'webrick'
     @server = WEBrick::HTTPServer.new(:Port => port, :BindAddress => host, :MaxClients => maxConnections,

+                                      :HTTPVersion    => HTTPVersion.new("1.0"),
                                       :Logger => WEBrick::Log.new(stdlog))
     @server.mount("/", self)

   end

--
Masaya TARUI
No Tool,No Life.


=end

[naruse (Yui NARUSE)]

• Status changed from Open to Assigned
• Assignee set to nahi (Hiroshi Nakamura)

[nahi (Hiroshi Nakamura)]

• Target version set to 1.9.3

[nahi (Hiroshi Nakamura)]

• Status changed from Assigned to Rejected

Shyouhei, I added a comment to the pull request. We can use call_async instead of call for non Keep-Alive TCP connection which is closed per connection. I think that method name is awfully confusing but we don't want to touch xmlrpc lib as far as possible.

Taru, I agree with you that :MacClients == 4 is too small. But we have macConnections parameter for configure and it would be enough.

I close this ticket as 'Not a Bug'. Please ask me if you think I'm wrong.