From 8ce6c26f9c7ef39b1427b3f924f63f9e1591ddcb Mon Sep 17 00:00:00 2001 From: Safihre Date: Sat, 13 Jun 2020 20:50:34 +0200 Subject: [PATCH] Do not scan incomplete directory recursively Also renamed the recursive_listdir function so it can also be non-recursive. Which was very easy due to the extensive test-set by @jcfp! Relates to #1304 (that NZB triggered invalid reading of sub-dirs in incomplete folder) --- sabnzbd/filesystem.py | 4 ++- sabnzbd/newsunpack.py | 14 +++++----- sabnzbd/postproc.py | 4 +-- tests/test_filesystem.py | 72 +++++++++++++++++++++++++++++++----------------- 4 files changed, 58 insertions(+), 36 deletions(-) diff --git a/sabnzbd/filesystem.py b/sabnzbd/filesystem.py index e205e90..2043171 100644 --- a/sabnzbd/filesystem.py +++ b/sabnzbd/filesystem.py @@ -605,7 +605,7 @@ def get_unique_filename(path): @synchronized(DIR_LOCK) -def recursive_listdir(input_dir): +def listdir_full(input_dir, recursive=True): """ List all files in dirs and sub-dirs """ filelist = [] for root, dirs, files in os.walk(input_dir): @@ -613,6 +613,8 @@ def recursive_listdir(input_dir): if ".AppleDouble" not in root and ".DS_Store" not in root: p = os.path.join(root, file) filelist.append(p) + if not recursive: + break return filelist diff --git a/sabnzbd/newsunpack.py b/sabnzbd/newsunpack.py index 55c911a..100a2dd 100644 --- a/sabnzbd/newsunpack.py +++ b/sabnzbd/newsunpack.py @@ -43,7 +43,7 @@ from sabnzbd.filesystem import ( clip_path, long_path, remove_file, - recursive_listdir, + listdir_full, setname_from_path, get_ext, get_filename, @@ -931,7 +931,7 @@ def unzip(nzo, workdir, workdir_complete, delete, one_folder, zips): tms = time.time() # For file-bookkeeping - orig_dir_content = recursive_listdir(workdir_complete) + orig_dir_content = listdir_full(workdir_complete) for _zip in zips: logging.info("Starting extract on zipfile: %s ", _zip) @@ -951,7 +951,7 @@ def unzip(nzo, workdir, workdir_complete, delete, one_folder, zips): nzo.set_unpack_info("Unpack", msg) # What's new? - new_files = list(set(orig_dir_content + recursive_listdir(workdir_complete))) + new_files = list(set(orig_dir_content + listdir_full(workdir_complete))) # Delete the old files if we have to if delete and not unzip_failed: @@ -1124,7 +1124,7 @@ def seven_extract_core(sevenset, extensions, extraction_path, one_folder, delete return 1, [], T('7ZIP set "%s" is incomplete, cannot unpack') % setname_from_path(sevenset) # For file-bookkeeping - orig_dir_content = recursive_listdir(extraction_path) + orig_dir_content = listdir_full(extraction_path) command = [SEVEN_COMMAND, method, "-y", overwrite, parm, case, password, "-o%s" % extraction_path, name] @@ -1155,7 +1155,7 @@ def seven_extract_core(sevenset, extensions, extraction_path, one_folder, delete msg = T("Could not unpack %s") % setname_from_path(sevenset) # What's new? - new_files = list(set(orig_dir_content + recursive_listdir(extraction_path))) + new_files = list(set(orig_dir_content + listdir_full(extraction_path))) if ret == 0 and delete: if extensions: @@ -2207,10 +2207,10 @@ def build_filelists(workdir, workdir_complete=None, check_both=False, check_rar= sevens, joinables, zips, rars, ts, filelist = ([], [], [], [], [], []) if workdir_complete: - filelist.extend(recursive_listdir(workdir_complete)) + filelist.extend(listdir_full(workdir_complete)) if workdir and (not filelist or check_both): - filelist.extend(recursive_listdir(workdir)) + filelist.extend(listdir_full(workdir, recursive=False)) for file in filelist: # Extra check for rar (takes CPU/disk) diff --git a/sabnzbd/postproc.py b/sabnzbd/postproc.py index ab872f6..51f414f 100644 --- a/sabnzbd/postproc.py +++ b/sabnzbd/postproc.py @@ -55,7 +55,7 @@ from sabnzbd.filesystem import ( sanitize_and_trim_path, sanitize_files_in_folder, remove_file, - recursive_listdir, + listdir_full, setname_from_path, create_all_dirs, get_unique_filename, @@ -1038,7 +1038,7 @@ def nzb_redirect(wdir, nzbname, pp, script, cat, priority): if so send to queue and remove if on clean-up list Returns list of processed NZB's """ - files = recursive_listdir(wdir) + files = listdir_full(wdir) for nzb_file in files: if get_ext(nzb_file) != ".nzb": diff --git a/tests/test_filesystem.py b/tests/test_filesystem.py index f0dd060..5745b3c 100644 --- a/tests/test_filesystem.py +++ b/tests/test_filesystem.py @@ -460,7 +460,7 @@ class TestTrimWinPath: @pytest.mark.skipif(sys.platform.startswith("win"), reason="Broken on Windows") -class TestRecursiveListdir(ffs.TestCase): +class TestListdirFull(ffs.TestCase): # Basic fake filesystem setup stanza def setUp(self): self.setUpPyfakefs() @@ -468,7 +468,7 @@ class TestRecursiveListdir(ffs.TestCase): self.fs.is_case_sensitive = True def test_nonexistent_dir(self): - assert filesystem.recursive_listdir("/foo/bar") == [] + assert filesystem.listdir_full("/foo/bar") == [] def test_no_exceptions(self): test_files = ( @@ -480,30 +480,38 @@ class TestRecursiveListdir(ffs.TestCase): self.fs.create_file(file) assert os.path.exists(file) is True # List our fake directory structure - results_subdir = filesystem.recursive_listdir("/test/dir") + results_subdir = filesystem.listdir_full("/test/dir") assert len(results_subdir) == 3 for entry in test_files: assert (entry in results_subdir) is True # List the same directory again, this time using its parent as the function argument. # Results should be identical, since there's nothing in /test but that one subdirectory - results_parent = filesystem.recursive_listdir("/test") + results_parent = filesystem.listdir_full("/test") # Don't make assumptions about the sorting of the lists of results results_parent.sort() results_subdir.sort() assert results_parent == results_subdir # List that subsubsub-directory; no sorting required for a single result - assert filesystem.recursive_listdir("/test/dir/sub/sub") == ["/test/dir/sub/sub/sub/dir/file3.ext"] + assert filesystem.listdir_full("/test/dir/sub/sub") == ["/test/dir/sub/sub/sub/dir/file3.ext"] + + # Test non-recursive version + assert filesystem.listdir_full(r"/test", recursive=False) == [] + assert filesystem.listdir_full(r"/test/dir/sub", recursive=False) == [] + assert len(filesystem.listdir_full(r"/test/dir", recursive=False)) == 2 def test_exception_appledouble(self): # Anything below a .AppleDouble directory should be omitted test_file = "/foo/bar/.AppleDouble/Oooooo.ps" self.fs.create_file(test_file) assert os.path.exists(test_file) is True - assert filesystem.recursive_listdir("/foo") == [] - assert filesystem.recursive_listdir("/foo/bar") == [] - assert filesystem.recursive_listdir("/foo/bar/.AppleDouble") == [] + assert filesystem.listdir_full("/foo") == [] + assert filesystem.listdir_full("/foo/bar") == [] + assert filesystem.listdir_full("/foo/bar/.AppleDouble") == [] + assert filesystem.listdir_full("/foo", recursive=False) == [] + assert filesystem.listdir_full("/foo/bar", recursive=False) == [] + assert filesystem.listdir_full("/foo/bar/.AppleDouble", recursive=False) == [] def test_exception_dsstore(self): # Anything below a .DS_Store directory should be omitted @@ -514,9 +522,12 @@ class TestRecursiveListdir(ffs.TestCase): ): self.fs.create_file(file) assert os.path.exists(file) is True - assert filesystem.recursive_listdir("/some") == ["/some/FILE"] - assert filesystem.recursive_listdir("/some/.DS_Store/") == [] - assert filesystem.recursive_listdir("/some/.DS_Store/subdir") == [] + assert filesystem.listdir_full("/some") == ["/some/FILE"] + assert filesystem.listdir_full("/some/.DS_Store/") == [] + assert filesystem.listdir_full("/some/.DS_Store/subdir") == [] + assert filesystem.listdir_full("/some", recursive=False) == ["/some/FILE"] + assert filesystem.listdir_full("/some/.DS_Store/", recursive=False) == [] + assert filesystem.listdir_full("/some/.DS_Store/subdir", recursive=False) == [] def test_invalid_file_argument(self): # This is obviously not intended use; the function expects a directory @@ -524,10 +535,10 @@ class TestRecursiveListdir(ffs.TestCase): test_file = "/dev/sleepy" self.fs.create_file(test_file) assert os.path.exists(test_file) is True - assert filesystem.recursive_listdir(test_file) == [] + assert filesystem.listdir_full(test_file) == [] -class TestRecursiveListdirWin(ffs.TestCase): +class TestListdirFullWin(ffs.TestCase): # Basic fake filesystem setup stanza @set_platform("win32") def setUp(self): @@ -537,7 +548,7 @@ class TestRecursiveListdirWin(ffs.TestCase): self.fs.is_case_sensitive = False def test_nonexistent_dir(self): - assert filesystem.recursive_listdir(r"F:\foo\bar") == [] + assert filesystem.listdir_full(r"F:\foo\bar") == [] def test_no_exceptions(self): test_files = ( @@ -549,32 +560,38 @@ class TestRecursiveListdirWin(ffs.TestCase): self.fs.create_file(file) assert os.path.exists(file) is True # List our fake directory structure - results_subdir = filesystem.recursive_listdir(r"f:\test\dir") + results_subdir = filesystem.listdir_full(r"f:\test\dir") assert len(results_subdir) == 3 for entry in test_files: assert (entry in results_subdir) is True # List the same directory again, this time using its parent as the function argument. # Results should be identical, since there's nothing in /test but that one subdirectory - results_parent = filesystem.recursive_listdir(r"f:\test") + results_parent = filesystem.listdir_full(r"f:\test") # Don't make assumptions about the sorting of the lists of results results_parent.sort() results_subdir.sort() assert results_parent == results_subdir # List that subsubsub-directory; no sorting required for a single result - assert ( - filesystem.recursive_listdir(r"F:\test\dir\SUB\sub")[0].lower() == r"f:\test\dir\sub\sub\sub\dir\file3.ext" - ) + assert filesystem.listdir_full(r"F:\test\dir\SUB\sub")[0].lower() == r"f:\test\dir\sub\sub\sub\dir\file3.ext" + + # Test non-recursive version + assert filesystem.listdir_full(r"f:\test", recursive=False) == [] + assert filesystem.listdir_full(r"F:\test\dir\SUB", recursive=False) == [] + assert len(filesystem.listdir_full(r"f:\test\dir", recursive=False)) == 2 def test_exception_appledouble(self): # Anything below a .AppleDouble directory should be omitted test_file = r"f:\foo\bar\.AppleDouble\Oooooo.ps" self.fs.create_file(test_file) assert os.path.exists(test_file) is True - assert filesystem.recursive_listdir(r"f:\foo") == [] - assert filesystem.recursive_listdir(r"f:\foo\bar") == [] - assert filesystem.recursive_listdir(r"F:\foo\bar\.AppleDouble") == [] + assert filesystem.listdir_full(r"f:\foo") == [] + assert filesystem.listdir_full(r"f:\foo\bar") == [] + assert filesystem.listdir_full(r"F:\foo\bar\.AppleDouble") == [] + assert filesystem.listdir_full(r"f:\foo", recursive=False) == [] + assert filesystem.listdir_full(r"f:\foo\bar", recursive=False) == [] + assert filesystem.listdir_full(r"F:\foo\bar\.AppleDouble", recursive=False) == [] def test_exception_dsstore(self): # Anything below a .DS_Store directory should be omitted @@ -585,9 +602,12 @@ class TestRecursiveListdirWin(ffs.TestCase): ): self.fs.create_file(file) assert os.path.exists(file) is True - assert filesystem.recursive_listdir(r"f:\some") == [r"f:\some\FILE"] - assert filesystem.recursive_listdir(r"f:\some\.DS_Store") == [] - assert filesystem.recursive_listdir(r"f:\some\.DS_Store\subdir") == [] + assert filesystem.listdir_full(r"f:\some") == [r"f:\some\FILE"] + assert filesystem.listdir_full(r"f:\some\.DS_Store") == [] + assert filesystem.listdir_full(r"f:\some\.DS_Store\subdir") == [] + assert filesystem.listdir_full(r"f:\some", recursive=True) == [r"f:\some\FILE"] + assert filesystem.listdir_full(r"f:\some\.DS_Store", recursive=True) == [] + assert filesystem.listdir_full(r"f:\some\.DS_Store\subdir", recursive=True) == [] def test_invalid_file_argument(self): # This is obviously not intended use; the function expects a directory @@ -595,7 +615,7 @@ class TestRecursiveListdirWin(ffs.TestCase): test_file = r"f:\dev\sleepy" self.fs.create_file(test_file) assert os.path.exists(test_file) is True - assert filesystem.recursive_listdir(test_file) == [] + assert filesystem.listdir_full(test_file) == [] @pytest.mark.skipif(sys.platform.startswith("win"), reason="Broken on Windows")