mirror of
https://github.com/arsenetar/send2trash.git
synced 2026-06-19 13:37:53 +00:00
fix: Correct is_parent() path handling
Prevent incorrect path handling in is_parent() to ensure that only actual parent directories are matched. Before performing the substring match it now ensures that the parent path ends with a separator, preventing incorrect matches where a directory name is a substring of another directory name. - Corrected matching logic - Added explicit test for this logic and verification e2e - Fix one use of is_parent() in tests that was relying on partially incorrect behavior
This commit is contained in:
@@ -46,6 +46,9 @@ def is_parent(parent, path):
|
|||||||
parent = op.realpath(parent)
|
parent = op.realpath(parent)
|
||||||
if isinstance(parent, str):
|
if isinstance(parent, str):
|
||||||
parent = os.fsencode(parent)
|
parent = os.fsencode(parent)
|
||||||
|
# Ensure parent ends with a separator to prevent invalid substring matches
|
||||||
|
if not parent.endswith(b"/"):
|
||||||
|
parent += b"/"
|
||||||
return path.startswith(parent)
|
return path.startswith(parent)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -99,7 +99,12 @@ class ExtVol:
|
|||||||
|
|
||||||
def s_getdev(path):
|
def s_getdev(path):
|
||||||
st = os.lstat(path)
|
st = os.lstat(path)
|
||||||
if is_parent(self.trash_topdir, path):
|
path_real = op.realpath(path)
|
||||||
|
if isinstance(path_real, bytes):
|
||||||
|
topdir_real = os.fsencode(op.realpath(self.trash_topdir))
|
||||||
|
else:
|
||||||
|
topdir_real = op.realpath(self.trash_topdir)
|
||||||
|
if path_real == topdir_real or is_parent(self.trash_topdir, path):
|
||||||
return "dev"
|
return "dev"
|
||||||
return st.st_dev
|
return st.st_dev
|
||||||
|
|
||||||
@@ -207,3 +212,65 @@ def test_trash_symlink(gen_ext_vol):
|
|||||||
is True
|
is True
|
||||||
)
|
)
|
||||||
os.remove(sl_dir)
|
os.remove(sl_dir)
|
||||||
|
|
||||||
|
|
||||||
|
def test_is_parent_substring_path_bug_e2e():
|
||||||
|
"""
|
||||||
|
End-to-end test that is_parent() substring bug doesn't affect trashing.
|
||||||
|
|
||||||
|
This test prevents a bug where paths like ~/.local/share (HOMETRASH) would be
|
||||||
|
incorrectly considered as the parent of ~/.local/shared due to simple substring
|
||||||
|
matching without checking for path separators. This would cause incorrect relative
|
||||||
|
path computation in the trash info file (should be absolute path since the file
|
||||||
|
is not actually under ~/.local/share).
|
||||||
|
"""
|
||||||
|
# Create a sibling directory to HOMETRASH with a similar name
|
||||||
|
shared_dir = op.expanduser("~/.local/shared")
|
||||||
|
os.makedirs(shared_dir, exist_ok=True)
|
||||||
|
|
||||||
|
try:
|
||||||
|
# Create a test file in the shared directory
|
||||||
|
test_file = op.join(shared_dir, "test_substring_bug.txt")
|
||||||
|
touch(test_file)
|
||||||
|
assert op.exists(test_file) is True
|
||||||
|
|
||||||
|
# Trash the file
|
||||||
|
s2t(test_file)
|
||||||
|
|
||||||
|
# File should be removed from original location
|
||||||
|
assert op.exists(test_file) is False
|
||||||
|
|
||||||
|
# Find the trash info file for this test file
|
||||||
|
trash_info_dir = op.join(HOMETRASH, "info")
|
||||||
|
trash_info_files = [f for f in os.listdir(trash_info_dir) if "substring_bug" in f]
|
||||||
|
assert len(trash_info_files) > 0, "No trash info file found for test file"
|
||||||
|
|
||||||
|
# Read the trash info file and verify the path is absolute
|
||||||
|
info_file_path = op.join(trash_info_dir, trash_info_files[0])
|
||||||
|
cfg = ConfigParser()
|
||||||
|
cfg.read(info_file_path)
|
||||||
|
trashed_path = cfg.get("Trash Info", "Path", raw=True)
|
||||||
|
|
||||||
|
# The path should be absolute, not relative
|
||||||
|
# If the bug exists, it might be a relative path like "shared/test_substring_bug.txt"
|
||||||
|
# instead of the full absolute path
|
||||||
|
assert op.isabs(trashed_path) or trashed_path.startswith("/"), (
|
||||||
|
f"Path in trash info should be absolute, got: {trashed_path}. "
|
||||||
|
"This suggests is_parent() is incorrectly treating ~/.local/share as parent."
|
||||||
|
)
|
||||||
|
|
||||||
|
finally:
|
||||||
|
# Cleanup
|
||||||
|
if op.exists(shared_dir):
|
||||||
|
shutil.rmtree(shared_dir)
|
||||||
|
# Clean up any trash files created by this test
|
||||||
|
trash_files_dir = op.join(HOMETRASH, "files")
|
||||||
|
trash_info_dir = op.join(HOMETRASH, "info")
|
||||||
|
if op.exists(trash_files_dir):
|
||||||
|
for f in os.listdir(trash_files_dir):
|
||||||
|
if "substring_bug" in f:
|
||||||
|
os.remove(op.join(trash_files_dir, f))
|
||||||
|
if op.exists(trash_info_dir):
|
||||||
|
for f in os.listdir(trash_info_dir):
|
||||||
|
if "substring_bug" in f:
|
||||||
|
os.remove(op.join(trash_info_dir, f))
|
||||||
|
|||||||
Reference in New Issue
Block a user