Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: b5835add0004
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 3813f0a53c22
Choose a head ref
  • 6 commits
  • 2 files changed
  • 2 contributors

Commits on Mar 1, 2018

  1. Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
    Copy the full SHA
    728778a View commit details
  2. Copy the full SHA
    a607961 View commit details
  3. Copy the full SHA
    f178b68 View commit details
  4. Copy the full SHA
    9df737b View commit details
  5. Copy the full SHA
    987406f View commit details

Commits on Mar 20, 2018

  1. Merge pull request #5074 from quixoten/fd_leak_in_resolv

    Fix file descriptor leak in resolv.rb
    headius authored Mar 20, 2018
    Copy the full SHA
    3813f0a View commit details
Showing with 109 additions and 32 deletions.
  1. +71 −32 lib/ruby/stdlib/resolv.rb
  2. +38 −0 test/mri/resolv/test_dns.rb
103 changes: 71 additions & 32 deletions lib/ruby/stdlib/resolv.rb
Original file line number Diff line number Diff line change
@@ -728,6 +728,10 @@ def close
socks&.each(&:close)
end

def lazy_initialize
self
end

class Sender # :nodoc:
def initialize(msg, data, sock)
@msg = msg
@@ -740,35 +744,47 @@ class UnconnectedUDP < Requester # :nodoc:
def initialize(*nameserver_port)
super()
@nameserver_port = nameserver_port
@socks_hash = {}
@socks = []
nameserver_port.each {|host, port|
if host.index(':')
bind_host = "::"
af = Socket::AF_INET6
else
bind_host = "0.0.0.0"
af = Socket::AF_INET
end
next if @socks_hash[bind_host]
begin
sock = UDPSocket.new(af)
rescue Errno::EAFNOSUPPORT
next # The kernel doesn't support the address family.
end
sock.do_not_reverse_lookup = true
DNS.bind_random_port(sock, bind_host)
@socks << sock
@socks_hash[bind_host] = sock
@mutex = Thread::Mutex.new
@initialized = false
end

def lazy_initialize
@mutex.synchronize {
next if @initialized
@initialized = true
@socks_hash = {}
@socks = []
@nameserver_port.each {|host, port|
if host.index(':')
bind_host = "::"
af = Socket::AF_INET6
else
bind_host = "0.0.0.0"
af = Socket::AF_INET
end
next if @socks_hash[bind_host]
begin
sock = UDPSocket.new(af)
rescue Errno::EAFNOSUPPORT
next # The kernel doesn't support the address family.
end
@socks << sock
@socks_hash[bind_host] = sock
sock.do_not_reverse_lookup = true
DNS.bind_random_port(sock, bind_host)
}
}
self
end

def recv_reply(readable_socks)
lazy_initialize
reply, from = readable_socks[0].recvfrom(UDPSize)
return reply, [IPAddr.new(from[3]),from[1]]
end

def sender(msg, data, host, port=Port)
lazy_initialize
sock = @socks_hash[host.index(':') ? "::" : "0.0.0.0"]
return nil if !sock
service = [IPAddr.new(host), port]
@@ -780,9 +796,14 @@ def sender(msg, data, host, port=Port)
end

def close
super
@senders.each_key {|service, id|
DNS.free_request_id(service[0], service[1], id)
@mutex.synchronize {
if @initialized
super
@senders.each_key {|service, id|
DNS.free_request_id(service[0], service[1], id)
}
@initialized = false
end
}
end

@@ -804,22 +825,34 @@ def send
class ConnectedUDP < Requester # :nodoc:
def initialize(host, port=Port)
super()
@mutex = Thread::Mutex.new
@host = host
@port = port
is_ipv6 = host.index(':')
sock = UDPSocket.new(is_ipv6 ? Socket::AF_INET6 : Socket::AF_INET)
@socks = [sock]
sock.do_not_reverse_lookup = true
DNS.bind_random_port(sock, is_ipv6 ? "::" : "0.0.0.0")
sock.connect(host, port)
@initialized = false
end

def lazy_initialize
@mutex.synchronize {
next if @initialized
@initialized = true
is_ipv6 = @host.index(':')
sock = UDPSocket.new(is_ipv6 ? Socket::AF_INET6 : Socket::AF_INET)
@socks = [sock]
sock.do_not_reverse_lookup = true
DNS.bind_random_port(sock, is_ipv6 ? "::" : "0.0.0.0")
sock.connect(@host, @port)
}
self
end

def recv_reply(readable_socks)
lazy_initialize
reply = readable_socks[0].recv(UDPSize)
return reply, nil
end

def sender(msg, data, host=@host, port=@port)
lazy_initialize
unless host == @host && port == @port
raise RequestError.new("host/port don't match: #{host}:#{port}")
end
@@ -830,9 +863,14 @@ def sender(msg, data, host=@host, port=@port)
end

def close
super
@senders.each_key {|from, id|
DNS.free_request_id(@host, @port, id)
@mutex.synchronize {
if @initialized
super
@senders.each_key {|from, id|
DNS.free_request_id(@host, @port, id)
}
@initialized = false
end
}
end

@@ -847,6 +885,7 @@ def send

class MDNSOneShot < UnconnectedUDP # :nodoc:
def sender(msg, data, host, port=Port)
lazy_initialize
id = DNS.allocate_request_id(host, port)
request = msg.encode
request[0,2] = [id].pack('n')
38 changes: 38 additions & 0 deletions test/mri/resolv/test_dns.rb
Original file line number Diff line number Diff line change
@@ -3,6 +3,8 @@
require 'resolv'
require 'socket'
require 'tempfile'
require 'timeout'
require 'minitest/mock'

class TestResolvDNS < Test::Unit::TestCase
def setup
@@ -221,4 +223,40 @@ def test_too_big_label_address
}
assert_operator(2**14, :<, m.to_s.length)
end

def test_timeout_without_leaking_file_descriptors_connected
socket = nil
bind_random_port = lambda do |udpsock, bind_host="0.0.0.0"|
socket = udpsock
sleep 3
end

Resolv::DNS.stub(:bind_random_port, bind_random_port) do
r = Resolv::DNS.new(nameserver_port: [['127.0.0.1', 53]])
begin
Timeout.timeout(0.5) { r.getname("8.8.8.8") }
rescue Timeout::Error
end
end

assert(socket.closed?, "file descriptor leaked")
end

def test_timeout_without_leaking_file_descriptors_unconnected
socket = nil
bind_random_port = lambda do |udpsock, bind_host="0.0.0.0"|
socket = udpsock
sleep 3
end

Resolv::DNS.stub(:bind_random_port, bind_random_port) do
r = Resolv::DNS.new
begin
Timeout.timeout(0.5) { r.getname("8.8.8.8") }
rescue Timeout::Error
end
end

assert(socket.closed?, "file descriptor leaked")
end
end