Add logic to sanitize windows filenames (#629)
diff --git a/mobly/controllers/android_device.py b/mobly/controllers/android_device.py
index c3197ca..54bf098 100644
--- a/mobly/controllers/android_device.py
+++ b/mobly/controllers/android_device.py
@@ -411,7 +411,7 @@
if begin_time is None:
begin_time = mobly_logger.get_log_file_timestamp()
else:
- begin_time = mobly_logger.normalize_log_line_timestamp(str(begin_time))
+ begin_time = mobly_logger.sanitize_filename(str(begin_time))
def take_br(test_name, begin_time, ad, destination):
ad.take_bug_report(test_name=test_name,
@@ -484,14 +484,12 @@
def _normalized_serial(self):
"""Normalized serial name for usage in log filename.
- Some Android emulators use ip:port as their serial names, while on
+ Some Android emulators use ip:port as their serial names, while on
Windows `:` is not valid in filename, it should be sanitized first.
"""
if self._serial is None:
return None
- normalized_serial = self._serial.replace(' ', '_')
- normalized_serial = normalized_serial.replace(':', '-')
- return normalized_serial
+ return mobly_logger.sanitize_filename(self._serial)
@property
def device_info(self):
diff --git a/mobly/logger.py b/mobly/logger.py
index a68398e..a39d185 100644
--- a/mobly/logger.py
+++ b/mobly/logger.py
@@ -23,6 +23,40 @@
from mobly import records
from mobly import utils
+# Filename sanitization mappings for Windows.
+# See https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions
+WINDOWS_MAX_FILENAME_LENGTH = 260
+WINDOWS_RESERVED_CHARACTERS_REPLACEMENTS = {
+ '<':
+ '-',
+ '>':
+ '-',
+ ':':
+ '-',
+ '"':
+ '_',
+ '/':
+ '_',
+ '\\':
+ '_',
+ '|':
+ ',',
+ '?':
+ ',',
+ '*':
+ ',',
+ # Integer zero (i.e. NUL) is not a valid character.
+ # While integers 1-31 are also usually valid, they aren't sanitized because
+ # they are situationally valid.
+ chr(0):
+ '0',
+}
+# Note, although the documentation does not specify as such, COM0 and LPT0 are
+# also invalid/reserved filenames.
+WINDOWS_RESERVED_FILENAME_REGEX = re.compile(
+ r'^(CON|PRN|AUX|NUL|(COM|LPT)[0-9])(\.[^.]*)?$', re.IGNORECASE)
+WINDOWS_RESERVED_FILENAME_PREFIX = 'mobly_'
+
log_line_format = '%(asctime)s.%(msecs).03d %(levelname)s %(message)s'
# The micro seconds are added by the format string above,
# so the time format does not include ms.
@@ -219,9 +253,73 @@
create_latest_log_alias(log_path, alias=alias)
+def _sanitize_windows_filename(filename):
+ """Sanitizes a filename for Windows.
+
+ Refer to the following Windows documentation page for the rules:
+ https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions
+
+ If the filename matches one of Window's reserved file namespaces, then the
+ `WINDOWS_RESERVED_FILENAME_PREFIX` (i.e. "mobly_") prefix will be appended
+ to the filename to convert it into a valid Windows filename.
+
+ Args:
+ filename: string, the filename to sanitize for the Windows file system.
+
+ Returns:
+ A filename that should be safe to use on Windows.
+ """
+ if re.match(WINDOWS_RESERVED_FILENAME_REGEX, filename):
+ return WINDOWS_RESERVED_FILENAME_PREFIX + filename
+
+ if len(filename) > WINDOWS_MAX_FILENAME_LENGTH:
+ filename = filename[:WINDOWS_MAX_FILENAME_LENGTH]
+
+ # In order to meet max length, none of these replacements should increase
+ # the length of the filename.
+ new_filename_chars = []
+ for char in filename:
+ if char in WINDOWS_RESERVED_CHARACTERS_REPLACEMENTS:
+ new_filename_chars.append(
+ WINDOWS_RESERVED_CHARACTERS_REPLACEMENTS[char])
+ else:
+ new_filename_chars.append(char)
+ filename = ''.join(new_filename_chars)
+ if filename.endswith('.') or filename.endswith(' '):
+ # Filenames cannot end with a period or space on Windows.
+ filename = filename[:-1] + '_'
+
+ return filename
+
+
+def sanitize_filename(filename):
+ """Sanitizes a filename for various operating systems.
+
+ Args:
+ filename: string, the filename to sanitize.
+
+ Returns:
+ A string that is safe to use as a filename on various operating systems.
+ """
+ # Split `filename` into the directory and filename in case the user
+ # accidentally passed in the full path instead of the name.
+ dirname = os.path.dirname(filename)
+ basename = os.path.basename(filename)
+ basename = _sanitize_windows_filename(basename)
+ # Replace spaces with underscores for convenience reasons.
+ basename = basename.replace(' ', '_')
+ return os.path.join(dirname, basename)
+
+
def normalize_log_line_timestamp(log_line_timestamp):
"""Replace special characters in log line timestamp with normal characters.
+ .. deprecated:: 1.9.2
+
+ This method is obsolete with the more general `sanitize_filename` method
+ and is only kept for backwards compatibility. In a future update, this
+ method may be removed.
+
Args:
log_line_timestamp: A string in the log line timestamp format. Obtained
with get_log_line_timestamp.
@@ -230,6 +328,4 @@
A string representing the same time as input timestamp, but without
special characters.
"""
- norm_tp = log_line_timestamp.replace(' ', '_')
- norm_tp = norm_tp.replace(':', '-')
- return norm_tp
+ return sanitize_filename(log_line_timestamp)
diff --git a/tests/mobly/logger_test.py b/tests/mobly/logger_test.py
index 2e34d46..2ad064c 100755
--- a/tests/mobly/logger_test.py
+++ b/tests/mobly/logger_test.py
@@ -13,6 +13,7 @@
# limitations under the License.
import mock
+import os
import pytz
import shutil
import tempfile
@@ -72,6 +73,133 @@
mock_create_latest_log_alias.assert_called_once_with(self.log_dir,
alias=mock_alias)
+ def test_sanitize_filename_when_valid(self):
+ fake_filename = 'logcat.txt'
+ expected_filename = 'logcat.txt'
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
+ def test_sanitize_filename_when_valid_with_path(self):
+ fake_filename = os.path.join('dir', 'logs', 'logcat.txt')
+ expected_filename = os.path.join('dir', 'logs', 'logcat.txt')
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
+ def test_sanitize_filename_when_random_spaces(self):
+ fake_filename = 'log cat file.txt'
+ expected_filename = 'log_cat_file.txt'
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
+ def test_sanitize_filename_when_over_260_characters(self):
+ fake_filename = 'l' * 300
+ expected_filename = 'l' * 260
+ self.assertEqual(len(logger.sanitize_filename(fake_filename)), 260)
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
+ def test__sanitize_windows_filename_when_path_characters(self):
+ fake_filename = '/\\'
+ expected_filename = '__'
+ self.assertEqual(logger._sanitize_windows_filename(fake_filename),
+ expected_filename)
+
+ def test_sanitize_filename_when_specical_characters(self):
+ fake_filename = '<>:"|?*\x00'
+ expected_filename = '---_,,,0'
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
+ def test_sanitize_filename_when_con(self):
+ for fake_filename, expected_filename in [
+ ('con', 'mobly_con'),
+ ('CON', 'mobly_CON'),
+ ('con.txt', 'mobly_con.txt'),
+ ('connections.log', 'connections.log'),
+ ]:
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
+ def test_sanitize_filename_when_prn(self):
+ for fake_filename, expected_filename in [
+ ('prn', 'mobly_prn'),
+ ('PRN', 'mobly_PRN'),
+ ('prn.txt', 'mobly_prn.txt'),
+ ('prnters.log', 'prnters.log'),
+ ]:
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
+ def test_sanitize_filename_when_aux(self):
+ for fake_filename, expected_filename in [
+ ('aux', 'mobly_aux'),
+ ('AUX', 'mobly_AUX'),
+ ('aux.txt', 'mobly_aux.txt'),
+ ('auxiliaries.log', 'auxiliaries.log'),
+ ]:
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
+ def test_sanitize_filename_when_nul(self):
+ for fake_filename, expected_filename in [
+ ('nul', 'mobly_nul'),
+ ('NUL', 'mobly_NUL'),
+ ('nul.txt', 'mobly_nul.txt'),
+ ('nullptrs.log', 'nullptrs.log'),
+ ]:
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
+ def test_sanitize_filename_when_com(self):
+ for fake_filename, expected_filename in [
+ ('com', 'com'),
+ ('COM0', 'mobly_COM0'),
+ ('com1', 'mobly_com1'),
+ ('COM2', 'mobly_COM2'),
+ ('com3', 'mobly_com3'),
+ ('COM4', 'mobly_COM4'),
+ ('com5', 'mobly_com5'),
+ ('COM6', 'mobly_COM6'),
+ ('com7', 'mobly_com7'),
+ ('COM8', 'mobly_COM8'),
+ ('com9', 'mobly_com9'),
+ ('com0.log', 'mobly_com0.log'),
+ ('com0files.log', 'com0files.log'),
+ ]:
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
+ def test_sanitize_filename_when_lpt(self):
+ for fake_filename, expected_filename in [
+ ('lpt', 'lpt'),
+ ('LPT0', 'mobly_LPT0'),
+ ('lpt1', 'mobly_lpt1'),
+ ('LPT2', 'mobly_LPT2'),
+ ('lpt3', 'mobly_lpt3'),
+ ('LPT4', 'mobly_LPT4'),
+ ('lpt5', 'mobly_lpt5'),
+ ('LPT6', 'mobly_LPT6'),
+ ('lpt7', 'mobly_lpt7'),
+ ('LPT8', 'mobly_LPT8'),
+ ('lpt9', 'mobly_lpt9'),
+ ('lpt3.txt', 'mobly_lpt3.txt'),
+ ('lpt3_file.txt', 'lpt3_file.txt'),
+ ]:
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
+ def test_sanitize_filename_when_ends_with_space(self):
+ fake_filename = 'logcat.txt '
+ expected_filename = 'logcat.txt_'
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
+ def test_sanitize_filename_when_ends_with_period(self):
+ fake_filename = 'logcat.txt.'
+ expected_filename = 'logcat.txt_'
+ self.assertEqual(logger.sanitize_filename(fake_filename),
+ expected_filename)
+
if __name__ == "__main__":
unittest.main()
