ExtensionUtils fix encoding non-ascii characters (#1329)

* added code that fixes decoding non-ascii characters

* added a comment

* added unit tests

* fixed unit test

* removed redundant code

* renamed test

* renamed test class

* Updated version

* updated version again as 1.15.10 was already released in test

Utils/extensionutils.py

@@ -66,15 +66,25 @@ def create_dir(dir_path, user, mode):
     change_owner(dir_path, user)
 
 
+def encode_for_writing_to_file(contents):
+    if type(contents) == str:
+        if sys.version_info[0] == 3:
+            """
+            utf-8 is a superset of ASCII and latin-1
+            in python 2 str is an alias for bytes, no need to encode it again
+            """
+            return contents.encode('utf-8')
+    return contents
+
+
 def set_file_contents(file_path, contents):
     """
     Write 'contents' to 'file_path'.
     """
-    if type(contents) == str:
-        contents = contents.encode('latin-1', 'ignore')
+    bytes_to_write = encode_for_writing_to_file(contents)
     try:
         with open(file_path, "wb+") as F:
-            F.write(contents)
+            F.write(bytes_to_write)
     except EnvironmentError as e:
         logger.error_with_prefix(
             'SetFileContents', 'Writing to file ' + file_path + ' Exception is ' + str(e))
@@ -86,14 +96,10 @@ def append_file_contents(file_path, contents):
     """
     Append 'contents' to 'file_path'.
     """
-    if type(contents) == str:
-        if sys.version_info[0] == 3:
-            contents = contents.encode('latin-1').decode('latin-1')
-        elif sys.version_info[0] == 2:
-            contents = contents.encode('latin-1')
+    bytes_to_write = encode_for_writing_to_file(contents)
     try:
-        with open(file_path, "a+") as F:
-            F.write(contents)
+        with open(file_path, "ab+") as F:
+            F.write(bytes_to_write)
     except EnvironmentError as e:
         logger.error_with_prefix(
             'AppendFileContents', 'Appending to file ' + file_path + ' Exception is ' + str(e))
@@ -123,10 +129,9 @@ def replace_file_with_contents_atomic(filepath, contents):
     Write 'contents' to 'filepath' by creating a temp file, and replacing original.
     """
     handle, temp = tempfile.mkstemp(dir=os.path.dirname(filepath))
-    if type(contents) == str:
-        contents = contents.encode('latin-1')
+    bytes_to_write = encode_for_writing_to_file(contents)
     try:
-        os.write(handle, contents)
+        os.write(handle, bytes_to_write)
     except EnvironmentError as e:
         logger.error_with_prefix(
             'ReplaceFileContentsAtomic', 'Writing to file ' + filepath + ' Exception is ' + str(e))

Utils/test/mock_sshd_config

@@ -0,0 +1,93 @@
+# Package generated configuration file
+# See the sshd_config(5) manpage for details
+
+# What ports, IPs and protocols we listen for
+Port 22
+# Use these options to restrict which interfaces/protocols sshd will bind to
+#ListenAddress ::
+#ListenAddress 0.0.0.0
+Protocol 2
+# HostKeys for protocol version 2
+HostKey /etc/ssh/ssh_host_rsa_key
+HostKey /etc/ssh/ssh_host_dsa_key
+HostKey /etc/ssh/ssh_host_ecdsa_key
+HostKey /etc/ssh/ssh_host_ed25519_key
+#Privilege Separation is turned on for security
+UsePrivilegeSeparation yes
+
+# Lifetime and size of ephemeral version 1 server key
+KeyRegenerationInterval 3600
+ServerKeyBits 1024
+
+# Logging
+SyslogFacility AUTH
+LogLevel INFO
+
+# Authentication:
+LoginGraceTime 10m
+PermitRootLogin without-password
+StrictModes yes
+
+RSAAuthentication yes
+PubkeyAuthentication yes
+#AuthorizedKeysFile %h/.ssh/authorized_keys
+
+# Don’t read the user’s ~/.rhosts and ~/.shosts files
+IgnoreRhosts yes
+# For this to work you will also need host keys in /etc/ssh_known_hosts
+RhostsRSAAuthentication no
+# similar for protocol version 2
+HostbasedAuthentication no
+# Uncomment if you don’t trust ~/.ssh/known_hosts for RhostsRSAAuthentication
+#IgnoreUserKnownHosts yes
+
+# To enable empty passwords, change to yes (NOT RECOMMENDED)
+PermitEmptyPasswords no
+
+# Change to yes to enable challenge-response passwords (beware issues with
+# some PAM modules and threads)
+ChallengeResponseAuthentication yes
+
+# Change to no to disable tunnelled clear text passwords
+PasswordAuthentication no
+
+# Kerberos options
+#KerberosAuthentication no
+#KerberosGetAFSToken no
+#KerberosOrLocalPasswd yes
+#KerberosTicketCleanup yes
+
+# GSSAPI options
+#GSSAPIAuthentication no
+#GSSAPICleanupCredentials yes
+
+X11Forwarding yes
+X11DisplayOffset 10
+PrintMotd no
+PrintLastLog yes
+TCPKeepAlive yes
+#UseLogin no
+
+#MaxStartups 10:30:60
+#Banner /etc/issue.net
+
+# Allow client to pass locale environment variables
+AcceptEnv LANG LC_*
+
+Subsystem sftp /usr/lib/openssh/sftp-server
+
+# Set this to ‘yes’ to enable PAM authentication, account processing,
+# and session processing. If this is enabled, PAM authentication will
+# be allowed through the ChallengeResponseAuthentication and
+# PasswordAuthentication. Depending on your PAM configuration,
+# PAM authentication via ChallengeResponseAuthentication may bypass
+# the setting of “PermitRootLogin without-password”.
+# If you just want the PAM account and session checks to run without
+# PAM authentication, then enable this but set PasswordAuthentication
+# and ChallengeResponseAuthentication to ‘no’.
+UsePAM yes
+
+# CLOUD_IMG: This file was created/modified by the Cloud Image build process
+ClientAliveInterval 120
+AuthorizedKeysCommand /usr/sbin/aad_certhandler %u %k
+AuthorizedKeysCommandUser root

Utils/test/test_encode.py

@@ -0,0 +1,31 @@
+#!/usr/bin/env python
+#
+# Copyright 2014 Microsoft Corporation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+import Utils.extensionutils as eu
+import unittest
+
+
+class TestEncode(unittest.TestCase):
+    def test_encode(self):
+        contents = eu.get_file_contents('mock_sshd_config')
+        encoded_contents = eu.encode_for_writing_to_file(contents)
+        known_non_ascii_character = b"%c" % encoded_contents[2353]
+        self.assertEqual(known_non_ascii_character, b'\x9d')
+
+
+if __name__ == '__main__':
+    unittest.main()

VMAccess/manifest.xml

@@ -2,7 +2,7 @@
 <ExtensionImage xmlns="http://schemas.microsoft.com/windowsazure">
   <ProviderNameSpace>Microsoft.OSTCExtensions</ProviderNameSpace>
   <Type>VMAccessForLinux</Type>
-  <Version>1.5.10</Version>
+  <Version>1.5.11</Version>
   <Label>Microsoft Azure VM Access Extension for Linux Virtual Machines</Label>
   <HostingResources>VmRole</HostingResources>
   <MediaLink></MediaLink>