Project

General

Profile

Actions

Bug #4070

closed

possible XMLRPC connectino leak

Added by shyouhei (Shyouhei Urabe) about 12 years ago. Updated over 11 years ago.

Status:
Rejected
Priority:
Normal
Target version:
ruby -v:
ruby 1.9.3dev (2010-11-16 trunk 29806) [x86_64-linux]
Backport:
[ruby-core:33254]

Description

=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

Actions #1

Updated by tarui (Masaya Tarui) about 12 years ago

=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

Updated by naruse (Yui NARUSE) over 11 years ago

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

Updated by nahi (Hiroshi Nakamura) over 11 years ago

  • Target version set to 1.9.3

Updated by nahi (Hiroshi Nakamura) over 11 years ago

  • 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.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0