Repository: saltstack/salt
Pull request: Fix UnboundLocalError when address cannot be resolved in win_network.connect
### What does this PR do?
`win_network.connect` could in certain cases end with `UnboundLocalError` because the variable used in except wasn't initialised yet, this fixes it

### What issues does this PR fix or reference?
Fixes: #53371

### Previous Behavior
```
The minion function caused an exception: Traceback (most recent call last):
File "c:\salt\bin\lib\site-packages\salt-3000.3-py2.7.egg\salt\minion.py", line 1676, in _thread_return
return_data = minion_instance.executors[fname](opts, data, func, args, kwargs)
File "c:\salt\bin\lib\site-packages\salt-3000.3-py2.7.egg\salt\executors\direct_call.py", line 12, in execute
return func(*args, **kwargs)
File "c:\salt\bin\lib\site-packages\salt-3000.3-py2.7.egg\salt\modules\win_network.py", line 512, in connect
.format(host, _address[0], proto, port)
UnboundLocalError: local variable '_address' referenced before assignment
```

### New Behavior
```
{
  "result": False,
  "comment": "Unable to connect to test-server (unknown) on tcp port 80"
}
```

### Merge requirements satisfied?
**[NOTICE] Bug fixes or features added to Salt require tests.**
<!-- Please review the [test documentation](https://docs.saltstack.com/en/master/topics/tutorials/writing_tests.html) for details on how to implement tests into Salt's test suite. -->
- [ ] Docs
- [x] Changelog - https://docs.saltstack.com/en/master/topics/development/changelog.html
- [x] Tests written/updated

### Commits signed with GPG?
Yes


Problem statement:
win_network network.connect gives error on failure
### Description of Issue
When running `salt 'Testmachine' network.connect 'badmachine' port=445`   where badmachine is unreachable produces the following error:

```
The minion function caused an exception: Traceback (most recent call last):
      File "c:\salt\bin\lib\site-packages\salt\modules\win_network.py", line 495, in connect
        _address) = socket.getaddrinfo(address, port, __family, 0, __proto)[0]
      File "c:\salt\bin\lib\socket.py", line 733, in getaddrinfo
        for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
    socket.gaierror: [Errno 11004] getaddrinfo failed

    During handling of the above exception, another exception occurred:

    Traceback (most recent call last):
      File "c:\salt\bin\lib\site-packages\salt\minion.py", line 1660, in _thread_return
        return_data = minion_instance.executors[fname](opts, data, func, args, kwargs)
      File "c:\salt\bin\lib\site-packages\salt\executors\direct_call.py", line 12, in execute
        return func(*args, **kwargs)
      File "c:\salt\bin\lib\site-packages\salt\modules\win_network.py", line 515, in connect
        .format(host, _address[0], proto, port)
    UnboundLocalError: local variable '_address' referenced before assignment
```

I would expect to see `False` instead.
Successful Attempts produce the expected result.

### Versions Report
Master:
```
Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: 1.6.0
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: 0.26.3
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.26.4
         Python: 2.7.5 (default, Apr  9 2019, 14:30:50)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.6.1810 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-957.12.2.el7.x86_64
         system: Linux
        version: CentOS Linux 7.6.1810 Core
```

Hints from issue discussion:
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

Thank you for updating this issue. It is no longer marked as stale.

Commit message: Merge branch 'master' into fix-win-network-connect-err

Patch to review:
diff --git a/changelog/53371.fixed b/changelog/53371.fixed
new file mode 100644
index 000000000000..fd515bd2353d
--- /dev/null
+++ b/changelog/53371.fixed
@@ -0,0 +1 @@
+Fixed UnboundLocalError when using win_network.connect
diff --git a/salt/modules/win_network.py b/salt/modules/win_network.py
index 7bfaeb80ae37..b82bf5169bbb 100644
--- a/salt/modules/win_network.py
+++ b/salt/modules/win_network.py
@@ -1,8 +1,6 @@
-# -*- coding: utf-8 -*-
 """
 Module for gathering and managing network information
 """
-from __future__ import absolute_import, print_function, unicode_literals
 
 import datetime
 import hashlib
@@ -112,7 +110,7 @@ def ping(host, timeout=False, return_boolean=False):
             "-n",
             "4",
             "-w",
-            six.text_type(timeout),
+            str(timeout),
             salt.utils.network.sanitize_host(host),
         ]
     else:
