diff --git a/send2trash/plat_other.py b/send2trash/plat_other.py index 6f80275..9c774e4 100644 --- a/send2trash/plat_other.py +++ b/send2trash/plat_other.py @@ -46,6 +46,9 @@ def is_parent(parent, path): parent = op.realpath(parent) if isinstance(parent, str): 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) diff --git a/tests/test_plat_other.py b/tests/test_plat_other.py index e451175..ea69673 100644 --- a/tests/test_plat_other.py +++ b/tests/test_plat_other.py @@ -99,7 +99,12 @@ class ExtVol: def s_getdev(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 st.st_dev @@ -207,3 +212,65 @@ def test_trash_symlink(gen_ext_vol): is True ) 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))