Repository: Azure/WALinuxAgent (python)

Issue:
'NoneType' object has no attribute 'encode'
We have noticed failures with CustomScriptForLinux with the following error message.

```text
'NoneType' object has no attribute 'encode'
```

This may affect other extensions in this repo, but the only report we have is for CustomScript.

Pull request:
ignore contents of None for write_file
Closes #1066

Patch:
diff --git a/azurelinuxagent/common/osutil/default.py b/azurelinuxagent/common/osutil/default.py
index 6acb7886..982d4a50 100644
--- a/azurelinuxagent/common/osutil/default.py
+++ b/azurelinuxagent/common/osutil/default.py
@@ -674,7 +674,6 @@ class DefaultOSUtil(object):
                              socket.IPPROTO_UDP)
         param = struct.pack('256s', (ifname[:15]+('\0'*241)).encode('latin-1'))
         info = fcntl.ioctl(sock.fileno(), IOCTL_SIOCGIFHWADDR, param)
-        sock.close()
         return ''.join(['%02X' % textutil.str_to_ord(char) for char in info[18:24]])
 
     @staticmethod
@@ -699,16 +698,13 @@ class DefaultOSUtil(object):
 
         buff = array.array('B', b'\0' * array_size)
         param = struct.pack('iL', array_size, buff.buffer_info()[0])
-
         sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, socket.IPPROTO_UDP)
         ret = fcntl.ioctl(sock.fileno(), IOCTL_SIOCGIFCONF, param)
         retsize = (struct.unpack('iL', ret)[0])
-        sock.close()
-
         if retsize == array_size:
             logger.warn(('SIOCGIFCONF returned more than {0} up '
                          'network interfaces.'), expected)
-
+        sock = None
         ifconf_buff = buff.tostring()
 
         ifaces = {}
@@ -824,7 +820,6 @@ class DefaultOSUtil(object):
         if not self.disable_route_warning:
             logger.info('interface [{0}] has flags [{1}], '
                         'is loopback [{2}]'.format(ifname, flags, isloopback))
-        s.close()
         return isloopback
 
     def get_dhcp_lease_endpoint(self):
