ruby-changes:34137
From: akr <ko1@a...>
Date: Thu, 29 May 2014 00:42:18 +0900 (JST)
Subject: [ruby-changes:34137] akr:r46218 (trunk): * ext/socket/unixsocket.c (rsock_init_unixsock): Open a socket
akr 2014-05-29 00:42:09 +0900 (Thu, 29 May 2014) New Revision: 46218 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?revision=46218&view=revision Log: * ext/socket/unixsocket.c (rsock_init_unixsock): Open a socket after path length check. This fixes a fd leak by TestSocket_UNIXSocket#test_too_long_path. Modified files: trunk/ChangeLog trunk/ext/socket/unixsocket.c trunk/test/socket/test_socket.rb trunk/test/socket/test_tcp.rb trunk/test/socket/test_unix.rb Index: ChangeLog =================================================================== --- ChangeLog (revision 46217) +++ ChangeLog (revision 46218) @@ -1,3 +1,9 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1 +Thu May 29 00:28:56 2014 Tanaka Akira <akr@f...> + + * ext/socket/unixsocket.c (rsock_init_unixsock): Open a socket + after path length check. + This fixes a fd leak by TestSocket_UNIXSocket#test_too_long_path. + Wed May 28 23:04:35 2014 Tanaka Akira <akr@f...> * test/ruby/test_io.rb (test_flush_in_finalizer1): Use Index: ext/socket/unixsocket.c =================================================================== --- ext/socket/unixsocket.c (revision 46217) +++ ext/socket/unixsocket.c (revision 46218) @@ -34,10 +34,6 @@ rsock_init_unixsock(VALUE sock, VALUE pa https://github.com/ruby/ruby/blob/trunk/ext/socket/unixsocket.c#L34 rb_io_t *fptr; SafeStringValue(path); - fd = rsock_socket(AF_UNIX, SOCK_STREAM, 0); - if (fd < 0) { - rsock_sys_fail_path("socket(2)", path); - } INIT_SOCKADDR_UN(&sockaddr, sizeof(struct sockaddr_un)); if (sizeof(sockaddr.sun_path) < (size_t)RSTRING_LEN(path)) { @@ -47,6 +43,11 @@ rsock_init_unixsock(VALUE sock, VALUE pa https://github.com/ruby/ruby/blob/trunk/ext/socket/unixsocket.c#L43 memcpy(sockaddr.sun_path, RSTRING_PTR(path), RSTRING_LEN(path)); sockaddrlen = rsock_unix_sockaddr_len(path); + fd = rsock_socket(AF_UNIX, SOCK_STREAM, 0); + if (fd < 0) { + rsock_sys_fail_path("socket(2)", path); + } + if (server) { status = bind(fd, (struct sockaddr*)&sockaddr, sockaddrlen); } Index: test/socket/test_unix.rb =================================================================== --- test/socket/test_unix.rb (revision 46217) +++ test/socket/test_unix.rb (revision 46218) @@ -55,16 +55,20 @@ class TestSocket_UNIXSocket < Test::Unit https://github.com/ruby/ruby/blob/trunk/test/socket/test_unix.rb#L55 ret = s2.recvmsg(:scm_rights=>true) _, _, _, *ctls = ret recv_io_ary = [] - ctls.each {|ctl| - next if ctl.level != Socket::SOL_SOCKET || ctl.type != Socket::SCM_RIGHTS - recv_io_ary.concat ctl.unix_rights - } - assert_equal(send_io_ary.length, recv_io_ary.length) - send_io_ary.length.times {|i| - assert_not_equal(send_io_ary[i].fileno, recv_io_ary[i].fileno) - assert(File.identical?(send_io_ary[i], recv_io_ary[i])) - assert(recv_io_ary[i].close_on_exec?) - } + begin + ctls.each {|ctl| + next if ctl.level != Socket::SOL_SOCKET || ctl.type != Socket::SCM_RIGHTS + recv_io_ary.concat ctl.unix_rights + } + assert_equal(send_io_ary.length, recv_io_ary.length) + send_io_ary.length.times {|i| + assert_not_equal(send_io_ary[i].fileno, recv_io_ary[i].fileno) + assert(File.identical?(send_io_ary[i], recv_io_ary[i])) + assert(recv_io_ary[i].close_on_exec?) + } + ensure + recv_io_ary.each {|io| io.close if !io.closed? } + end } } ensure @@ -92,16 +96,20 @@ class TestSocket_UNIXSocket < Test::Unit https://github.com/ruby/ruby/blob/trunk/test/socket/test_unix.rb#L96 ret = s2.recvmsg(:scm_rights=>true) _, _, _, *ctls = ret recv_io_ary = [] - ctls.each {|ctl| - next if ctl.level != Socket::SOL_SOCKET || ctl.type != Socket::SCM_RIGHTS - recv_io_ary.concat ctl.unix_rights - } - assert_equal(send_io_ary.length, recv_io_ary.length) - send_io_ary.length.times {|i| - assert_not_equal(send_io_ary[i].fileno, recv_io_ary[i].fileno) - assert(File.identical?(send_io_ary[i], recv_io_ary[i])) - assert(recv_io_ary[i].close_on_exec?) - } + begin + ctls.each {|ctl| + next if ctl.level != Socket::SOL_SOCKET || ctl.type != Socket::SCM_RIGHTS + recv_io_ary.concat ctl.unix_rights + } + assert_equal(send_io_ary.length, recv_io_ary.length) + send_io_ary.length.times {|i| + assert_not_equal(send_io_ary[i].fileno, recv_io_ary[i].fileno) + assert(File.identical?(send_io_ary[i], recv_io_ary[i])) + assert(recv_io_ary[i].close_on_exec?) + } + ensure + recv_io_ary.each {|io| io.close if !io.closed? } + end } } ensure @@ -248,31 +256,43 @@ class TestSocket_UNIXSocket < Test::Unit https://github.com/ruby/ruby/blob/trunk/test/socket/test_unix.rb#L256 tmpfile = Tempfile.new("s") path = tmpfile.path tmpfile.close(true) - yield klass.new(path), path + io = klass.new(path) + yield io, path ensure + io.close File.unlink path if path && File.socket?(path) end def test_addr bound_unix_socket(UNIXServer) {|serv, path| - c = UNIXSocket.new(path) - s = serv.accept - assert_equal(["AF_UNIX", path], c.peeraddr) - assert_equal(["AF_UNIX", ""], c.addr) - assert_equal(["AF_UNIX", ""], s.peeraddr) - assert_equal(["AF_UNIX", path], s.addr) - assert_equal(path, s.path) - assert_equal("", c.path) + UNIXSocket.open(path) {|c| + s = serv.accept + begin + assert_equal(["AF_UNIX", path], c.peeraddr) + assert_equal(["AF_UNIX", ""], c.addr) + assert_equal(["AF_UNIX", ""], s.peeraddr) + assert_equal(["AF_UNIX", path], s.addr) + assert_equal(path, s.path) + assert_equal("", c.path) + ensure + s.close + end + } } end def test_cloexec bound_unix_socket(UNIXServer) {|serv, path| - c = UNIXSocket.new(path) - s = serv.accept - assert(serv.close_on_exec?) - assert(c.close_on_exec?) - assert(s.close_on_exec?) + UNIXSocket.open(path) {|c| + s = serv.accept + begin + assert(serv.close_on_exec?) + assert(c.close_on_exec?) + assert(s.close_on_exec?) + ensure + s.close + end + } } end @@ -393,12 +413,13 @@ class TestSocket_UNIXSocket < Test::Unit https://github.com/ruby/ruby/blob/trunk/test/socket/test_unix.rb#L413 end def test_epipe # [ruby-dev:34619] - s1, s2 = UNIXSocket.pair - s1.shutdown(Socket::SHUT_WR) - assert_raise(Errno::EPIPE) { s1.write "a" } - assert_equal(nil, s2.read(1)) - s2.write "a" - assert_equal("a", s1.read(1)) + UNIXSocket.pair {|s1, s2| + s1.shutdown(Socket::SHUT_WR) + assert_raise(Errno::EPIPE) { s1.write "a" } + assert_equal(nil, s2.read(1)) + s2.write "a" + assert_equal("a", s1.read(1)) + } end def test_socket_pair_with_block @@ -463,15 +484,21 @@ class TestSocket_UNIXSocket < Test::Unit https://github.com/ruby/ruby/blob/trunk/test/socket/test_unix.rb#L484 return if /linux/ !~ RUBY_PLATFORM Dir.mktmpdir {|d| sockpath = "#{d}/sock" - serv = Socket.unix_server_socket(sockpath) - Socket.unix(sockpath) - s, = serv.accept - cred = s.getsockopt(:SOCKET, :PEERCRED) - inspect = cred.inspect - assert_match(/ pid=#{$$} /, inspect) - assert_match(/ euid=#{Process.euid} /, inspect) - assert_match(/ egid=#{Process.egid} /, inspect) - assert_match(/ \(ucred\)/, inspect) + Socket.unix_server_socket(sockpath) {|serv| + Socket.unix(sockpath) {|c| + s, = serv.accept + begin + cred = s.getsockopt(:SOCKET, :PEERCRED) + inspect = cred.inspect + assert_match(/ pid=#{$$} /, inspect) + assert_match(/ euid=#{Process.euid} /, inspect) + assert_match(/ egid=#{Process.egid} /, inspect) + assert_match(/ \(ucred\)/, inspect) + ensure + s.close + end + } + } } end @@ -493,18 +520,24 @@ class TestSocket_UNIXSocket < Test::Unit https://github.com/ruby/ruby/blob/trunk/test/socket/test_unix.rb#L520 return if /linux/ !~ RUBY_PLATFORM Dir.mktmpdir {|d| sockpath = "#{d}/sock" - serv = Socket.unix_server_socket(sockpath) - c = Socket.unix(sockpath) - s, = serv.accept - s.setsockopt(:SOCKET, :PASSCRED, 1) - c.print "a" - msg, _, _, cred = s.recvmsg - inspect = cred.inspect - assert_equal("a", msg) - assert_match(/ pid=#{$$} /, inspect) - assert_match(/ uid=#{Process.uid} /, inspect) - assert_match(/ gid=#{Process.gid} /, inspect) - assert_match(/ \(ucred\)/, inspect) + Socket.unix_server_socket(sockpath) {|serv| + Socket.unix(sockpath) {|c| + s, = serv.accept + begin + s.setsockopt(:SOCKET, :PASSCRED, 1) + c.print "a" + msg, _, _, cred = s.recvmsg + inspect = cred.inspect + assert_equal("a", msg) + assert_match(/ pid=#{$$} /, inspect) + assert_match(/ uid=#{Process.uid} /, inspect) + assert_match(/ gid=#{Process.gid} /, inspect) + assert_match(/ \(ucred\)/, inspect) + ensure + s.close + end + } + } } end @@ -550,14 +583,18 @@ class TestSocket_UNIXSocket < Test::Unit https://github.com/ruby/ruby/blob/trunk/test/socket/test_unix.rb#L583 def test_getpeereid Dir.mktmpdir {|d| path = "#{d}/sock" - serv = Socket.unix_server_socket(path) - c = Socket.unix(path) - s, = serv.accept - begin - assert_equal([Process.euid, Process.egid], c.getpeereid) - assert_equal([Process.euid, Process.egid], s.getpeereid) - rescue NotImplementedError - end + Socket.unix_server_socket(path) {|serv| + Socket.unix(path) {|c| + s, = serv.accept + begin + assert_equal([Process.euid, Process.egid], c.getpeereid) + assert_equal([Process.euid, Process.egid], s.getpeereid) + rescue NotImplementedError + ensure + s.close + end + } + } } end Index: test/socket/test_tcp.rb =================================================================== --- test/socket/test_tcp.rb (revision 46217) +++ test/socket/test_tcp.rb (revision 46218) @@ -45,36 +45,35 @@ class TestSocket_TCPSocket < Test::Unit: https://github.com/ruby/ruby/blob/trunk/test/socket/test_tcp.rb#L45 end def test_recvfrom - svr = TCPServer.new("localhost", 0) - th = Thread.new { - c = svr.accept - c.write "foo" - c.close + TCPServer.open("localhost", 0) {|svr| + th = Thread.new { + c = svr.accept + c.write "foo" + c.close + } + addr = svr.addr + TCPSocket.open(addr[3], addr[1]) {|sock| + assert_equal(["foo", nil], sock.recvfrom(0x10000)) + } + th.join } - addr = svr.addr - sock = TCPSocket.open(addr[3], addr[1]) - assert_equal(["foo", nil], sock.recvfrom(0x10000)) - ensure - th.kill if th - th.join if th end def test_encoding - svr = TCPServer.new("localhost", 0) - th = Thread.new { - c = svr.accept - c.write "foo\r\n" - c.close + TCPServer.open("localhost", 0) {|svr| + th = Thread.new { + c = svr.accept + c.write "foo\r\n" + c.close + } + addr = svr.addr + TCPSocket.open(addr[3], addr[1]) {|sock| + assert_equal(true, sock.binmode?) + s = sock.gets + assert_equal("foo\r\n", s) + assert_equal(Encoding.find("ASCII-8BIT"), s.encoding) + } + th.join } - addr = svr.addr - sock = TCPSocket.open(addr[3], addr[1]) - assert_equal(true, sock.binmode?) - s = sock.gets - assert_equal("foo\r\n", s) - assert_equal(Encoding.find("ASCII-8BIT"), s.encoding) - ensure - th.kill if th - th.join if th - sock.close if sock end end if defined?(TCPSocket) Index: test/socket/test_socket.rb =================================================================== --- test/socket/test_socket.rb (revision 46217) +++ test/socket/test_socket.rb (revision 46218) @@ -538,6 +538,7 @@ class TestSocket < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/socket/test_socket.rb#L538 assert_raise(IOError, bug4390) {client_thread.join} end ensure + serv_thread.value.close server.close end -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/