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

shutil.copytree() crashes copying to VFAT on Linux: AttributeError: 'PermissionError' object has no attribute 'winerror' #65974

Closed
gward mannequin opened this issue Jun 15, 2014 · 10 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@gward
Copy link
Mannequin

gward mannequin commented Jun 15, 2014

BPO 21775
Nosy @vstinner, @berkerpeksag, @serhiy-storchaka

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2014-12-10.00:55:36.980>
created_at = <Date 2014-06-15.21:37:40.917>
labels = ['type-bug', 'library']
title = "shutil.copytree() crashes copying to VFAT on Linux: AttributeError: 'PermissionError' object has no attribute 'winerror'"
updated_at = <Date 2014-12-10.13:11:01.975>
user = 'https://bugs.python.org/gward'

bugs.python.org fields:

activity = <Date 2014-12-10.13:11:01.975>
actor = 'vstinner'
assignee = 'gward'
closed = True
closed_date = <Date 2014-12-10.00:55:36.980>
closer = 'berker.peksag'
components = ['Library (Lib)']
creation = <Date 2014-06-15.21:37:40.917>
creator = 'gward'
dependencies = []
files = []
hgrepos = []
issue_num = 21775
keywords = ['3.4regression']
message_count = 10.0
messages = ['220676', '220677', '220679', '231340', '231342', '231463', '231464', '232408', '232409', '232425']
nosy_count = 5.0
nosy_names = ['gward', 'vstinner', 'python-dev', 'berker.peksag', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21775'
versions = ['Python 3.4', 'Python 3.5']

@gward
Copy link
Mannequin Author

gward mannequin commented Jun 15, 2014

When using shutil.copytree() on Linux to copy to a VFAT filesystem, it crashes like this:

Traceback (most recent call last):
  File "/data/src/cpython/3.4/Lib/shutil.py", line 336, in copytree
    copystat(src, dst)
  File "/data/src/cpython/3.4/Lib/shutil.py", line 190, in copystat
    lookup("chmod")(dst, mode, follow_symlinks=follow)
PermissionError: [Errno 1] Operation not permitted: '/mnt/example_nt'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "copytree-crash.py", line 14, in <module>
    shutil.copytree('PC/example_nt', '/mnt/example_nt')
  File "/data/src/cpython/3.4/Lib/shutil.py", line 339, in copytree
    if why.winerror is None:
AttributeError: 'PermissionError' object has no attribute 'winerror'

I am *not* complaining about the PermissionError. That has been bpo-1545. Rather, I'm complaining about the crash that happens while attempting to handle the PermissionError.

Reproducing this is fairly easy, although it requires root privilege.

  1. dd if=/dev/zero of=dummy bs=1024 count=1024
  2. mkfs.vfat -v dummy
  3. sudo mount -o loop /tmp/dummy /mnt

Then create the reproduction script in the root of Python's source dir:

cat > copytree-crash.py <<EOF
import os
import shutil

# assumptions:
# 1. /mnt is a directory writeable by current user
# 2. /mnt is a VFAT filesystem
# 3. current OS is Linux-ish

if os.path.exists('/mnt/example_nt'):
    print('cleaning up')
    shutil.rmtree('/mnt/example_nt')

print('copying')
shutil.copytree('PC/example_nt', '/mnt/example_nt')
EOF

and run it:

sudo ./python copytree-crash.py

Crash happens as documented above.

@gward gward mannequin added the stdlib Python modules in the Lib dir label Jun 15, 2014
@gward
Copy link
Mannequin Author

gward mannequin commented Jun 15, 2014

In 3.3 and earlier, copytree() crashes roughly as described in bpo-1545, with shutil.Error that wraps the underlying "Operation not permitted" error from trying to chmod() something in a VFAT filesystem. Since this appears to accurately reflect what's coming from the kernel, I don't *think* this is a bug. Only the AttributeError, which is new in 3.4, is clearly a bug.

@gward
Copy link
Mannequin Author

gward mannequin commented Jun 15, 2014

Bad news: because reproducing this requires sudo (to mount an arbitrary filesystem), I'm not sure it's possible/desirable to add test code for it.

Good news: the fix is trivial, and it passes my manual test. Here's a patch:

--- a/Lib/shutil.py
+++ b/Lib/shutil.py
@@ -336,7 +336,7 @@
         copystat(src, dst)
     except OSError as why:
         # Copying file access times may fail on Windows
-        if why.winerror is None:
+        if getattr(why, 'winerror', None) is None:
             errors.append((src, dst, str(why)))
     if errors:
         raise Error(errors)

Running test suite now to make sure this doesn't break anything else...

@serhiy-storchaka
Copy link
Member

LGTM. Go ahead Greg.

@serhiy-storchaka serhiy-storchaka added the type-bug An unexpected behavior, bug, or error label Nov 18, 2014
@vstinner
Copy link
Member

Would it be possible to write a unit test, maybe using unittest.mock to mock most parts?

@gward
Copy link
Mannequin Author

gward mannequin commented Nov 21, 2014

Would it be possible to write a unit test, maybe using unittest.mock to
mock most parts?

Good idea! Turns out this was quite straightforward. The test patch is:

--- a/Lib/test/test_shutil.py
+++ b/Lib/test/test_shutil.py
@@ -1,6 +1,7 @@
 # Copyright (C) 2003 Python Software Foundation
 
 import unittest
+import unittest.mock
 import shutil
 import tempfile
 import sys
@@ -758,6 +759,20 @@
         self.assertEqual(os.stat(restrictive_subdir).st_mode,
                           os.stat(restrictive_subdir_dst).st_mode)
 
+    @unittest.mock.patch('os.chmod')
+    def test_copytree_winerror(self, mock_patch):
+        # When copying to VFAT, copystat() raises OSError. On Windows, the
+        # exception object has a meaningful 'winerror' attribute, but not
+        # on other operating systems. Do not assume 'winerror' is set.
+        src_dir = tempfile.mkdtemp()
+        dst_dir = os.path.join(tempfile.mkdtemp(), 'destination')
+        self.addCleanup(shutil.rmtree, src_dir)
+        self.addCleanup(shutil.rmtree, os.path.dirname(dst_dir))
+
+        mock_patch.side_effect = PermissionError('ka-boom')
+        with self.assertRaises(shutil.Error):
+            shutil.copytree(src_dir, dst_dir)
+
     @unittest.skipIf(os.name == 'nt', 'temporarily disabled on Windows')
     @unittest.skipUnless(hasattr(os, 'link'), 'requires os.link')
     def test_dont_copy_file_onto_link_to_itself(self):

When run without the bug fix, this reproduces my original failure nicely:

$ ./python Lib/test/test_shutil.py
................s........................s.s........E..s..............................s..

======================================================================
ERROR: test_copytree_winerror (main.TestShutil)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/data/src/cpython/3.4/Lib/shutil.py", line 337, in copytree
    copystat(src, dst)
  File "/data/src/cpython/3.4/Lib/shutil.py", line 191, in copystat
    lookup("chmod")(dst, mode, follow_symlinks=follow)
  File "/data/src/cpython/3.4/Lib/unittest/mock.py", line 896, in __call__
    return _mock_self._mock_call(*args, **kwargs)
  File "/data/src/cpython/3.4/Lib/unittest/mock.py", line 952, in _mock_call
    raise effect
PermissionError: ka-boom

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/data/src/cpython/3.4/Lib/unittest/mock.py", line 1136, in patched
    return func(*args, **keywargs)
  File "Lib/test/test_shutil.py", line 774, in test_copytree_winerror
    shutil.copytree(src_dir, dst_dir)
  File "/data/src/cpython/3.4/Lib/shutil.py", line 340, in copytree
    if why.winerror is None:
AttributeError: 'PermissionError' object has no attribute 'winerror'

Ran 89 tests in 0.095s

FAILED (errors=1, skipped=5)

Excellent! No need for root privs, and the bug is quite obvious.

Apply my getattr() fix, and the test passes.

I'll commit on branch 3.4 and merge to default.

@gward
Copy link
Mannequin Author

gward mannequin commented Nov 21, 2014

I'll commit on branch 3.4 and merge to default.

Whoops, never mind. Looks like I don't have push permission to hg.python.org after all. It's been 8 years since my last commit, so I shouldn't complain.

So... can someone with commit privs please

hg pull -r 3a1fc6452340 http://hg.gerg.ca/cpython

then merge to trunk and push?

Thanks!

@python-dev
Copy link
Mannequin

python-dev mannequin commented Dec 10, 2014

New changeset 50517a4d7cce by Berker Peksag in branch '3.4':
Issue bpo-21775: shutil.copytree(): fix crash when copying to VFAT
https://hg.python.org/cpython/rev/50517a4d7cce

New changeset 7d5754af95a9 by Berker Peksag in branch 'default':
Issue bpo-21775: shutil.copytree(): fix crash when copying to VFAT
https://hg.python.org/cpython/rev/7d5754af95a9

@berkerpeksag
Copy link
Member

Committed the patch with a NEWS entry. Thanks Greg.

(You can send your SSH key to hgaccounts@python.org. See also https://docs.python.org/devguide/coredev.html#ssh)

@vstinner
Copy link
Member

Note: Python 2.7 is not affected, I cannot find "winerror" in shutil.py.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants