classification
Title: pathlib.(r)glob stops on PermissionDenied exception
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.6, Python 3.4, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gvanrossum Nosy List: Gregorio, gvanrossum, pitrou, python-dev, serhiy.storchaka, ulope, zach.ware
Priority: normal Keywords: patch

Created on 2015-05-03 17:52 by Gregorio, last changed 2016-01-07 19:00 by gvanrossum. This issue is now closed.

Files
File name Uploaded Description Edit
issue24120.diff ulope, 2015-06-05 20:19 review
issue24120_2.diff ulope, 2015-07-02 13:57 review
Messages (17)
msg242496 - (view) Author: Gregorio (Gregorio) Date: 2015-05-03 17:52
Debian 8
Python 3.4.3

Path.(r)glob stops when it encounters a PermissionDenied exception.

However, os.walk just skips files/directories it does not have permission to access. 

In my opinion, the os.walk behavior is better because it doesn't prevent usage in situations where a single file/directory in a tree has restricted permissions.
msg242727 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2015-05-07 19:13
From Frank Woodall on python-ideas:
==================================
How to reproduce:

mkdir /tmp/path_test && cd /tmp/path_test && mkdir dir1 dir2 dir2/dir3 && touch dir1/file1 dir1/file2 dir2/file1 dir2/file2 dir2/dir3/file1
su
chmod 700 dir2/dir3/
chown root:root dir2/dir3/
exit

python 3.4.1

from pathlib import Path
p = Path('/tmp/path_test')
for x in p.rglob('*') : print(x)
msg244876 - (view) Author: Ulrich Petri (ulope) * Date: 2015-06-05 20:19
The attached patch adds an unaccessible directory to the pathlib tests to provoke the problem and also fixes the cause in pathlib.

It applies to at least 3.4 - 3.6
msg244899 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-06-06 12:01
The glob module is not affected because it ignores all OSErrors (including "Permission denied" and "Too many levels of symbolic links").

>>> import glob
>>> glob.glob('**/*', recursive=True)
['dir2/file1', 'dir2/file2', 'dir2/dir3', 'dir1/file1', 'dir1/file2']

There is no special test for PermissionError.
msg245032 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-06-08 18:47
I'm not sure that Path.(r)glob() (and perhaps glob.glob()) should unconditionally ignore errors. Python has lover level than shell, and even bash has an option that controls this behavior.

    failglob
            If set, patterns which fail to match filenames during pathname expansion result in an expansion error.
msg245034 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2015-06-08 19:13
Failing to find any matches with a pattern is an entirely different class of error than finding matches but hitting permission problems or broken links or suddenly deleted files or ...

If glob doesn't already have a 'failglob' option we could add that, but in a different issue.
msg245884 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015-06-27 17:29
Thank you Ulrich for the patch. I basically agree with the solution you proposed (I'm not a fan of the non-ASCII "ASCII art", by the way, but I won't block the patch on that :-)).

Have you tested the patch and tests under Windows? Otherwise, I (or someone else) will have to do that before accepting it.
msg246077 - (view) Author: Ulrich Petri (ulope) * Date: 2015-07-02 13:57
Antoine, thanks for the review. I didn't realise that `tree` outputs non-ASCII by default. I've updated the patch with a pure ASCII file tree.

Unfortunately I don't have a Windows dev environment available at the moment, so I can't easily test for that.
msg246085 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015-07-02 15:49
Ok, thanks. The tests are running ok on my Windows VM.
msg246092 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-07-02 18:11
May be slightly refactor the code?

    def _select_from(self, parent_path, is_dir, exists, listdir):
        try:
            if not is_dir(parent_path):
                return
            yield from self._select_from2(parent_path, is_dir, exists, listdir)
        except PermissionError:
            return

    def _select_from2(self, parent_path, is_dir, exists, listdir):
        ... # implementation
msg257510 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-01-05 01:56
I just ran into this issue. What's keeping it from being committed, except perhaps that Antoine decided to leave core developments?
msg257618 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-01-06 17:42
I'm just going to commit this.
msg257623 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-01-06 17:54
New changeset bac18cb7b011 by Guido van Rossum in branch '3.4':
Issue #24120: Ignore PermissionError in pathlib.Path.[r]glob(). Ulrich Petri.
https://hg.python.org/cpython/rev/bac18cb7b011