@@ -265,7 +263,7 @@ def get_route(ip):
 
         salt '*' network.get_route 10.10.10.10
     """
-    cmd = "Find-NetRoute -RemoteIPAddress {0}".format(ip)
+    cmd = "Find-NetRoute -RemoteIPAddress {}".format(ip)
     out = __salt__["cmd.run"](cmd, shell="powershell", python_shell=True)
     regexp = re.compile(
         r"^IPAddress\s+:\s(?P<source>[\d\.:]+)?.*"
@@ -498,7 +496,10 @@ def connect(host, port=None, **kwargs):
     ):
         address = host
     else:
-        address = "{0}".format(salt.utils.network.sanitize_host(host))
+        address = "{}".format(salt.utils.network.sanitize_host(host))
+
+    # just in case we encounter error on getaddrinfo
+    _address = ("unknown",)
 
     try:
         if proto == "udp":
@@ -538,13 +539,13 @@ def connect(host, port=None, **kwargs):
             skt.shutdown(2)
     except Exception as exc:  # pylint: disable=broad-except
         ret["result"] = False
-        ret["comment"] = "Unable to connect to {0} ({1}) on {2} port {3}".format(
+        ret["comment"] = "Unable to connect to {} ({}) on {} port {}".format(
             host, _address[0], proto, port
         )
         return ret
 
     ret["result"] = True
-    ret["comment"] = "Successfully connected to {0} ({1}) on {2} port {3}".format(
+    ret["comment"] = "Successfully connected to {} ({}) on {} port {}".format(
         host, _address[0], proto, port
     )
     return ret
diff --git a/tests/unit/modules/test_win_network.py b/tests/unit/modules/test_win_network.py
index 7d3071480a70..91370f4f1f85 100644
--- a/tests/unit/modules/test_win_network.py
+++ b/tests/unit/modules/test_win_network.py
@@ -1,10 +1,9 @@
-# -*- coding: utf-8 -*-
 """
     :codeauthor: Jayesh Kariya <jayeshk@saltstack.com>
 """
 
 # Import Python Libs
-from __future__ import absolute_import, print_function, unicode_literals
+import socket
 
 import salt.modules.win_network as win_network
 
@@ -24,7 +23,7 @@
     HAS_WMI = False
 
 
-class Mockwmi(object):
+class Mockwmi:
     """
     Mock wmi class
     """
@@ -35,7 +34,7 @@ def __init__(self):
         pass
 
 
-class Mockwinapi(object):
+class Mockwinapi:
     """
     Mock winapi class
     """
@@ -43,7 +42,7 @@ class Mockwinapi(object):
     def __init__(self):
         pass
 
-    class winapi(object):
+    class winapi:
         """
         Mock winapi class
         """
@@ -51,7 +50,7 @@ class winapi(object):
         def __init__(self):
             pass
 
-        class Com(object):
+        class Com:
             """
             Mock Com method
             """
@@ -312,3 +311,20 @@ def test_get_route(self):
                     "source": "10.0.0.15",
                 },
             )
+
+    def test_connect_53371(self):
+        """
+        Test that UnboundLocalError is not thrown on socket.gaierror
+        as reported in #53371
+        """
+        with patch(
+            "socket.getaddrinfo",
+            MagicMock(side_effect=socket.gaierror("[Errno 11004] getaddrinfo failed")),
+        ):
+            rtn = win_network.connect("test-server", 80)
+            assert rtn
+            assert not rtn["result"]
+            assert (
+                rtn["comment"]
+                == "Unable to connect to test-server (unknown) on tcp port 80"
+            )


Code review comments:
diff_hunk: @@ -312,3 +311,20 @@ def test_get_route(self):
                     "source": "10.0.0.15",
                 },
             )
+
+    def test_connect_53371(self):
+        """
+        Test that UnboundLocalError is not thrown on socket.gaierror
+        as reported in #53371
+        """
+        with patch(
+            "socket.getaddrinfo",
+            MagicMock(side_effect=socket.gaierror("[Errno 11004] getaddrinfo failed")),
path: tests/unit/modules/test_win_network.py
text: @user1:
**suggestion**: pretty sure we want autospec here. Also can just provide `side_effect` as an argument to patch rather than creating a MagicMock.
```suggestion
            autospec=True,
            side_effect=socket.gaierror("[Errno 11004] getaddrinfo failed"),
```

Otherwise, 👍 

@author:
Thanks, I've applied your suggestion and checked if pre-commit is fine with that. I've also updated the branch to latest master, so test could run with all current content.