
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2014-06-15 21:37 bygward, last changed2022-04-11 14:58 byadmin. This issue is nowclosed.
| Messages (10) | |||
|---|---|---|---|
| msg220676 -(view) | Author: Greg Ward (gward)![]() | Date: 2014-06-15 21:37 | |
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 beenissue1545. 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=10242. mkfs.vfat -v dummy3. sudo mount -o loop /tmp/dummy /mntThen create the reproduction script in the root of Python's source dir:cat > copytree-crash.py <<EOFimport osimport shutil# assumptions:# 1. /mnt is a directory writeable by current user# 2. /mnt is a VFAT filesystem# 3. current OS is Linux-ishif os.path.exists('/mnt/example_nt'): print('cleaning up') shutil.rmtree('/mnt/example_nt')print('copying')shutil.copytree('PC/example_nt', '/mnt/example_nt')EOFand run it:sudo ./python copytree-crash.pyCrash happens as documented above. | |||
| msg220677 -(view) | Author: Greg Ward (gward)![]() | Date: 2014-06-15 21:40 | |
In 3.3 and earlier, copytree() crashes roughly as described inissue1545, 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. | |||
| msg220679 -(view) | Author: Greg Ward (gward)![]() | Date: 2014-06-15 21:44 | |
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... | |||
| msg231340 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2014-11-18 20:19 | |
LGTM. Go ahead Greg. | |||
| msg231342 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2014-11-18 20:42 | |
Would it be possible to write a unit test, maybe using unittest.mock to mock most parts? | |||
| msg231463 -(view) | Author: Greg Ward (gward)![]() | Date: 2014-11-21 01:37 | |
> 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:$ ./pythonLib/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 effectPermissionError: ka-boomDuring 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.095sFAILED (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. | |||
| msg231464 -(view) | Author: Greg Ward (gward)![]() | Date: 2014-11-21 01:55 | |
> 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 -r3a1fc6452340http://hg.gerg.ca/cpythonthen merge to trunk and push?Thanks! | |||
| msg232408 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2014-12-10 00:51 | |
New changeset50517a4d7cce by Berker Peksag in branch '3.4':Issue#21775: shutil.copytree(): fix crash when copying to VFAThttps://hg.python.org/cpython/rev/50517a4d7cceNew changeset7d5754af95a9 by Berker Peksag in branch 'default':Issue#21775: shutil.copytree(): fix crash when copying to VFAThttps://hg.python.org/cpython/rev/7d5754af95a9 | |||
| msg232409 -(view) | Author: Berker Peksag (berker.peksag)*![]() | Date: 2014-12-10 00:55 | |
Committed the patch with a NEWS entry. Thanks Greg.(You can send your SSH key tohgaccounts@python.org. See alsohttps://docs.python.org/devguide/coredev.html#ssh) | |||
| msg232425 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2014-12-10 13:11 | |
Note: Python 2.7 is not affected, I cannot find "winerror" in shutil.py. | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:05 | admin | set | github: 65974 |
| 2014-12-10 13:11:01 | vstinner | set | messages: +msg232425 |
| 2014-12-10 00:55:36 | berker.peksag | set | status: open -> closed nosy: +berker.peksag messages: +msg232409 resolution: fixed stage: commit review -> resolved |
| 2014-12-10 00:51:29 | python-dev | set | nosy: +python-dev messages: +msg232408 |
| 2014-11-21 01:55:48 | gward | set | messages: +msg231464 |
| 2014-11-21 01:37:30 | gward | set | messages: +msg231463 |
| 2014-11-18 20:42:12 | vstinner | set | nosy: +vstinner messages: +msg231342 |
| 2014-11-18 20:20:06 | serhiy.storchaka | set | type: behavior |
| 2014-11-18 20:19:50 | serhiy.storchaka | set | versions: + Python 3.5 nosy: +serhiy.storchaka messages: +msg231340 assignee:gward stage: commit review |
| 2014-06-15 21:44:19 | gward | set | messages: +msg220679 |
| 2014-06-15 21:40:21 | gward | set | keywords: +3.4regression messages: +msg220677 |
| 2014-06-15 21:37:40 | gward | create | |