[GR-16115] Fix TCPServer#accept to set #do_not_reverse_lookup correctly on the created TCPSocket.

PullRequest: truffleruby/867

Author: eregon

CHANGELOG.md

@@ -15,6 +15,7 @@ Bug fixes:
 * Procs will now yield to the block in their declaration context even when called with a block argument (#1657).
 * Fixed problems with calling POSIX methods if `Symbol#[]` is redefined (#1665).
 * Fixed sharing of `Array` and `Hash` elements for thread-safety of objects (#1601).
+* Fix `TCPServer#accept` to set `#do_not_reverse_lookup` correctly on the created `TCPSocket`.
 
 Compatibility
 

lib/truffle/socket/basic_socket.rb

@@ -25,10 +25,10 @@
 # EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 class BasicSocket < IO
-  def self.for_fd(fixnum)
+  def self.for_fd(fd)
     sock = allocate
 
-    IO.setup(sock, fixnum, nil, true)
+    IO.setup(sock, fd, nil, true)
     sock.binmode
     # TruffleRuby: start
     sock.do_not_reverse_lookup = do_not_reverse_lookup

lib/truffle/socket/truffle.rb

@@ -81,10 +81,7 @@ def self.accept(source, new_class)
 
         Error.read_error('accept(2)', source) if fd < 0
 
-        socket = new_class.allocate
-
-        ::IO.setup(socket, fd, nil, true)
-        socket.binmode
+        socket = new_class.for_fd(fd)
 
         socktype = source.getsockopt(:SOCKET, :TYPE).int
         addrinfo = Addrinfo.new(sockaddr.to_s, sockaddr.family, socktype)

lib/truffle/socket/truffle/foreign.rb

@@ -208,7 +208,7 @@ def self.getnameinfo(sockaddr, flags = ::Socket::NI_NUMERICHOST | ::Socket::NI_N
 
               raise SocketError, gai_strerror(err) unless err == 0
 
-              sa_family = SockaddrIn.with_sockaddr(sockaddr)[:sin_family]
+              sa_family = SockaddrIn.new(sockaddr_p)[:sin_family]
 
               name_info[0] = ::Socket::Constants::AF_TO_FAMILY[sa_family]
               name_info[1] = service.read_string

spec/ruby/CONTRIBUTING.md

@@ -188,7 +188,7 @@ variables from the implementor spec: `@method` and `@object`, which the implemen
 
 Here's an example of a snippet of a shared spec and two specs which integrates it:
 
-``` ruby
+```ruby
 # core/hash/shared/key.rb
 describe :hash_key_p, shared: true do
   it "returns true if the key's matching value was false" do
@@ -216,7 +216,7 @@ Sometimes, shared specs require more context from the implementor class than a s
 this by passing a lambda as the method, which will have the scope of the implementor.  Here's an example of
 how this is used currently:
 
-``` ruby
+```ruby
 describe :kernel_sprintf, shared: true do
   it "raises TypeError exception if cannot convert to Integer" do
     -> { @method.call("%b", Object.new) }.should raise_error(TypeError)

spec/ruby/library/socket/basicsocket/do_not_reverse_lookup_spec.rb

@@ -37,3 +37,67 @@
     @socket.peeraddr[2].should == "127.0.0.1"
   end
 end
+
+describe :socket_do_not_reverse_lookup, shared: true do
+  it "inherits from BasicSocket.do_not_reverse_lookup when the socket is created" do
+    @socket = @method.call
+    reverse = BasicSocket.do_not_reverse_lookup
+    @socket.do_not_reverse_lookup.should == reverse
+
+    BasicSocket.do_not_reverse_lookup = !reverse
+    @socket.do_not_reverse_lookup.should == reverse
+  end
+
+  it "is true when BasicSocket.do_not_reverse_lookup is true" do
+    BasicSocket.do_not_reverse_lookup = true
+    @socket = @method.call
+    @socket.do_not_reverse_lookup.should == true
+  end
+
+  it "is false when BasicSocket.do_not_reverse_lookup is false" do
+    BasicSocket.do_not_reverse_lookup = false
+    @socket = @method.call
+    @socket.do_not_reverse_lookup.should == false
+  end
+
+  it "can be changed with #do_not_reverse_lookup=" do
+    @socket = @method.call
+    reverse = @socket.do_not_reverse_lookup
+    @socket.do_not_reverse_lookup = !reverse
+    @socket.do_not_reverse_lookup.should == !reverse
+  end
+end
+
+describe "BasicSocket#do_not_reverse_lookup" do
+  before :each do
+    @do_not_reverse_lookup = BasicSocket.do_not_reverse_lookup
+    @server = TCPServer.new('127.0.0.1', 0)
+    @port = @server.addr[1]
+  end
+
+  after :each do
+    @server.close unless @server.closed?
+    @socket.close if @socket && !@socket.closed?
+    BasicSocket.do_not_reverse_lookup = @do_not_reverse_lookup
+  end
+
+  describe "for an TCPSocket.new socket" do
+    it_behaves_like :socket_do_not_reverse_lookup, -> {
+      TCPSocket.new('127.0.0.1', @port)
+    }
+  end
+
+  describe "for an TCPServer#accept socket" do
+    before :each do
+      @client = TCPSocket.new('127.0.0.1', @port)
+    end
+
+    after :each do
+      @client.close if @client && !@client.closed?
+    end
+
+    it_behaves_like :socket_do_not_reverse_lookup, -> {
+      @server.accept
+    }
+  end
+end

spec/tags/library/socket/basicsocket/do_not_reverse_lookup_tags.txt

@@ -0,0 +1,2 @@
+fails:BasicSocket#do_not_reverse_lookup for an TCPSocket.new socket is false when BasicSocket.do_not_reverse_lookup is false
+fails:BasicSocket#do_not_reverse_lookup for an TCPServer#accept socket is false when BasicSocket.do_not_reverse_lookup is false