From 16fe684787f15c6991509bbaec0c5afac3855157 Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Thu, 18 Jul 2024 22:46:33 +0000 Subject: [PATCH 01/14] Prevent chown on Windows Use sys.platform checks to please mypy check sys.platform not platform.system Use sys.platform checks to please mypy check sys.platform not platform.system --- ofrak_core/ofrak/core/filesystem.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/ofrak_core/ofrak/core/filesystem.py b/ofrak_core/ofrak/core/filesystem.py index b9c655a81..c9b4b52ad 100644 --- a/ofrak_core/ofrak/core/filesystem.py +++ b/ofrak_core/ofrak/core/filesystem.py @@ -1,5 +1,6 @@ import os import stat +import sys import tempfile from dataclasses import dataclass from typing import Dict, Iterable, Optional, Type, Union @@ -134,8 +135,9 @@ def apply_stat_attrs(self, path: str): :param path: Path on disk to set attributes of. """ if self.stat: - os.chown(path, self.stat.st_uid, self.stat.st_gid) - os.chmod(path, self.stat.st_mode) + if sys.platform != "win32": + os.chown(path, self.stat.st_uid, self.stat.st_gid) + os.chmod(path, self.stat.st_mode) os.utime(path, (self.stat.st_atime, self.stat.st_mtime)) if self.xattrs: for attr, value in self.xattrs.items(): @@ -174,10 +176,13 @@ async def flush_to_disk(self, root_path: str = ".", filename: Optional[str] = No assert len(list(await self.resource.get_children())) == 0 if self.stat: # https://docs.python.org/3/library/os.html#os.supports_follow_symlinks - if os.chown in os.supports_follow_symlinks: - os.chown(link_name, self.stat.st_uid, self.stat.st_gid, follow_symlinks=False) - if os.chmod in os.supports_follow_symlinks: - os.chmod(link_name, self.stat.st_mode, follow_symlinks=False) + if sys.platform != "Windows": + if os.chown in os.supports_follow_symlinks: + os.chown( + link_name, self.stat.st_uid, self.stat.st_gid, follow_symlinks=False + ) + if os.chmod in os.supports_follow_symlinks: + os.chmod(link_name, self.stat.st_mode, follow_symlinks=False) if os.utime in os.supports_follow_symlinks: os.utime( link_name, From 9cdd968f9a1b8d2da2a50326c09085ccb6ce5046 Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Fri, 19 Jul 2024 14:29:16 +0000 Subject: [PATCH 02/14] Make filesystem.py more robust on Windows --- ofrak_core/ofrak/core/filesystem.py | 69 +++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 13 deletions(-) diff --git a/ofrak_core/ofrak/core/filesystem.py b/ofrak_core/ofrak/core/filesystem.py index c9b4b52ad..a1e110846 100644 --- a/ofrak_core/ofrak/core/filesystem.py +++ b/ofrak_core/ofrak/core/filesystem.py @@ -1,6 +1,7 @@ import os import stat import sys +import warnings import tempfile from dataclasses import dataclass from typing import Dict, Iterable, Optional, Type, Union @@ -23,6 +24,22 @@ ) +def _warn_chmod_chown_windows(): + warnings.warn( + f"os.chown and os.chmod do not work on Windows platforms. \ + Unix-like file ownership and permissions will not be properly handled while using OFRAK on this platform. \ + If you require extended attributes, please use a different platform." + ) + + +def _warn_chmod_chown_windows(): + warnings.warn( + f"os.chown and os.chmod do not work on Windows platforms. \ + Unix-like file ownership and permissions will not be properly handled while using OFRAK on this platform. \ + If you require extended attributes, please use a different platform." + ) + + @dataclass class FilesystemEntry(ResourceView): """ @@ -135,7 +152,9 @@ def apply_stat_attrs(self, path: str): :param path: Path on disk to set attributes of. """ if self.stat: - if sys.platform != "win32": + if sys.platform == "win32": + _warn_chmod_chown_windows() + else: os.chown(path, self.stat.st_uid, self.stat.st_gid) os.chmod(path, self.stat.st_mode) os.utime(path, (self.stat.st_atime, self.stat.st_mtime)) @@ -175,8 +194,10 @@ async def flush_to_disk(self, root_path: str = ".", filename: Optional[str] = No os.symlink(link_view.source_path, link_name) assert len(list(await self.resource.get_children())) == 0 if self.stat: - # https://docs.python.org/3/library/os.html#os.supports_follow_symlinks - if sys.platform != "Windows": + if sys.platform == "win32": + _warn_chmod_chown_windows() + else: + # https://docs.python.org/3/library/os.html#os.supports_follow_symlinks if os.chown in os.supports_follow_symlinks: os.chown( link_name, self.stat.st_uid, self.stat.st_gid, follow_symlinks=False @@ -203,21 +224,43 @@ async def flush_to_disk(self, root_path: str = ".", filename: Optional[str] = No self.apply_stat_attrs(file_name) elif self.is_device(): device_name = os.path.join(root_path, entry_path) - if self.stat is None: - raise ValueError( + if sys.platform == "win32" or not hasattr(os, "mknod"): + warnings.warn( f"Cannot create a device {entry_path} for a " - f"BlockDevice or CharacterDevice resource with no stat!" + f"BlockDevice or CharacterDevice resource on platform {sys.platform}! " + f"Creating an empty regular file instead." ) - os.mknod(device_name, self.stat.st_mode, self.stat.st_rdev) - self.apply_stat_attrs(device_name) + with open(device_name, "w") as f: + pass + self.apply_stat_attrs(device_name) + else: + if self.stat is None: + raise ValueError( + f"Cannot create a device {entry_path} for a " + f"BlockDevice or CharacterDevice resource with no stat!" + ) + os.mknod(device_name, self.stat.st_mode, self.stat.st_rdev) + self.apply_stat_attrs(device_name) elif self.is_fifo_pipe(): fifo_name = os.path.join(root_path, entry_path) - if self.stat is None: - raise ValueError( - f"Cannot create a fifo {entry_path} for a FIFOPipe resource " "with no stat!" + if sys.platform == "win32" or not hasattr(os, "mkfifo"): + warnings.warn( + f"Cannot create a fifo {entry_path} for a " + f"FIFOPipe resource on platform {sys.platform}! " + f"Creating an empty regular file instead." ) - os.mkfifo(fifo_name, self.stat.st_mode) - self.apply_stat_attrs(fifo_name) + with open(fifo_name, "w") as f: + pass + self.apply_stat_attrs(fifo_name) + else: + fifo_name = os.path.join(root_path, entry_path) + if self.stat is None: + raise ValueError( + f"Cannot create a fifo {entry_path} for a FIFOPipe resource " + "with no stat!" + ) + os.mkfifo(fifo_name, self.stat.st_mode) + self.apply_stat_attrs(fifo_name) else: entry_info = f"Stat: {stat.S_IFMT(self.stat.st_mode):o}" if self.stat else "" raise NotImplementedError( From 45dfb5c7bf500640e3e4843a22371c7002057d72 Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Fri, 19 Jul 2024 14:43:12 +0000 Subject: [PATCH 03/14] Remove hardcoded forward slashes from filesystem.py --- ofrak_core/ofrak/core/filesystem.py | 8 ++++---- .../test_ofrak/components/test_filesystem_component.py | 9 ++++++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/ofrak_core/ofrak/core/filesystem.py b/ofrak_core/ofrak/core/filesystem.py index a1e110846..b281d3b1f 100644 --- a/ofrak_core/ofrak/core/filesystem.py +++ b/ofrak_core/ofrak/core/filesystem.py @@ -52,8 +52,8 @@ class FilesystemEntry(ResourceView): @index def Name(self) -> str: - name = self.name.rstrip("/") - return name.split("/")[-1] + name = self.name.rstrip(os.path.sep) + return name.split(os.path.sep)[-1] @classmethod def caption(cls, all_attributes) -> str: @@ -304,7 +304,7 @@ async def get_entry(self, path: str) -> Optional[FilesystemEntry]: for d in descendants: descendant_path = await d.get_path() - if descendant_path.split(f"{self.name}/")[-1] == path: + if descendant_path.split(f"{self.name}{os.path.sep}")[-1] == path: return d return None @@ -581,7 +581,7 @@ async def add_folder( """ # Normalizes and cleans up paths beginning with "./" and containing "./../" as well as # other extraneous separators - split_dir = os.path.normpath(path).rstrip("/").lstrip("/").split("/") + split_dir = os.path.normpath(path).strip(os.path.sep).split(os.path.sep) parent: Union[FilesystemRoot, Folder] = self for directory in split_dir: diff --git a/ofrak_core/test_ofrak/components/test_filesystem_component.py b/ofrak_core/test_ofrak/components/test_filesystem_component.py index 08339b04d..44cfe9a21 100644 --- a/ofrak_core/test_ofrak/components/test_filesystem_component.py +++ b/ofrak_core/test_ofrak/components/test_filesystem_component.py @@ -2,6 +2,8 @@ import re import stat import subprocess +import sys +import warnings import tempfile import pytest @@ -104,7 +106,12 @@ async def test_flush_to_disk(self, ofrak_context: OFRAKContext): with tempfile.TemporaryDirectory() as flush_dir: await filesystem_root.flush_to_disk(flush_dir) - diff_directories(temp_dir, flush_dir, extra_diff_flags="") + if sys.platform != "win32": + diff_directories(temp_dir, flush_dir, extra_diff_flags="") + else: + warnings.warn( + "Directories not compared on Windows. TODO: implement a basic comparison" + ) async def test_get_entry(self, filesystem_root: FilesystemRoot): """ From e0f0dca6f11996895cf0a9bf5ddcded0deacee74 Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Fri, 19 Jul 2024 15:49:43 +0000 Subject: [PATCH 04/14] Represent filesystem paths in POSIX style internally --- ofrak_core/ofrak/core/filesystem.py | 71 ++++++++++++++++++----------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/ofrak_core/ofrak/core/filesystem.py b/ofrak_core/ofrak/core/filesystem.py index b281d3b1f..fb721a9f0 100644 --- a/ofrak_core/ofrak/core/filesystem.py +++ b/ofrak_core/ofrak/core/filesystem.py @@ -3,6 +3,8 @@ import sys import warnings import tempfile +import posixpath +from pathlib import PurePath from dataclasses import dataclass from typing import Dict, Iterable, Optional, Type, Union @@ -52,8 +54,8 @@ class FilesystemEntry(ResourceView): @index def Name(self) -> str: - name = self.name.rstrip(os.path.sep) - return name.split(os.path.sep)[-1] + name = self.name.rstrip("/") + return name.split("/")[-1] @classmethod def caption(cls, all_attributes) -> str: @@ -127,6 +129,7 @@ async def get_path(self) -> str: Get a folder's path, with the `FilesystemRoot` as the path root. :return: The full path name, with the `FilesystemRoot` ancestor as the path root + Always returns POSIX style paths with forward slashes. """ path = [self.get_name()] @@ -140,9 +143,8 @@ async def get_path(self) -> str: break a_view = await a.view_as(FilesystemEntry) path.append(a_view.get_name()) - path.reverse() - return os.path.join(*path) + return posixpath.join(*reversed(path)) def apply_stat_attrs(self, path: str): """ @@ -253,7 +255,6 @@ async def flush_to_disk(self, root_path: str = ".", filename: Optional[str] = No pass self.apply_stat_attrs(fifo_name) else: - fifo_name = os.path.join(root_path, entry_path) if self.stat is None: raise ValueError( f"Cannot create a fifo {entry_path} for a FIFOPipe resource " @@ -293,7 +294,7 @@ async def get_entry(self, path: str) -> Optional[FilesystemEntry]: :return: The child `FilesystemEntry` resource that was found. If nothing was found, `None` is returned """ - basename = os.path.basename(path) + basename = posixpath.basename(path) # only searching paths with the same base name should reduce the search space by quite a lot descendants = await self.resource.get_descendants_as_view( FilesystemEntry, @@ -304,7 +305,7 @@ async def get_entry(self, path: str) -> Optional[FilesystemEntry]: for d in descendants: descendant_path = await d.get_path() - if descendant_path.split(f"{self.name}{os.path.sep}")[-1] == path: + if descendant_path.split(f"{self.name}/")[-1] == path: return d return None @@ -361,6 +362,13 @@ class CharacterDevice(SpecialFileType): """ +def _path_to_posixpath(path: str) -> str: + """ + Converts an OS-specific path to a POSIX style path + """ + return str(PurePath(path).as_posix()) + + @dataclass class FilesystemRoot(ResourceView): """ @@ -389,7 +397,10 @@ async def initialize_from_disk( for root, dirs, files in os.walk(root_path): for d in sorted(dirs): absolute_path = os.path.join(root, d) - relative_path = os.path.join(os.path.relpath(root, root_path), d) + relative_path_posix = posixpath.join( + _path_to_posixpath(os.path.relpath(root, root_path)), d + ) + folder_attributes_stat = os.lstat(absolute_path) mode = folder_attributes_stat.st_mode @@ -413,9 +424,9 @@ async def initialize_from_disk( folder_attributes_xattr = self._get_xattr_map(absolute_path) if os.path.islink(absolute_path): await self.add_special_file_entry( - relative_path, + relative_path_posix, SymbolicLink( - relative_path, + relative_path_posix, folder_attributes_stat, folder_attributes_xattr, os.readlink(absolute_path), @@ -423,13 +434,15 @@ async def initialize_from_disk( ) else: await self.add_folder( - relative_path, + relative_path_posix, folder_attributes_stat, folder_attributes_xattr, ) for f in sorted(files): absolute_path = os.path.join(root, f) - relative_path = os.path.normpath(os.path.join(os.path.relpath(root, root_path), f)) + relative_path_posix = _path_to_posixpath( + os.path.normpath(os.path.join(os.path.relpath(root, root_path), f)) + ) file_attributes_stat = os.lstat(absolute_path) mode = file_attributes_stat.st_mode @@ -450,9 +463,9 @@ async def initialize_from_disk( file_attributes_xattr = self._get_xattr_map(absolute_path) if os.path.islink(absolute_path): await self.add_special_file_entry( - relative_path, + relative_path_posix, SymbolicLink( - relative_path, + relative_path_posix, file_attributes_stat, file_attributes_xattr, os.readlink(absolute_path), @@ -461,25 +474,29 @@ async def initialize_from_disk( elif os.path.isfile(absolute_path): with open(absolute_path, "rb") as fh: await self.add_file( - relative_path, + relative_path_posix, fh.read(), file_attributes_stat, file_attributes_xattr, ) elif stat.S_ISFIFO(mode): await self.add_special_file_entry( - relative_path, - FIFOPipe(relative_path, file_attributes_stat, file_attributes_xattr), + relative_path_posix, + FIFOPipe(relative_path_posix, file_attributes_stat, file_attributes_xattr), ) elif stat.S_ISBLK(mode): await self.add_special_file_entry( - relative_path, - BlockDevice(relative_path, file_attributes_stat, file_attributes_xattr), + relative_path_posix, + BlockDevice( + relative_path_posix, file_attributes_stat, file_attributes_xattr + ), ) elif stat.S_ISCHR(mode): await self.add_special_file_entry( - relative_path, - CharacterDevice(relative_path, file_attributes_stat, file_attributes_xattr), + relative_path_posix, + CharacterDevice( + relative_path_posix, file_attributes_stat, file_attributes_xattr + ), ) else: raise NotImplementedError( @@ -530,7 +547,7 @@ async def get_entry(self, path: str): :return: the descendant `FilesystemEntry`, if found; otherwise, returns `None` """ - basename = os.path.basename(path) + basename = posixpath.basename(path) # only searching paths with the same base name should reduce the search space by quite a lot descendants = await self.resource.get_descendants_as_view( FilesystemEntry, @@ -540,7 +557,7 @@ async def get_entry(self, path: str): ) for d in descendants: - if await d.get_path() == os.path.normpath(path): + if await d.get_path() == posixpath.normpath(path): return d return None @@ -581,7 +598,7 @@ async def add_folder( """ # Normalizes and cleans up paths beginning with "./" and containing "./../" as well as # other extraneous separators - split_dir = os.path.normpath(path).strip(os.path.sep).split(os.path.sep) + split_dir = posixpath.normpath(path).strip("/").split("/") parent: Union[FilesystemRoot, Folder] = self for directory in split_dir: @@ -633,8 +650,8 @@ async def add_file( :return: the `File` resource that was added to the `FilesystemRoot` """ - dirname = os.path.dirname(path) - filename = os.path.basename(path) + dirname = posixpath.dirname(path) + filename = posixpath.basename(path) if dirname == "": parent_folder = self @@ -687,7 +704,7 @@ async def add_special_file_entry( :return: The special `FilesystemEntry` resource that was added to the `FilesystemRoot` """ - dirname = os.path.dirname(path) + dirname = posixpath.dirname(path) if dirname == "": parent_folder = self From eb1436e8e73782f79c8b9b21016ec82f346289d8 Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Fri, 19 Jul 2024 16:16:29 +0000 Subject: [PATCH 05/14] Use tmp_path fixture for filesystem component tests --- .../components/test_filesystem_component.py | 112 ++++++++++-------- 1 file changed, 62 insertions(+), 50 deletions(-) diff --git a/ofrak_core/test_ofrak/components/test_filesystem_component.py b/ofrak_core/test_ofrak/components/test_filesystem_component.py index 44cfe9a21..8393383d2 100644 --- a/ofrak_core/test_ofrak/components/test_filesystem_component.py +++ b/ofrak_core/test_ofrak/components/test_filesystem_component.py @@ -30,47 +30,56 @@ DEVICE_NAME = "device" -class FilesystemRootDirectory(tempfile.TemporaryDirectory): +@pytest.fixture +def filesystem_root_directory(tmp_path) -> str: """ Create a root filesystem directory for testing """ - def __enter__(self): - temp_dir = self.name - child_folder = os.path.join(temp_dir, CHILD_FOLDER) - child_file = os.path.join(temp_dir, CHILD_TEXTFILE_NAME) + child_folder = os.path.join(tmp_path, CHILD_FOLDER) + child_file = os.path.join(tmp_path, CHILD_TEXTFILE_NAME) - subchild_file = os.path.join(child_folder, SUBCHILD_TEXTFILE_NAME) - subchild_folder = os.path.join(child_folder, SUBCHILD_FOLDER) + subchild_file = os.path.join(child_folder, SUBCHILD_TEXTFILE_NAME) + subchild_folder = os.path.join(child_folder, SUBCHILD_FOLDER) - if not os.path.exists(child_folder): - os.mkdir(child_folder) - if not os.path.exists(subchild_folder): - os.mkdir(subchild_folder) + if not os.path.exists(child_folder): + os.mkdir(child_folder) + if not os.path.exists(subchild_folder): + os.mkdir(subchild_folder) +<<<<<<< HEAD child_fifo = os.path.join(temp_dir, FIFO_PIPE_NAME) block_device = os.path.join(temp_dir, DEVICE_NAME) if not os.path.exists(child_fifo): os.mkfifo(child_fifo) +======= + if hasattr(os, "mkfifo"): + child_fifo = os.path.join(tmp_path, FIFO_PIPE_NAME) + if not os.path.exists(child_fifo): + os.mkfifo(child_fifo) + + if hasattr(os, "mkdev"): + block_device = os.path.join(tmp_path, DEVICE_NAME) +>>>>>>> 3c47373 (Use tmp_path fixture for filesystem component tests) if not os.path.exists(block_device): os.makedev(1, 2) - with open(child_file, "w") as f: - f.write(CHILD_TEXT) - with open(subchild_file, "w") as f: - f.write(SUBCHILD_TEXT) - return temp_dir + with open(child_file, "w") as f: + f.write(CHILD_TEXT) + with open(subchild_file, "w") as f: + f.write(SUBCHILD_TEXT) + + return str(tmp_path) @pytest.fixture -async def filesystem_root(ofrak_context: OFRAKContext) -> Resource: - with FilesystemRootDirectory() as temp_dir: - resource = await ofrak_context.create_root_resource( - name=temp_dir, data=b"", tags=[FilesystemRoot] - ) - filesystem_root = await resource.view_as(FilesystemRoot) - await filesystem_root.initialize_from_disk(temp_dir) - yield filesystem_root +async def filesystem_root(ofrak_context: OFRAKContext, filesystem_root_directory) -> Resource: + resource = await ofrak_context.create_root_resource( + name=filesystem_root_directory, data=b"", tags=[FilesystemRoot] + ) + filesystem_root = await resource.view_as(FilesystemRoot) + await filesystem_root.initialize_from_disk(filesystem_root_directory) + yield filesystem_root class TestFilesystemRoot: @@ -78,40 +87,40 @@ class TestFilesystemRoot: Test FilesystemRoot methods. """ - async def test_initialize_from_disk(self, ofrak_context: OFRAKContext): + async def test_initialize_from_disk( + self, ofrak_context: OFRAKContext, filesystem_root_directory + ): """ Test that FilesystemRoot.initialize_from_disk modifies a resources tree summary. """ - with FilesystemRootDirectory() as temp_dir: - resource = await ofrak_context.create_root_resource( - name=temp_dir, data=b"", tags=[FilesystemRoot] - ) - original_tree = await resource.summarize_tree() - filesystem_root = await resource.view_as(FilesystemRoot) - await filesystem_root.initialize_from_disk(temp_dir) - initialized_tree = await resource.summarize_tree() - assert original_tree != initialized_tree + resource = await ofrak_context.create_root_resource( + name=filesystem_root_directory, data=b"", tags=[FilesystemRoot] + ) + original_tree = await resource.summarize_tree() + filesystem_root = await resource.view_as(FilesystemRoot) + await filesystem_root.initialize_from_disk(filesystem_root_directory) + initialized_tree = await resource.summarize_tree() + assert original_tree != initialized_tree - async def test_flush_to_disk(self, ofrak_context: OFRAKContext): + async def test_flush_to_disk(self, ofrak_context: OFRAKContext, filesystem_root_directory): """ Test that FilesystemRoot.flush_to_disk correctly flushes the filesystem resources. """ - with FilesystemRootDirectory() as temp_dir: - resource = await ofrak_context.create_root_resource( - name=temp_dir, data=b"", tags=[FilesystemRoot] - ) - filesystem_root = await resource.view_as(FilesystemRoot) - await filesystem_root.initialize_from_disk(temp_dir) + resource = await ofrak_context.create_root_resource( + name=filesystem_root_directory, data=b"", tags=[FilesystemRoot] + ) + filesystem_root = await resource.view_as(FilesystemRoot) + await filesystem_root.initialize_from_disk(filesystem_root_directory) - with tempfile.TemporaryDirectory() as flush_dir: - await filesystem_root.flush_to_disk(flush_dir) + with tempfile.TemporaryDirectory() as flush_dir: + await filesystem_root.flush_to_disk(flush_dir) - if sys.platform != "win32": - diff_directories(temp_dir, flush_dir, extra_diff_flags="") - else: - warnings.warn( - "Directories not compared on Windows. TODO: implement a basic comparison" - ) + if sys.platform != "win32": + diff_directories(filesystem_root_directory, flush_dir, extra_diff_flags="") + else: + warnings.warn( + "Directories not compared on Windows. TODO: implement a basic comparison" + ) async def test_get_entry(self, filesystem_root: FilesystemRoot): """ @@ -125,7 +134,10 @@ async def test_list_dir(self, filesystem_root: FilesystemRoot): Test that FilesystemRoot.list_dir returns the expected directory contents. """ list_dir_output = await filesystem_root.list_dir() - assert set(list_dir_output.keys()) == {FIFO_PIPE_NAME, CHILD_FOLDER, CHILD_TEXTFILE_NAME} + expected = {CHILD_FOLDER, CHILD_TEXTFILE_NAME} + if sys.platform != "win32": + expected.add(FIFO_PIPE_NAME) + assert set(list_dir_output.keys()) == expected async def test_add_folder(self, filesystem_root: FilesystemRoot, tmp_path): """ From 9f04d8bdae64cdc720e5b757436fd68dec4faa8c Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Fri, 19 Jul 2024 16:22:57 +0000 Subject: [PATCH 06/14] Use tmp_path fixture for FilesystemPackUnpackVerifyPattern --- .../patterns/pack_unpack_filesystem.py | 22 ++++++++++--------- .../components/test_filesystem_component.py | 7 ------ 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/ofrak_core/pytest_ofrak/patterns/pack_unpack_filesystem.py b/ofrak_core/pytest_ofrak/patterns/pack_unpack_filesystem.py index 36bb39d8d..d1bed54c1 100644 --- a/ofrak_core/pytest_ofrak/patterns/pack_unpack_filesystem.py +++ b/ofrak_core/pytest_ofrak/patterns/pack_unpack_filesystem.py @@ -1,6 +1,5 @@ import os import stat -import tempfile from abc import ABC, abstractmethod from subprocess import CalledProcessError @@ -28,17 +27,20 @@ def setup(self): self.check_xattrs = True self.check_stat = True - async def test_pack_unpack_verify(self, ofrak_context: OFRAKContext): + async def test_pack_unpack_verify(self, ofrak_context: OFRAKContext, tmp_path): + root_path = tmp_path / "root" + root_path.mkdir() + extract_dir = tmp_path / "extract" + extract_dir.mkdir() + try: self.setup() - with tempfile.TemporaryDirectory() as root_path: - self.create_local_file_structure(root_path) - root_resource = await self.create_root_resource(ofrak_context, root_path) - await self.unpack(root_resource) - await self.repack(root_resource) - with tempfile.TemporaryDirectory() as extract_dir: - await self.extract(root_resource, extract_dir) - self.verify_filesystem_equality(root_path, extract_dir) + self.create_local_file_structure(root_path) + root_resource = await self.create_root_resource(ofrak_context, root_path) + await self.unpack(root_resource) + await self.repack(root_resource) + await self.extract(root_resource, extract_dir) + self.verify_filesystem_equality(root_path, extract_dir) except CalledProcessError as e: # Better printing of errors if something goes wrong in test setup/execution raise ComponentSubprocessError(e) diff --git a/ofrak_core/test_ofrak/components/test_filesystem_component.py b/ofrak_core/test_ofrak/components/test_filesystem_component.py index 8393383d2..2dc2b088e 100644 --- a/ofrak_core/test_ofrak/components/test_filesystem_component.py +++ b/ofrak_core/test_ofrak/components/test_filesystem_component.py @@ -47,12 +47,6 @@ def filesystem_root_directory(tmp_path) -> str: if not os.path.exists(subchild_folder): os.mkdir(subchild_folder) -<<<<<<< HEAD - child_fifo = os.path.join(temp_dir, FIFO_PIPE_NAME) - block_device = os.path.join(temp_dir, DEVICE_NAME) - if not os.path.exists(child_fifo): - os.mkfifo(child_fifo) -======= if hasattr(os, "mkfifo"): child_fifo = os.path.join(tmp_path, FIFO_PIPE_NAME) if not os.path.exists(child_fifo): @@ -60,7 +54,6 @@ def filesystem_root_directory(tmp_path) -> str: if hasattr(os, "mkdev"): block_device = os.path.join(tmp_path, DEVICE_NAME) ->>>>>>> 3c47373 (Use tmp_path fixture for filesystem component tests) if not os.path.exists(block_device): os.makedev(1, 2) From 5f1ebddefb9fce89578f7bb16e6fb874747d80cf Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Fri, 19 Jul 2024 17:29:53 +0000 Subject: [PATCH 07/14] Use posixpath instead of os.path in dtb.py --- ofrak_core/ofrak/core/dtb.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ofrak_core/ofrak/core/dtb.py b/ofrak_core/ofrak/core/dtb.py index 9df32fe59..aa02549e3 100644 --- a/ofrak_core/ofrak/core/dtb.py +++ b/ofrak_core/ofrak/core/dtb.py @@ -3,7 +3,7 @@ For more information see: https://devicetree-specification.readthedocs.io/en/stable/flattened-format.html """ -import os +import posixpath import struct from dataclasses import dataclass from enum import Enum @@ -55,7 +55,7 @@ async def get_path(self) -> str: return self.name parent_node = await self.resource.get_parent_as_view(v_type=DtbNode) - return os.path.join(await parent_node.get_path(), self.name) + return posixpath.join(await parent_node.get_path(), self.name) class DeviceTreeBlob(GenericBinary): @@ -183,7 +183,7 @@ def caption(cls, attributes) -> str: async def get_path(self): parent_node = await self.resource.get_parent_as_view(v_type=DtbNode) - return os.path.join(await parent_node.get_path(), self.name) + return posixpath.join(await parent_node.get_path(), self.name) async def get_value(self) -> Union[str, List[str], int, List[int], bytes, bytearray, None]: if self.p_type is DtbPropertyType.DtbPropNoValue: @@ -321,7 +321,7 @@ async def pack(self, resource: Resource, config: ComponentConfig = None): ): # By default, add_item adds the missing nodes to complete the path of a previous node if not dtb.exist_node(await node.get_path()): - dtb.add_item(fdt.Node(node.name), os.path.dirname(await node.get_path())) + dtb.add_item(fdt.Node(node.name), posixpath.dirname(await node.get_path())) for prop in await node.resource.get_children_as_view( v_type=DtbProperty, r_filter=ResourceFilter(tags=[DtbProperty]), From ff915f97f0360146024c82e7424eeb011439a1b4 Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Fri, 19 Jul 2024 18:03:30 +0000 Subject: [PATCH 08/14] use os.sep instead of / in tar check --- ofrak_core/ofrak/core/tar.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ofrak_core/ofrak/core/tar.py b/ofrak_core/ofrak/core/tar.py index 9c1a7b397..fabe7f876 100644 --- a/ofrak_core/ofrak/core/tar.py +++ b/ofrak_core/ofrak/core/tar.py @@ -59,7 +59,7 @@ async def unpack(self, resource: Resource, config: ComponentConfig = None) -> No for filename in stdout.decode().splitlines(): # Handles relative parent paths and rooted paths, and normalizes paths like "./../" rel_filename = os.path.relpath(filename) - if rel_filename.startswith("../"): + if rel_filename.startswith(".." + os.sep): raise UnpackerError( f"Tar archive contains a file {filename} that would extract to a parent " f"directory {rel_filename}." From 5fb8e64f628eb1eeee4d579910a2ff0014acaf19 Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Wed, 20 Nov 2024 19:50:31 +0000 Subject: [PATCH 09/14] Use posixpath instead of os.path in iso9660.py --- ofrak_core/ofrak/core/iso9660.py | 6 +++--- ofrak_core/test_ofrak/components/test_iso_component.py | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ofrak_core/ofrak/core/iso9660.py b/ofrak_core/ofrak/core/iso9660.py index c407c6ab1..dcbb05113 100644 --- a/ofrak_core/ofrak/core/iso9660.py +++ b/ofrak_core/ofrak/core/iso9660.py @@ -1,6 +1,6 @@ import asyncio import logging -import os +import posixpath import tempfile from dataclasses import dataclass from io import BytesIO @@ -195,7 +195,7 @@ async def unpack(self, resource: Resource, config=None): for root, dirs, files in iso.walk(**{path_var: "/"}): for d in dirs: - path = os.path.join(root, d) + path = posixpath.join(root, d) folder_tags = (ISO9660Entry, Folder) entry = ISO9660Entry( name=d, @@ -211,7 +211,7 @@ async def unpack(self, resource: Resource, config=None): path, None, None, folder_tags, entry.get_attributes_instances().values() ) for f in files: - path = os.path.join(root, f) + path = posixpath.join(root, f) file_tags = (ISO9660Entry, File) fp = BytesIO() diff --git a/ofrak_core/test_ofrak/components/test_iso_component.py b/ofrak_core/test_ofrak/components/test_iso_component.py index e385b793a..834353253 100644 --- a/ofrak_core/test_ofrak/components/test_iso_component.py +++ b/ofrak_core/test_ofrak/components/test_iso_component.py @@ -1,5 +1,6 @@ import os from io import BytesIO +import posixpath from typing import Optional import pytest @@ -20,7 +21,7 @@ class Iso9660UnpackModifyPackPattern(CompressedFileUnpackModifyPackPattern): TEST_ISO_NAME = "test.iso" TEST_DIR_NAME = "/TEST" - TEST_FILE_NAME = os.path.join(TEST_DIR_NAME, "TEST.TXT") + TEST_FILE_NAME = posixpath.join(TEST_DIR_NAME, "TEST.TXT") TEST_SYS_ID = "TestSysID" TEST_VOL_ID = "TestVolID" TEST_APP_ID = "Test Application" From dc6f707ec59b4a0e5c09ec2066fa7374ec8fa326 Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Wed, 20 Nov 2024 19:54:21 +0000 Subject: [PATCH 10/14] Remove duplicated function definition (rebase artifact) --- ofrak_core/ofrak/core/filesystem.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/ofrak_core/ofrak/core/filesystem.py b/ofrak_core/ofrak/core/filesystem.py index fb721a9f0..c31e80558 100644 --- a/ofrak_core/ofrak/core/filesystem.py +++ b/ofrak_core/ofrak/core/filesystem.py @@ -26,14 +26,6 @@ ) -def _warn_chmod_chown_windows(): - warnings.warn( - f"os.chown and os.chmod do not work on Windows platforms. \ - Unix-like file ownership and permissions will not be properly handled while using OFRAK on this platform. \ - If you require extended attributes, please use a different platform." - ) - - def _warn_chmod_chown_windows(): warnings.warn( f"os.chown and os.chmod do not work on Windows platforms. \ From 7073ce9b4e7539b52e4fe99dda210a54734eebd9 Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Wed, 20 Nov 2024 20:36:01 +0000 Subject: [PATCH 11/14] Add #pragma: no cover to warning function --- ofrak_core/ofrak/core/filesystem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ofrak_core/ofrak/core/filesystem.py b/ofrak_core/ofrak/core/filesystem.py index c31e80558..604c7758a 100644 --- a/ofrak_core/ofrak/core/filesystem.py +++ b/ofrak_core/ofrak/core/filesystem.py @@ -26,7 +26,7 @@ ) -def _warn_chmod_chown_windows(): +def _warn_chmod_chown_windows(): # pragma: no cover warnings.warn( f"os.chown and os.chmod do not work on Windows platforms. \ Unix-like file ownership and permissions will not be properly handled while using OFRAK on this platform. \ From f0b4200897be90298912b6e7d15bbbc627587ca1 Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Wed, 4 Dec 2024 20:11:49 +0000 Subject: [PATCH 12/14] Change docstring in get_path --- ofrak_core/ofrak/core/filesystem.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ofrak_core/ofrak/core/filesystem.py b/ofrak_core/ofrak/core/filesystem.py index 604c7758a..d5857ef0e 100644 --- a/ofrak_core/ofrak/core/filesystem.py +++ b/ofrak_core/ofrak/core/filesystem.py @@ -120,8 +120,8 @@ async def get_path(self) -> str: """ Get a folder's path, with the `FilesystemRoot` as the path root. - :return: The full path name, with the `FilesystemRoot` ancestor as the path root - Always returns POSIX style paths with forward slashes. + :return: The full path name, with the `FilesystemRoot` ancestor as the path root; + always POSIX style paths with forward slashes """ path = [self.get_name()] From 2c4babffea9c56d25ab47b52ba269d67860beb56 Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Wed, 4 Dec 2024 20:27:31 +0000 Subject: [PATCH 13/14] Respond to PR review --- ofrak_core/ofrak/core/filesystem.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/ofrak_core/ofrak/core/filesystem.py b/ofrak_core/ofrak/core/filesystem.py index d5857ef0e..a6849b609 100644 --- a/ofrak_core/ofrak/core/filesystem.py +++ b/ofrak_core/ofrak/core/filesystem.py @@ -27,10 +27,10 @@ def _warn_chmod_chown_windows(): # pragma: no cover + # warnings.warn instead of logging.warning prevents duplicating this static warning message warnings.warn( - f"os.chown and os.chmod do not work on Windows platforms. \ - Unix-like file ownership and permissions will not be properly handled while using OFRAK on this platform. \ - If you require extended attributes, please use a different platform." + "os.chown and os.chmod do not work on Windows platforms. Unix-like file ownership and " + "permissions will not be properly handled while using OFRAK on this platform." ) @@ -226,7 +226,6 @@ async def flush_to_disk(self, root_path: str = ".", filename: Optional[str] = No ) with open(device_name, "w") as f: pass - self.apply_stat_attrs(device_name) else: if self.stat is None: raise ValueError( @@ -234,7 +233,8 @@ async def flush_to_disk(self, root_path: str = ".", filename: Optional[str] = No f"BlockDevice or CharacterDevice resource with no stat!" ) os.mknod(device_name, self.stat.st_mode, self.stat.st_rdev) - self.apply_stat_attrs(device_name) + + self.apply_stat_attrs(device_name) elif self.is_fifo_pipe(): fifo_name = os.path.join(root_path, entry_path) if sys.platform == "win32" or not hasattr(os, "mkfifo"): @@ -245,7 +245,6 @@ async def flush_to_disk(self, root_path: str = ".", filename: Optional[str] = No ) with open(fifo_name, "w") as f: pass - self.apply_stat_attrs(fifo_name) else: if self.stat is None: raise ValueError( @@ -253,7 +252,8 @@ async def flush_to_disk(self, root_path: str = ".", filename: Optional[str] = No "with no stat!" ) os.mkfifo(fifo_name, self.stat.st_mode) - self.apply_stat_attrs(fifo_name) + + self.apply_stat_attrs(fifo_name) else: entry_info = f"Stat: {stat.S_IFMT(self.stat.st_mode):o}" if self.stat else "" raise NotImplementedError( From 316732ac65a0fe6ba5010dc9c0bd3c9a3f594a76 Mon Sep 17 00:00:00 2001 From: Albert Zhang Date: Wed, 4 Dec 2024 22:58:26 +0000 Subject: [PATCH 14/14] Add changelog entry --- ofrak_core/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ofrak_core/CHANGELOG.md b/ofrak_core/CHANGELOG.md index 854ec4d02..8f564fb7a 100644 --- a/ofrak_core/CHANGELOG.md +++ b/ofrak_core/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Add UEFI binary unpacker. ([#399](https://github.com/redballoonsecurity/ofrak/pull/399)) - Add recursive identify functionality in the GUI. ([#435](https://github.com/redballoonsecurity/ofrak/pull/435)) - Add generic DecompilationAnalysis classes. ([#453](https://github.com/redballoonsecurity/ofrak/pull/453)) +- Add support for running on Windows to the `Filesystem` component. ([#521](https://github.com/redballoonsecurity/ofrak/pull/521)) ### Fixed - Improved flushing of filesystem entries (including symbolic links and other types) to disk. ([#373](https://github.com/redballoonsecurity/ofrak/pull/373)) @@ -27,6 +28,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Fix bug in OFRAK GUI server which causes an error when parsing a default config value of bytes. ([#409](https://github.com/redballoonsecurity/ofrak/pull/409)) - Set default fallback font to system default monospace, instead of variable-width sans-serif. ([#422](https://github.com/redballoonsecurity/ofrak/pull/422)) - View resource attribute string values containing only digits primarily as strings, alternatively as hex numbers. ([#423](https://github.com/redballoonsecurity/ofrak/pull/423)) +- Fix bugs on Windows arising from using `os.path` methods when only forward-slashes are acceptable ([#521](https://github.com/redballoonsecurity/ofrak/pull/521)) ### Changed - By default, the ofrak log is now `ofrak-YYYYMMDDhhmmss.log` rather than just `ofrak.log` and the name can be specified on the command line ([#480](https://github.com/redballoonsecurity/ofrak/pull/480))