Island: Raise distinct errors when openning a file

IFileRepository now distincts between file not found and a file that
could not be retrieved
This commit is contained in:
Ilija Lazoroski 2022-06-20 14:55:26 +02:00
parent b959763318
commit 3cb678ad32
3 changed files with 22 additions and 7 deletions

View File

@ -30,7 +30,8 @@ class IFileRepository(metaclass=abc.ABCMeta):
:param unsafe_file_name: An unsanitized file name that identifies the file to be opened :param unsafe_file_name: An unsanitized file name that identifies the file to be opened
:return: A file-like object providing access to the file's contents :return: A file-like object providing access to the file's contents
:raises FileRetrievalError: if the file cannot be opened :raises FileRetrievalError: if the file cannot be retrieved
:raises FileNotFoundError: if the file cannot be found
""" """
pass pass

View File

@ -1,3 +1,4 @@
import errno
import logging import logging
import shutil import shutil
from pathlib import Path from pathlib import Path
@ -44,11 +45,12 @@ class LocalStorageFileRepository(IFileRepository):
logger.debug(f"Opening {safe_file_path}") logger.debug(f"Opening {safe_file_path}")
return open(safe_file_path, "rb") return open(safe_file_path, "rb")
except OSError as err: except OSError as err:
# TODO: The interface should make a destinction between file not found and an error when if err.errno == errno.ENOENT:
# retrieving a file that should exist. The built-in `FileNotFoundError` is not raise FileNotFoundError(f"No file {safe_file_path} found: {err}") from err
# sufficient because it inherits from `OSError` and the interface does not else:
# guarantee that the file is stored on the local file system. raise FileRetrievalError(
raise FileRetrievalError(f"Failed to retrieve file {safe_file_path}: {err}") from err f"Failed to retrieve file {safe_file_path}: {err}"
) from err
def delete_file(self, unsafe_file_name: str): def delete_file(self, unsafe_file_name: str):
safe_file_path = self._get_safe_file_path(unsafe_file_name) safe_file_path = self._get_safe_file_path(unsafe_file_name)

View File

@ -1,5 +1,7 @@
import errno
import io import io
from pathlib import Path from pathlib import Path
from unittest.mock import Mock, patch
import pytest import pytest
from tests.monkey_island.utils import assert_linux_permissions, assert_windows_permissions from tests.monkey_island.utils import assert_linux_permissions, assert_windows_permissions
@ -131,5 +133,15 @@ def test_remove_nonexistant_file(tmp_path):
def test_open_nonexistant_file(tmp_path): def test_open_nonexistant_file(tmp_path):
fss = LocalStorageFileRepository(tmp_path) fss = LocalStorageFileRepository(tmp_path)
with pytest.raises(FileRetrievalError): with pytest.raises(FileNotFoundError):
fss.open_file("nonexistant_file.txt") fss.open_file("nonexistant_file.txt")
def test_open_locked_file(tmp_path, monkeypatch):
fss = LocalStorageFileRepository(tmp_path)
with patch(
"builtins.open", Mock(side_effect=OSError(errno.EIO, "File could not be retrieved"))
):
with pytest.raises(FileRetrievalError):
fss.open_file("locked_file.txt")