Ruff checks
This commit is contained in:
parent
fc437f529c
commit
968237bec0
2 changed files with 81 additions and 37 deletions
|
|
@ -111,8 +111,7 @@ _WINDOWS_RESERVED_NAMES = {
|
||||||
|
|
||||||
|
|
||||||
def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]:
|
def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]:
|
||||||
"""Check if a file path has any unsafe elements in it, such as
|
"""Check if a file path has any unsafe elements in it.
|
||||||
unsafe characters or reserved names for its OS.
|
|
||||||
|
|
||||||
The path is not coerced into a safe form because if we ask
|
The path is not coerced into a safe form because if we ask
|
||||||
for a file to be written to a location we shouldn't be changing
|
for a file to be written to a location we shouldn't be changing
|
||||||
|
|
@ -124,6 +123,7 @@ def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]:
|
||||||
any unsafe features were found.
|
any unsafe features were found.
|
||||||
"""
|
"""
|
||||||
# Split by separators first to sanitise components independently
|
# Split by separators first to sanitise components independently
|
||||||
|
|
||||||
components = re.split(r"([/\\])", unsafe_path_string)
|
components = re.split(r"([/\\])", unsafe_path_string)
|
||||||
|
|
||||||
unsafe_character_pattern = (
|
unsafe_character_pattern = (
|
||||||
|
|
@ -146,10 +146,16 @@ def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]:
|
||||||
|
|
||||||
# 1. Check for relative paths in the wrong place
|
# 1. Check for relative paths in the wrong place
|
||||||
if component in (".", "..") and not unsafe_relative_path:
|
if component in (".", "..") and not unsafe_relative_path:
|
||||||
is_first = (i == 0)
|
is_first = i == 0
|
||||||
is_second = (i == 2 and components[i-1] in ("/", "\\") and components[i-2] == "..")
|
is_second = (
|
||||||
|
i == 2
|
||||||
|
and components[i - 1] in ("/", "\\")
|
||||||
|
and components[i - 2] == ".."
|
||||||
|
)
|
||||||
if not (is_first or is_second):
|
if not (is_first or is_second):
|
||||||
LOGGER.warning(f"File path {unsafe_path_string} may be unsafe due to unexpected relative navigation.")
|
LOGGER.warning(
|
||||||
|
f"File path {unsafe_path_string} may be unsafe due to unexpected relative navigation."
|
||||||
|
)
|
||||||
unsafe_relative_path = True
|
unsafe_relative_path = True
|
||||||
continue
|
continue
|
||||||
|
|
||||||
|
|
@ -157,26 +163,45 @@ def make_path_safe(unsafe_path_string: str) -> tuple[str, bool]:
|
||||||
if "." in component and i != len(components) - 1 and not trailing_dots:
|
if "." in component and i != len(components) - 1 and not trailing_dots:
|
||||||
# Check for trailing dots in the file path before the file name
|
# Check for trailing dots in the file path before the file name
|
||||||
# e.g.: foo/bar./file is bad, but foo/bar.file.py is fine
|
# e.g.: foo/bar./file is bad, but foo/bar.file.py is fine
|
||||||
LOGGER.warning(f'File path {unsafe_path_string} may be unsafe due to trailing dots.')
|
LOGGER.warning(
|
||||||
|
f"File path {unsafe_path_string} may be unsafe due to trailing dots."
|
||||||
|
)
|
||||||
trailing_dots = True
|
trailing_dots = True
|
||||||
|
|
||||||
# Check for trailing spaces - Windows specific
|
# Check for trailing spaces - Windows specific
|
||||||
if sys.platform.startswith("win") and component.endswith(" ") and not trailing_whitespace:
|
if (
|
||||||
|
sys.platform.startswith("win")
|
||||||
|
and component.endswith(" ")
|
||||||
|
and not trailing_whitespace
|
||||||
|
):
|
||||||
LOGGER.warning(f"{unsafe_path_string} contains unsafe trailing spaces.")
|
LOGGER.warning(f"{unsafe_path_string} contains unsafe trailing spaces.")
|
||||||
trailing_whitespace = True
|
trailing_whitespace = True
|
||||||
|
|
||||||
# 3. Check for unsafe characters (Regex)
|
# 3. Check for unsafe characters (Regex)
|
||||||
if unsafe_character_pattern.search(component) and not unsafe_characters:
|
if unsafe_character_pattern.search(component) and not unsafe_characters:
|
||||||
LOGGER.warning(f"{unsafe_path_string} contains characters that may be unsafe on this platform.")
|
LOGGER.warning(
|
||||||
|
f"{unsafe_path_string} contains characters that may be unsafe on this platform."
|
||||||
|
)
|
||||||
unsafe_characters = True
|
unsafe_characters = True
|
||||||
|
|
||||||
# 4. Check for Reserved Names (e.g., CON, PRN, LPT1)
|
# 4. Check for Reserved Names (e.g., CON, PRN, LPT1)
|
||||||
# Assuming _sanitise_reserved returns a different string if it's a reserved name
|
# Assuming _sanitise_reserved returns a different string if it's a reserved name
|
||||||
if _sanitise_reserved(component, is_filename=False) != component and not reserved_word:
|
if (
|
||||||
LOGGER.warning(f"{unsafe_path_string} contains a reserved system name and may cause issues.")
|
_sanitise_reserved(component, is_filename=False) != component
|
||||||
|
and not reserved_word
|
||||||
|
):
|
||||||
|
LOGGER.warning(
|
||||||
|
f"{unsafe_path_string} contains a reserved system name and may cause issues."
|
||||||
|
)
|
||||||
reserved_word = True
|
reserved_word = True
|
||||||
|
|
||||||
warning_raised = unsafe_relative_path or trailing_dots or trailing_whitespace or unsafe_characters or reserved_word
|
warning_raised = (
|
||||||
|
unsafe_relative_path
|
||||||
|
or trailing_dots
|
||||||
|
or trailing_whitespace
|
||||||
|
or unsafe_characters
|
||||||
|
or reserved_word
|
||||||
|
)
|
||||||
return unsafe_path_string, warning_raised
|
return unsafe_path_string, warning_raised
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,26 +1,19 @@
|
||||||
"""Unit tests for utility functions."""
|
"""Unit tests for utility functions."""
|
||||||
|
|
||||||
import sys
|
|
||||||
import logging
|
import logging
|
||||||
|
import sys
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from openflexure_microscope_server.utilities import (
|
from openflexure_microscope_server.utilities import (
|
||||||
_WINDOWS_RESERVED_NAMES,
|
_WINDOWS_RESERVED_NAMES,
|
||||||
make_name_safe,
|
make_name_safe,
|
||||||
make_path_safe
|
make_path_safe,
|
||||||
)
|
)
|
||||||
|
|
||||||
PATHS_WITH_DOTS = {
|
PATHS_WITH_DOTS = {"a./b/c.", "path./to/file.", "./path./to/file."}
|
||||||
"a./b/c.",
|
|
||||||
"path./to/file.",
|
|
||||||
"./path./to/file."
|
|
||||||
}
|
|
||||||
|
|
||||||
RESERVED_NAME_PATHS = {
|
RESERVED_NAME_PATHS = {"path/CON/file", "path/NUL.txt/file", "C:/AUX/test"}
|
||||||
"path/CON/file",
|
|
||||||
"path/NUL.txt/file",
|
|
||||||
"C:/AUX/test"
|
|
||||||
}
|
|
||||||
|
|
||||||
if sys.platform.startswith("win"):
|
if sys.platform.startswith("win"):
|
||||||
UNSAFE_CHARACTER_PATHS = {
|
UNSAFE_CHARACTER_PATHS = {
|
||||||
|
|
@ -34,7 +27,7 @@ else:
|
||||||
"/var/log/app:backup.log",
|
"/var/log/app:backup.log",
|
||||||
"/etc/configs/network-settings\\v1",
|
"/etc/configs/network-settings\\v1",
|
||||||
"/tmp/file_with_@_symbol.txt",
|
"/tmp/file_with_@_symbol.txt",
|
||||||
"/home/user/script(1).py"
|
"/home/user/script(1).py",
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -89,10 +82,16 @@ def test_make_path_safe_basic(caplog):
|
||||||
with caplog.at_level(logging.WARNING):
|
with caplog.at_level(logging.WARNING):
|
||||||
if sys.platform.startswith("win"):
|
if sys.platform.startswith("win"):
|
||||||
assert make_path_safe("C:\\path\\to/file")[0] == "C:\\path\\to/file"
|
assert make_path_safe("C:\\path\\to/file")[0] == "C:\\path\\to/file"
|
||||||
assert make_path_safe("C:\\a very long path\\to/file")[0] == "C:\\a very long path\\to/file"
|
assert (
|
||||||
|
make_path_safe("C:\\a very long path\\to/file")[0]
|
||||||
|
== "C:\\a very long path\\to/file"
|
||||||
|
)
|
||||||
else:
|
else:
|
||||||
assert make_path_safe("path/to/file")[0] == "path/to/file"
|
assert make_path_safe("path/to/file")[0] == "path/to/file"
|
||||||
assert make_path_safe("a very long path/to/file")[0] == "a very long path/to/file"
|
assert (
|
||||||
|
make_path_safe("a very long path/to/file")[0]
|
||||||
|
== "a very long path/to/file"
|
||||||
|
)
|
||||||
|
|
||||||
# No errors should be raised. These paths are safe.
|
# No errors should be raised. These paths are safe.
|
||||||
assert len(caplog.records) == 0
|
assert len(caplog.records) == 0
|
||||||
|
|
@ -103,7 +102,10 @@ def test_make_path_safe_unsafe_characters_in_components(caplog, path):
|
||||||
"""Test unsafe characters within path components."""
|
"""Test unsafe characters within path components."""
|
||||||
with caplog.at_level(logging.WARNING):
|
with caplog.at_level(logging.WARNING):
|
||||||
assert make_path_safe(path)[0] == path
|
assert make_path_safe(path)[0] == path
|
||||||
assert f"{path} contains characters that may be unsafe on this platform."
|
assert (
|
||||||
|
f"{path} contains characters that may be unsafe on this platform."
|
||||||
|
in caplog.messages
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize("path", RESERVED_NAME_PATHS)
|
@pytest.mark.parametrize("path", RESERVED_NAME_PATHS)
|
||||||
|
|
@ -111,29 +113,40 @@ def test_make_path_safe_reserved_in_components(caplog, path):
|
||||||
"""Test reserved names within path components."""
|
"""Test reserved names within path components."""
|
||||||
with caplog.at_level(logging.WARNING):
|
with caplog.at_level(logging.WARNING):
|
||||||
assert make_path_safe(path)[0] == path
|
assert make_path_safe(path)[0] == path
|
||||||
assert f"{path} contains a reserved system name and may cause issues."
|
assert (
|
||||||
|
f"{path} contains a reserved system name and may cause issues."
|
||||||
|
in caplog.messages
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_make_path_safe_trailing_space_in_components(caplog):
|
def test_make_path_safe_trailing_space_in_components(caplog):
|
||||||
"""Test trailing spaces in path components."""
|
"""Test trailing spaces in path components."""
|
||||||
with caplog.at_level(logging.WARNING):
|
with caplog.at_level(logging.WARNING):
|
||||||
# Test Windows - cannot have trailingspaces
|
# Test Windows - cannot have trailingspaces
|
||||||
if sys.platform.startswith("win"):
|
if sys.platform.startswith("win"):
|
||||||
assert make_path_safe("path /to /file ")[0] == "path /to /file "
|
assert make_path_safe("path /to /file ")[0] == "path /to /file "
|
||||||
assert make_path_safe("path/to /file")[0] == "path/to /file"
|
assert make_path_safe("path/to /file")[0] == "path/to /file"
|
||||||
# Check directories can have spaces in them but not at the end
|
# Check directories can have spaces in them but not at the end
|
||||||
# TODO - dont want underscores, want to keep interim spaces
|
assert (
|
||||||
assert make_path_safe("path/a long path name /file")[0] == "path/a long path name /file"
|
make_path_safe("path/a long path name /file")[0]
|
||||||
|
== "path/a long path name /file"
|
||||||
|
)
|
||||||
assert len(caplog.records) == 3
|
assert len(caplog.records) == 3
|
||||||
assert f"path /to /file contains unsafe trailing spaces." in caplog.messages
|
assert "path /to /file contains unsafe trailing spaces." in caplog.messages
|
||||||
assert f"path/to /file contains unsafe trailing spaces." in caplog.messages
|
assert "path/to /file contains unsafe trailing spaces." in caplog.messages
|
||||||
assert f"path/a long path name /file contains unsafe trailing spaces." in caplog.messages
|
assert (
|
||||||
|
"path/a long path name /file contains unsafe trailing spaces."
|
||||||
|
in caplog.messages
|
||||||
|
)
|
||||||
else:
|
else:
|
||||||
# Trailing spaces allows in POSIX systems
|
# Trailing spaces allows in POSIX systems
|
||||||
assert make_path_safe("path /to /file ")[0] == "path /to /file "
|
assert make_path_safe("path /to /file ")[0] == "path /to /file "
|
||||||
assert make_path_safe("path/to /file")[0] == "path/to /file"
|
assert make_path_safe("path/to /file")[0] == "path/to /file"
|
||||||
# Check directories can have spaces in them but not at the end
|
# Check directories can have spaces in them but not at the end
|
||||||
assert make_path_safe("path/a long path name /file")[0] == "path/a long path name file"
|
assert (
|
||||||
|
make_path_safe("path/a long path name /file")[0]
|
||||||
|
== "path/a long path name file"
|
||||||
|
)
|
||||||
# No warnings should be raised
|
# No warnings should be raised
|
||||||
assert len(caplog.records) == 0
|
assert len(caplog.records) == 0
|
||||||
|
|
||||||
|
|
@ -152,7 +165,10 @@ def test_make_path_safe_relative(caplog):
|
||||||
|
|
||||||
assert make_path_safe("a_bad/../relative_path")[0] == "a_bad/../relative_path"
|
assert make_path_safe("a_bad/../relative_path")[0] == "a_bad/../relative_path"
|
||||||
assert len(caplog.records) == 1
|
assert len(caplog.records) == 1
|
||||||
assert(f"File path a_bad/../relative_path may be unsafe due to unexpected relative navigation.")
|
assert (
|
||||||
|
"File path a_bad/../relative_path may be unsafe due to unexpected relative navigation."
|
||||||
|
in caplog.messages
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize("filepath", PATHS_WITH_DOTS)
|
@pytest.mark.parametrize("filepath", PATHS_WITH_DOTS)
|
||||||
|
|
@ -160,4 +176,7 @@ def test_make_path_safe_dots_warning(caplog, filepath):
|
||||||
"""Test that a user is warned about trailing dots in their file paths. The paths should remain unchanged."""
|
"""Test that a user is warned about trailing dots in their file paths. The paths should remain unchanged."""
|
||||||
with caplog.at_level(logging.WARNING):
|
with caplog.at_level(logging.WARNING):
|
||||||
assert make_path_safe(filepath)[0] == filepath
|
assert make_path_safe(filepath)[0] == filepath
|
||||||
assert f"File path {filepath} may be unsafe due to trailing dots." in caplog.messages
|
assert (
|
||||||
|
f"File path {filepath} may be unsafe due to trailing dots."
|
||||||
|
in caplog.messages
|
||||||
|
)
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue