Merge pull request #11810 from chrisroberts/f-host-port-valid

Log warning on unknown host_ip for port forwards

lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb

@@ -269,12 +269,11 @@ module Vagrant
             end
           else
             # Do a regular check
-            if test_host_ip == "0.0.0.0" || Addrinfo.ip(test_host_ip).ipv4_loopback? ||
-                Vagrant::Util::IPv4Interfaces.ipv4_interfaces.detect { |iface| iface[1] == test_host_ip }
-              Vagrant::Util::IsPortOpen.is_port_open?(test_host_ip, host_port)
-            else
-              raise Errors::ForwardPortHostIPNotFound, name: machine.name, host_ip: host_ip
+            if test_host_ip != "0.0.0.0" && !Addrinfo.ip(test_host_ip).ipv4_loopback? &&
+                Vagrant::Util::IPv4Interfaces.ipv4_interfaces.none? { |iface| iface[1] == test_host_ip }
+              @logger.warn("host IP address is not local to this device host_ip=#{test_host_ip}")
             end
+            Vagrant::Util::IsPortOpen.is_port_open?(test_host_ip, host_port)
           end
         end
 

lib/vagrant/util/is_port_open.rb

@@ -1,5 +1,4 @@
 require "socket"
-require "timeout"
 
 module Vagrant
   module Util
@@ -14,25 +13,13 @@ module Vagrant
       # @return [Boolean] `true` if the port is open (listening), `false`
       #   otherwise.
       def is_port_open?(host, port)
-        # We wrap this in a timeout because once in awhile the TCPSocket
-        # _will_ hang, but this signals that the port is closed.
-        Timeout.timeout(1) do
-          # Attempt to make a connection
-          s = TCPSocket.new(host, port)
-
-          # A connection was made! Properly clean up the socket, not caring
-          # at all if any exception is raised, because we already know the
-          # result.
-          s.close rescue nil
-
-          # The port is open if we reached this point, since we were able
-          # to connect.
-          return true
+        begin
+          Socket.tcp(host, port, connect_timeout: 0.1).close
+          true
+        rescue Errno::ETIMEDOUT, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, \
+            Errno::ENETUNREACH, Errno::EACCES, Errno::ENOTCONN
+          false
         end
-      rescue Timeout::Error, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, \
-             Errno::ENETUNREACH, Errno::EACCES, Errno::ENOTCONN
-        # Any of the above exceptions signal that the port is closed.
-        return false
       end
 
       extend IsPortOpen

test/unit/vagrant/action/builtin/handle_forwarded_port_collisions_test.rb

@@ -198,10 +198,10 @@ describe Vagrant::Action::Builtin::HandleForwardedPortCollisions do
       let(:host_ip) { "192.168.99.100" }
       let(:name) { "default" }
 
-      it "should raise an error including the machine name" do
+      it "should not raise an error" do
         allow(machine).to receive(:name).and_return(name)
         expect{ instance.send(:port_check, host_ip, host_port) }.
-          to raise_error(Vagrant::Errors::ForwardPortHostIPNotFound, /#{name}/)
+          not_to raise_error
       end
     end
 

test/unit/vagrant/util/is_port_open_test.rb

@@ -47,13 +47,12 @@ describe Vagrant::Util::IsPortOpen do
   end
 
   it "should handle connection refused" do
-    expect(TCPSocket).to receive(:new).with("0.0.0.0", closed_port).and_raise(Errno::ECONNREFUSED)
+    expect(Socket).to receive(:tcp).with("0.0.0.0", closed_port, any_args).and_raise(Errno::ECONNREFUSED)
     expect(subject.is_port_open?("0.0.0.0", closed_port)).to be(false)
   end
 
   it "should raise an error if cannot assign requested address" do
-    expect(TCPSocket).to receive(:new).with("0.0.0.0", open_port).and_raise(Errno::EADDRNOTAVAIL)
+    expect(Socket).to receive(:tcp).with("0.0.0.0", open_port, any_args).and_raise(Errno::EADDRNOTAVAIL)
     expect { subject.is_port_open?("0.0.0.0", open_port) }.to raise_error(Errno::EADDRNOTAVAIL)
   end
 end
-