diff --git a/azurelinuxagent/common/osutil/factory.py b/azurelinuxagent/common/osutil/factory.py
index 4d4f501c..5ee611fd 100644
--- a/azurelinuxagent/common/osutil/factory.py
+++ b/azurelinuxagent/common/osutil/factory.py
@@ -69,7 +69,7 @@ def get_osutil(distro_name=DISTRO_NAME,
     if distro_name == "coreos" or distro_code_name == "coreos":
         return CoreOSUtil()
 
-    if distro_name in ("suse", "sles", "opensuse"):
+    if distro_name == "suse":
         if distro_full_name == 'SUSE Linux Enterprise Server' \
                 and Version(distro_version) < Version('12') \
                 or distro_full_name == 'openSUSE' and Version(distro_version) < Version('13.2'):
diff --git a/azurelinuxagent/common/utils/fileutil.py b/azurelinuxagent/common/utils/fileutil.py
index 9da6737f..1f0c7ac4 100644
--- a/azurelinuxagent/common/utils/fileutil.py
+++ b/azurelinuxagent/common/utils/fileutil.py
@@ -193,10 +193,9 @@ def findstr_in_file(file_path, line_str):
     (Trailing whitespace is ignored.)
     """
     try:
-        with open(file_path, 'r') as fh:
-            for line in fh.readlines():
-                if line_str == line.rstrip():
-                    return True
+        for line in (open(file_path, 'r')).readlines():
+            if line_str == line.rstrip():
+                return True
     except Exception:
         # swallow exception
         pass
@@ -208,12 +207,11 @@ def findre_in_file(file_path, line_re):
     Return match object if found in file.
     """
     try:
-        with open(file_path, 'r') as fh:
-            pattern = re.compile(line_re)
-            for line in fh.readlines():
-                match = re.search(pattern, line)
-                if match:
-                    return match
+        pattern = re.compile(line_re)
+        for line in (open(file_path, 'r')).readlines():
+            match = re.search(pattern, line)
+            if match:
+                return match
     except:
         pass
 
diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py
index 6c7357ed..7203ab63 100644
--- a/azurelinuxagent/ga/exthandlers.py
+++ b/azurelinuxagent/ga/exthandlers.py
@@ -29,6 +29,7 @@ import shutil
 import stat
 import subprocess
 import time
+import traceback
 import zipfile
 
 import azurelinuxagent.common.conf as conf
@@ -1073,11 +1074,17 @@ class ExtHandlerInstance(object):
         status_file = os.path.join(state_dir, "HandlerStatus")
 
         try:
-            fileutil.write_file(status_file, json.dumps(get_properties(handler_status)))
+            handler_status_json = json.dumps(get_properties(handler_status))
+            if handler_status_json is not None:
+                fileutil.write_file(status_file, handler_status_json)
+            else:
+                self.logger.error("Failed to create JSON document of handler status for {0} version {1}".format(
+                    self.ext_handler.name,
+                    self.ext_handler.properties.version))
         except (IOError, ValueError, ProtocolError) as e:
             fileutil.clean_ioerror(e,
                 paths=[status_file])
-            self.logger.error("Failed to save handler status: {0}", e)
+            self.logger.error("Failed to save handler status: {0}", traceback.format_exc())
         
     def get_handler_status(self):
         state_dir = self.get_conf_dir()


Test patch:
diff --git a/tests/common/osutil/test_default.py b/tests/common/osutil/test_default.py
index cc58fb85..481536b8 100644
--- a/tests/common/osutil/test_default.py
+++ b/tests/common/osutil/test_default.py
@@ -18,20 +18,17 @@
 import socket
 import glob
 import mock
-import traceback
 
 import azurelinuxagent.common.osutil.default as osutil
 import azurelinuxagent.common.utils.shellutil as shellutil
 from azurelinuxagent.common.exception import OSUtilError
 from azurelinuxagent.common.future import ustr
 from azurelinuxagent.common.osutil import get_osutil
+from azurelinuxagent.common.utils import fileutil
+from azurelinuxagent.common.utils.flexible_version import FlexibleVersion
 from tests.tools import *
 
 
-def fake_is_loopback(_, iface):
-    return iface.startswith('lo')
-
-
 class TestOSUtil(AgentTestCase):
     def test_restart(self):
         # setup
@@ -91,27 +88,7 @@ class TestOSUtil(AgentTestCase):
                         self.assertTrue(msg in ustr(ose))
                         self.assertTrue(patch_run.call_count == 6)
 
-
-    @patch('azurelinuxagent.common.osutil.default.DefaultOSUtil.get_primary_interface', return_value='eth0')
-    @patch('azurelinuxagent.common.osutil.default.DefaultOSUtil._get_all_interfaces', return_value={'eth0':'10.0.0.1'})
-    @patch('azurelinuxagent.common.osutil.default.DefaultOSUtil.is_loopback', fake_is_loopback)
-    def test_get_first_if(self, get_all_interfaces_mock, get_primary_interface_mock):
-        """
-        Validate that the agent can find the first active non-loopback
-        interface.
-
-        This test case used to run live, but not all developers have an eth*
-        interface. It is perfectly valid to have a br*, but this test does not
-        account for that.
-        """
-        ifname, ipaddr = osutil.DefaultOSUtil().get_first_if()
-        self.assertEqual(ifname, 'eth0')
-        self.assertEqual(ipaddr, '10.0.0.1')
-
-    @patch('azurelinuxagent.common.osutil.default.DefaultOSUtil.get_primary_interface', return_value='bogus0')
-    @patch('azurelinuxagent.common.osutil.default.DefaultOSUtil._get_all_interfaces', return_value={'eth0':'10.0.0.1', 'lo': '127.0.0.1'})
-    @patch('azurelinuxagent.common.osutil.default.DefaultOSUtil.is_loopback', fake_is_loopback)
-    def test_get_first_if_nosuchprimary(self, get_all_interfaces_mock, get_primary_interface_mock):
+    def test_get_first_if(self):
         ifname, ipaddr = osutil.DefaultOSUtil().get_first_if()
         self.assertTrue(ifname.startswith('eth'))
         self.assertTrue(ipaddr is not None)
@@ -120,6 +97,18 @@ class TestOSUtil(AgentTestCase):
         except socket.error:
             self.fail("not a valid ip address")
 
+    def test_get_first_if_nosuchprimary(self):
+        fake_ifaces = {'eth0':'192.0.2.100','lo':'127.0.0.1'}
+        with patch.object(osutil.DefaultOSUtil, 'get_primary_interface', return_value='bogus0'):
+            with patch.object(osutil.DefaultOSUtil, '_get_all_interfaces', return_value=fake_ifaces):
+                ifname, ipaddr = osutil.DefaultOSUtil().get_first_if()
+                self.assertTrue(ifname.startswith('eth'))
+                self.assertTrue(ipaddr is not None)
+                try:
+                    socket.inet_aton(ipaddr)
+                except socket.error:
+                    self.fail("not a valid ip address")
+
     def test_get_first_if_all_loopback(self):
         fake_ifaces = {'lo':'127.0.0.1'}
         with patch.object(osutil.DefaultOSUtil, 'get_primary_interface', return_value='bogus0'):
@@ -207,7 +196,6 @@ class TestOSUtil(AgentTestCase):
             try:
                 osutil.DefaultOSUtil().get_first_if()[0]
             except Exception as e:
-                print(traceback.format_exc())
                 exception = True
             self.assertFalse(exception)
 
diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py
index d922a263..4cac2baa 100644
--- a/tests/ga/test_extension.py
+++ b/tests/ga/test_extension.py
@@ -233,6 +233,24 @@ class TestHandlerStateMigration(AgentTestCase):
         self.assertEquals(handler_status.message, message)
         return
 
+    def test_set_handler_status_ignores_none_content(self):
+        """
+        Validate that set_handler_status ignore cases where json.dumps
+        returns a value of None.
+        """
+        self._prepare_handler_state()
+        self._prepare_handler_config()
+
+        status = "Ready"
+        code = 0
+        message = "A message"
+
+        try:
+           with patch('json.dumps', return_value=None):
+                self.ext_handler_i.set_handler_status(status=status, code=code, message=message)
+        except Exception as e:
+            self.fail("set_handler_status threw an exception")
+
     @patch("shutil.move", side_effect=Exception)
     def test_migration_ignores_move_errors(self, shutil_mock):
         self._prepare_handler_state()
diff --git a/tests/utils/test_file_util.py b/tests/utils/test_file_util.py
index 87bce8c0..f1e6c680 100644
--- a/tests/utils/test_file_util.py
+++ b/tests/utils/test_file_util.py
@@ -38,7 +38,19 @@ class TestFileOperations(AgentTestCase):
         content_read = fileutil.read_file(test_file)
         self.assertEquals(content, content_read)
         os.remove(test_file)
-    
+
+    def test_write_file_content_is_None(self):
+        """
+        write_file throws when content is None. No file is created.
+        """
+        try:
+            test_file=os.path.join(self.tmp_dir, self.test_file)
+            fileutil.write_file(test_file, None)
+
+            self.fail("expected write_file to throw an exception")
+        except:
+            self.assertEquals(False, os.path.exists(test_file))
+
     def test_rw_utf8_file(self):
         test_file=os.path.join(self.tmp_dir, self.test_file)
         content = u"\u6211"