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

@@ -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
 
 

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)

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":

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")