utils.astring: Make the "string_to_safe_path" Windows friendly

Windows does not allow `<>:"/\|?*` characters in the filename. Let's
include these in our `astring.string_to_safe_path` method and use it
everywhere we create files.
Signed-off-by: NLukáš Doktor <ldoktor@redhat.com>

avocado/utils/astring.py

@@ -26,8 +26,14 @@ And not notice until their code starts failing.
 """
 
 import itertools
-import os.path
 import re
+import string
+
+
+#: String containing all fs-unfriendly chars (Windows-fat/Linux-ext3)
+FS_UNSAFE_CHARS = '<>:"/\\|?*'
+#: Translate table to replace fs-unfriendly chars
+FS_TRANSLATE = string.maketrans(FS_UNSAFE_CHARS, "_" * len(FS_UNSAFE_CHARS))
 
 
 def bitlist_to_string(data):
@@ -234,5 +240,12 @@ def string_to_safe_path(input_str):
     :return: String which is safe to pass as a file/dir name (on recent fs)
     """
     if input_str.startswith("."):
-        input_str = "_" + input_str[1:]
-    return input_str.replace(os.path.sep, '_')[:255]
+        input_str = "_" + input_str[1:255]
+    elif len(input_str) > 255:
+        input_str = input_str[:255]
+    try:
+        return string.translate(input_str, FS_TRANSLATE)
+    except UnicodeDecodeError:
+        for bad_chr in FS_UNSAFE_CHARS:
+            input_str = input_str.replace(bad_chr, "_")
+    return input_str

selftests/functional/test_basic.py

@@ -134,6 +134,14 @@ READ_BINARY = probe_binary('read')
 SLEEP_BINARY = probe_binary('sleep')
 
 
+def html_capable():
+    try:
+        pkg_resources.require('avocado-framework-plugin-result-html')
+        return True
+    except pkg_resources.DistributionNotFound:
+        return False
+
+
 class RunnerOperationTest(unittest.TestCase):
 
     def setUp(self):
@@ -631,7 +639,7 @@ class RunnerHumanOutputTest(unittest.TestCase):
                          " test-results dir, but only one test was executed: "
                          "%s" % (test_dirs))
         self.assertEqual(os.path.basename(test_dirs[0]),
-                         '1-foo\\\\n\\\'\\"\\\\nbar_baz')
+                         "1-foo__n_'____nbar_baz")
 
     def test_replay_skip_skipped(self):
         cmd = ("%s run --job-results-dir %s --json - "
@@ -747,6 +755,36 @@ class RunnerSimpleTest(unittest.TestCase):
         self.assertIn('ERROR| Error message (ordinary message not changing '
                       'the results)', result.stdout, result)
 
+    @unittest.skipIf(not GNU_ECHO_BINARY, "Uses echo as test")
+    def test_fs_unfriendly_run(self):
+        os.chdir(basedir)
+        commands_path = os.path.join(self.tmpdir, "commands")
+        script.make_script(commands_path, "echo '\"\\/|?*<>'")
+        config_path = os.path.join(self.tmpdir, "config.conf")
+        script.make_script(config_path,
+                           "[sysinfo.collectibles]\ncommands = %s"
+                           % commands_path)
+        cmd_line = ("%s --show all --config %s run --job-results-dir %s "
+                    "--sysinfo=on --external-runner %s -- \"'\\\"\\/|?*<>'\""
+                    % (AVOCADO, config_path, self.tmpdir, GNU_ECHO_BINARY))
+        result = process.run(cmd_line)
+        self.assertTrue(os.path.exists(os.path.join(self.tmpdir, "latest",
+                                                    "test-results",
+                                                    "1-\'________\'/")))
+        self.assertTrue(os.path.exists(os.path.join(self.tmpdir, "latest",
+                                                    "sysinfo", "pre",
+                                                    "echo \'________\'")))
+
+        if html_capable():
+            with open(os.path.join(self.tmpdir, "latest",
+                                   "results.html")) as html_res:
+                html_results = html_res.read()
+            # test results should replace odd chars with "_"
+            self.assertIn(os.path.join("test-results", "1-'________'"),
+                          html_results)
+            # sysinfo replaces "_" with " "
+            self.assertIn("echo '________'", html_results)
+
     def test_non_absolute_path(self):
         avocado_path = os.path.join(basedir, 'scripts', 'avocado')
         test_base_dir = os.path.dirname(self.pass_script.path)
@@ -1034,12 +1072,9 @@ class PluginsTest(AbsPluginsTest, unittest.TestCase):
 
         result_plugins = ["json", "xunit", "zip_archive"]
         result_outputs = ["results.json", "results.xml"]
-        try:
-            pkg_resources.require('avocado-framework-plugin-result-html')
+        if html_capable():
             result_plugins.append("html")
             result_outputs.append("results.html")
-        except pkg_resources.DistributionNotFound:
-            pass
 
         cmd_line = '%s plugins' % AVOCADO
         result = process.run(cmd_line, ignore_status=True)
@@ -1244,7 +1279,7 @@ class PluginsJSONTest(AbsPluginsTest, unittest.TestCase):
                          '1--ne foo\\\\n\\\'\\"\\\\nbar/baz')
         # logdir name should escape special chars (/)
         self.assertEqual(os.path.basename(data['tests'][0]['logdir']),
-                         '1--ne foo\\\\n\\\'\\"\\\\nbar_baz')
+                         "1--ne foo__n_'____nbar_baz")
 
     def tearDown(self):
         shutil.rmtree(self.tmpdir)

selftests/unit/test_astring.py

@@ -62,6 +62,12 @@ class AstringTest(unittest.TestCase):
                       "a\u0430 123")
         self.assertEqual(astring.tabular_output(matrix), str_matrix)
 
+    def test_safe_path(self):
+        self.assertEqual(astring.string_to_safe_path('a<>:"/\\|\?*b'),
+                         "a__________b")
+        self.assertEqual(astring.string_to_safe_path('..'), "_.")
+        self.assertEqual(len(astring.string_to_safe_path(" " * 300)), 255)
+
 
 if __name__ == '__main__':
     unittest.main()

selftests/unit/test_test.py

@@ -6,7 +6,7 @@ import unittest
 from flexmock import flexmock, flexmock_teardown
 
 from avocado.core import test, exceptions
-from avocado.utils import script
+from avocado.utils import astring, script
 
 PASS_SCRIPT_CONTENTS = """#!/bin/sh
 true
@@ -253,7 +253,8 @@ class TestID(unittest.TestCase):
         test_id = test.TestID(uid, name)
         self.assertEqual(test_id.uid, 1)
         self.assertEqual(test_id.str_uid, '1')
-        self.assertEqual(test_id.str_filesystem, '%s-%s' % (uid, name))
+        self.assertEqual(test_id.str_filesystem,
+                         astring.string_to_safe_path('%s-%s' % (uid, name)))
         self.assertIs(test_id.variant, None)
         self.assertIs(test_id.str_variant, '')
 
@@ -263,7 +264,8 @@ class TestID(unittest.TestCase):
         test_id = test.TestID(uid, name, no_digits=2)
         self.assertEqual(test_id.uid, 1)
         self.assertEqual(test_id.str_uid, '01')
-        self.assertEqual(test_id.str_filesystem, '%s-%s' % ('01', name))
+        self.assertEqual(test_id.str_filesystem,
+                         astring.string_to_safe_path('%s-%s' % ('01', name)))
         self.assertIs(test_id.variant, None)
         self.assertIs(test_id.str_variant, '')