New changeset 224a026b4ca1 by Guido van Rossum in branch '3.5':
Issue #24120: Ignore PermissionError in pathlib.Path.[r]glob(). Ulrich Petri. (Merge 3.4->3.5)
https://hg.python.org/cpython/rev/224a026b4ca1

New changeset f6ae90450a4d by Guido van Rossum in branch 'default':
Issue #24120: Ignore PermissionError in pathlib.Path.[r]glob(). Ulrich Petri. (Merge 3.5->3.6)
https://hg.python.org/cpython/rev/f6ae90450a4d
msg257627 - (view) Author: Gregorio (Gregorio) Date: 2016-01-06 18:19
thanks

On 01/06/2016 06:42 PM, Guido van Rossum wrote:
> Guido van Rossum added the comment:
>
> I'm just going to commit this.
>
> ----------
> assignee:  -> gvanrossum
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue24120>
> _______________________________________
>
msg257707 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-01-07 18:43
I think this is not fixed yet. The try/except clause is currently around certain loops whereas it should be around specific calls (is_dir, exists, listdir. and is_symlink). As it currently is, the behavior is dependent on the ordering of the names returned by listdir(), which is just wrong.

I think this is the root cause of the breakage reported in issue #26012.
msg257710 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-01-07 18:58
New changeset 4043e08e6e52 by Guido van Rossum in branch '3.4':
Add another try/except PermissionError to avoid depending on listdir order. Fix issues #24120 and #26012.
https://hg.python.org/cpython/rev/4043e08e6e52

New changeset 8a3b0c1fb3d3 by Guido van Rossum in branch '3.5':
Add another try/except PermissionError to avoid depending on listdir order. Fix issues #24120 and #26012. (Merge 3.4->3.5)
https://hg.python.org/cpython/rev/8a3b0c1fb3d3

New changeset 398cb8c183da by Guido van Rossum in branch 'default':
Add another try/except PermissionError to avoid depending on listdir order. Fix issues #24120 and #26012. (Merge 3.5->3.6)
https://hg.python.org/cpython/rev/398cb8c183da
msg257713 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-01-07 19:00
Should be fixed for real now.
History
Date User Action Args
2016-01-07 19:00:14gvanrossumsetstatus: open -> closed

messages: + msg257713
2016-01-07 18:58:59python-devsetmessages: + msg257710
2016-01-07 18:43:56gvanrossumsetstatus: closed -> open

messages: + msg257707
2016-01-06 18:19:31Gregoriosetmessages: + msg257627
2016-01-06 17:55:03gvanrossumsetstatus: open -> closed
resolution: fixed
2016-01-06 17:54:12python-devsetnosy: + python-dev
messages: + msg257623
2016-01-06 17:42:33gvanrossumsetassignee: gvanrossum
messages: + msg257618
2016-01-05 01:56:22gvanrossumsetnosy: + gvanrossum
messages: + msg257510
2015-07-21 07:07:21ethan.furmansetnosy: - ethan.furman
2015-07-02 18:11:55serhiy.storchakasetmessages: + msg246092
2015-07-02 15:49:14pitrousetmessages: + msg246085
2015-07-02 13:57:59ulopesetfiles: + issue24120_2.diff

messages: + msg246077
2015-06-27 17:29:50pitrousetnosy: + zach.ware
messages: + msg245884
2015-06-27 17:26:57pitrousetstage: patch review
versions: + Python 3.5, Python 3.6
2015-06-08 19:13:27ethan.furmansetmessages: + msg245034
2015-06-08 18:47:33serhiy.storchakasetmessages: + msg245032
2015-06-06 12:01:58serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg244899
2015-06-05 20:19:39ulopesetfiles: + issue24120.diff

nosy: + ulope
messages: + msg244876

keywords: + patch
2015-05-07 19:13:32ethan.furmansetmessages: + msg242727
2015-05-07 19:10:59ethan.furmansetnosy: + ethan.furman
2015-05-05 20:39:07ned.deilysetnosy: + pitrou
2015-05-03 17:52:28Gregoriocreate