Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.12] gh-116401: Fix blocking os.fwalk() and shutil.rmtree() on opening a named pipe (GH-116421) #116716

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Lib/os.py
Expand Up @@ -473,7 +473,7 @@ def fwalk(top=".", topdown=True, onerror=None, *, follow_symlinks=False, dir_fd=
# lstat()/open()/fstat() trick.
if not follow_symlinks:
orig_st = stat(top, follow_symlinks=False, dir_fd=dir_fd)
topfd = open(top, O_RDONLY, dir_fd=dir_fd)
topfd = open(top, O_RDONLY | O_NONBLOCK, dir_fd=dir_fd)
try:
if (follow_symlinks or (st.S_ISDIR(orig_st.st_mode) and
path.samestat(orig_st, stat(topfd)))):
Expand Down Expand Up @@ -522,7 +522,7 @@ def _fwalk(topfd, toppath, isbytes, topdown, onerror, follow_symlinks):
assert entries is not None
name, entry = name
orig_st = entry.stat(follow_symlinks=False)
dirfd = open(name, O_RDONLY, dir_fd=topfd)
dirfd = open(name, O_RDONLY | O_NONBLOCK, dir_fd=topfd)
except OSError as err:
if onerror is not None:
onerror(err)
Expand Down
4 changes: 2 additions & 2 deletions Lib/shutil.py
Expand Up @@ -676,7 +676,7 @@ def _rmtree_safe_fd(topfd, path, onexc):
continue
if is_dir:
try:
dirfd = os.open(entry.name, os.O_RDONLY, dir_fd=topfd)
dirfd = os.open(entry.name, os.O_RDONLY | os.O_NONBLOCK, dir_fd=topfd)
dirfd_closed = False
except OSError as err:
onexc(os.open, fullname, err)
Expand Down Expand Up @@ -775,7 +775,7 @@ def onexc(*args):
onexc(os.lstat, path, err)
return
try:
fd = os.open(path, os.O_RDONLY, dir_fd=dir_fd)
fd = os.open(path, os.O_RDONLY | os.O_NONBLOCK, dir_fd=dir_fd)
fd_closed = False
except Exception as err:
onexc(os.open, path, err)
Expand Down
12 changes: 12 additions & 0 deletions Lib/test/test_glob.py
Expand Up @@ -343,6 +343,18 @@ def test_glob_non_directory(self):
eq(self.rglob('nonexistent', '*'), [])
eq(self.rglob('nonexistent', '**'), [])

@unittest.skipUnless(hasattr(os, "mkfifo"), 'requires os.mkfifo()')
@unittest.skipIf(sys.platform == "vxworks",
"fifo requires special path on VxWorks")
def test_glob_named_pipe(self):
path = os.path.join(self.tempdir, 'mypipe')
os.mkfifo(path)
self.assertEqual(self.rglob('mypipe'), [path])
self.assertEqual(self.rglob('mypipe*'), [path])
self.assertEqual(self.rglob('mypipe', ''), [])
self.assertEqual(self.rglob('mypipe', 'sub'), [])
self.assertEqual(self.rglob('mypipe', '*'), [])

def test_glob_many_open_files(self):
depth = 30
base = os.path.join(self.tempdir, 'deep')
Expand Down
82 changes: 78 additions & 4 deletions Lib/test/test_os.py
Expand Up @@ -1290,6 +1290,7 @@ def test_ror_operator(self):

class WalkTests(unittest.TestCase):
"""Tests for os.walk()."""
is_fwalk = False

# Wrapper to hide minor differences between os.walk and os.fwalk
# to tests both functions with the same code base
Expand Down Expand Up @@ -1324,14 +1325,14 @@ def setUp(self):
self.sub11_path = join(self.sub1_path, "SUB11")
sub2_path = join(self.walk_path, "SUB2")
sub21_path = join(sub2_path, "SUB21")
tmp1_path = join(self.walk_path, "tmp1")
self.tmp1_path = join(self.walk_path, "tmp1")
tmp2_path = join(self.sub1_path, "tmp2")
tmp3_path = join(sub2_path, "tmp3")
tmp5_path = join(sub21_path, "tmp3")
self.link_path = join(sub2_path, "link")
t2_path = join(os_helper.TESTFN, "TEST2")
tmp4_path = join(os_helper.TESTFN, "TEST2", "tmp4")
broken_link_path = join(sub2_path, "broken_link")
self.broken_link_path = join(sub2_path, "broken_link")
broken_link2_path = join(sub2_path, "broken_link2")
broken_link3_path = join(sub2_path, "broken_link3")

Expand All @@ -1341,13 +1342,13 @@ def setUp(self):
os.makedirs(sub21_path)
os.makedirs(t2_path)

for path in tmp1_path, tmp2_path, tmp3_path, tmp4_path, tmp5_path:
for path in self.tmp1_path, tmp2_path, tmp3_path, tmp4_path, tmp5_path:
with open(path, "x", encoding='utf-8') as f:
f.write("I'm " + path + " and proud of it. Blame test_os.\n")

if os_helper.can_symlink():
os.symlink(os.path.abspath(t2_path), self.link_path)
os.symlink('broken', broken_link_path, True)
os.symlink('broken', self.broken_link_path, True)
os.symlink(join('tmp3', 'broken'), broken_link2_path, True)
os.symlink(join('SUB21', 'tmp5'), broken_link3_path, True)
self.sub2_tree = (sub2_path, ["SUB21", "link"],
Expand Down Expand Up @@ -1443,6 +1444,11 @@ def test_walk_symlink(self):
else:
self.fail("Didn't follow symlink with followlinks=True")

walk_it = self.walk(self.broken_link_path, follow_symlinks=True)
if self.is_fwalk:
self.assertRaises(FileNotFoundError, next, walk_it)
self.assertRaises(StopIteration, next, walk_it)

def test_walk_bad_dir(self):
# Walk top-down.
errors = []
Expand All @@ -1464,6 +1470,73 @@ def test_walk_bad_dir(self):
finally:
os.rename(path1new, path1)

def test_walk_bad_dir2(self):
walk_it = self.walk('nonexisting')
if self.is_fwalk:
self.assertRaises(FileNotFoundError, next, walk_it)
self.assertRaises(StopIteration, next, walk_it)

walk_it = self.walk('nonexisting', follow_symlinks=True)
if self.is_fwalk:
self.assertRaises(FileNotFoundError, next, walk_it)
self.assertRaises(StopIteration, next, walk_it)

walk_it = self.walk(self.tmp1_path)
self.assertRaises(StopIteration, next, walk_it)

walk_it = self.walk(self.tmp1_path, follow_symlinks=True)
if self.is_fwalk:
self.assertRaises(NotADirectoryError, next, walk_it)
self.assertRaises(StopIteration, next, walk_it)

@unittest.skipUnless(hasattr(os, "mkfifo"), 'requires os.mkfifo()')
@unittest.skipIf(sys.platform == "vxworks",
"fifo requires special path on VxWorks")
def test_walk_named_pipe(self):
path = os_helper.TESTFN + '-pipe'
os.mkfifo(path)
self.addCleanup(os.unlink, path)

walk_it = self.walk(path)
self.assertRaises(StopIteration, next, walk_it)

walk_it = self.walk(path, follow_symlinks=True)
if self.is_fwalk:
self.assertRaises(NotADirectoryError, next, walk_it)
self.assertRaises(StopIteration, next, walk_it)

@unittest.skipUnless(hasattr(os, "mkfifo"), 'requires os.mkfifo()')
@unittest.skipIf(sys.platform == "vxworks",
"fifo requires special path on VxWorks")
def test_walk_named_pipe2(self):
path = os_helper.TESTFN + '-dir'
os.mkdir(path)
self.addCleanup(shutil.rmtree, path)
os.mkfifo(os.path.join(path, 'mypipe'))

errors = []
walk_it = self.walk(path, onerror=errors.append)
next(walk_it)
self.assertRaises(StopIteration, next, walk_it)
self.assertEqual(errors, [])

errors = []
walk_it = self.walk(path, onerror=errors.append)
root, dirs, files = next(walk_it)
self.assertEqual(root, path)
self.assertEqual(dirs, [])
self.assertEqual(files, ['mypipe'])
dirs.extend(files)
files.clear()
if self.is_fwalk:
self.assertRaises(NotADirectoryError, next, walk_it)
self.assertRaises(StopIteration, next, walk_it)
if self.is_fwalk:
self.assertEqual(errors, [])
else:
self.assertEqual(len(errors), 1, errors)
self.assertIsInstance(errors[0], NotADirectoryError)

def test_walk_many_open_files(self):
depth = 30
base = os.path.join(os_helper.TESTFN, 'deep')
Expand Down Expand Up @@ -1529,6 +1602,7 @@ def test_walk_above_recursion_limit(self):
@unittest.skipUnless(hasattr(os, 'fwalk'), "Test needs os.fwalk()")
class FwalkTests(WalkTests):
"""Tests for os.fwalk()."""
is_fwalk = True

def walk(self, top, **kwargs):
for root, dirs, files, root_fd in self.fwalk(top, **kwargs):
Expand Down
17 changes: 17 additions & 0 deletions Lib/test/test_shutil.py
Expand Up @@ -669,6 +669,23 @@ def test_rmtree_on_junction(self):
finally:
shutil.rmtree(TESTFN, ignore_errors=True)

@unittest.skipUnless(hasattr(os, "mkfifo"), 'requires os.mkfifo()')
@unittest.skipIf(sys.platform == "vxworks",
"fifo requires special path on VxWorks")
def test_rmtree_on_named_pipe(self):
os.mkfifo(TESTFN)
try:
with self.assertRaises(NotADirectoryError):
shutil.rmtree(TESTFN)
self.assertTrue(os.path.exists(TESTFN))
finally:
os.unlink(TESTFN)

os.mkdir(TESTFN)
os.mkfifo(os.path.join(TESTFN, 'mypipe'))
shutil.rmtree(TESTFN)
self.assertFalse(os.path.exists(TESTFN))


class TestCopyTree(BaseTest, unittest.TestCase):

Expand Down
@@ -0,0 +1,2 @@
Fix blocking :func:`os.fwalk` and :func:`shutil.rmtree` on opening named
pipe.