pathlib.(r)glob stops on PermissionDenied exception

[Gregorio]

BPO	24120
Nosy	@gvanrossum, @pitrou, @blueyed, @zware, @serhiy-storchaka
PRs	[WIP/RFC] pathlib: revisit error handling #23025
Files	issue24120.diff issue24120_2.diff

^{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 = 'https://github.com/gvanrossum'
closed_at = <Date 2016-01-07.19:00:14.939>
created_at = <Date 2015-05-03.17:52:28.914>
labels = ['type-bug', 'library']
title = 'pathlib.(r)glob stops on PermissionDenied exception'
updated_at = <Date 2020-10-29.12:06:03.904>
user = 'https://bugs.python.org/Gregorio'

bugs.python.org fields:

activity = <Date 2020-10-29.12:06:03.904>
actor = 'blueyed'
assignee = 'gvanrossum'
closed = True
closed_date = <Date 2016-01-07.19:00:14.939>
closer = 'gvanrossum'
components = ['Library (Lib)']
creation = <Date 2015-05-03.17:52:28.914>
creator = 'Gregorio'
dependencies = []
files = ['39634', '39843']
hgrepos = []
issue_num = 24120
keywords = ['patch']
message_count = 17.0
messages = ['242496', '242727', '244876', '244899', '245032', '245034', '245884', '246077', '246085', '246092', '257510', '257618', '257623', '257627', '257707', '257710', '257713']
nosy_count = 8.0
nosy_names = ['gvanrossum', 'pitrou', 'blueyed', 'python-dev', 'zach.ware', 'serhiy.storchaka', 'Gregorio', 'ulope']
pr_nums = ['23025']
priority = 'normal'
resolution = 'fixed'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue24120'
versions = ['Python 3.4', 'Python 3.5', 'Python 3.6']

[Gregorio]

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.

[ethanfurman]

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)

[ulope]

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

[serhiy-storchaka]

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.

[serhiy-storchaka]

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.

[ethanfurman]

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.

[pitrou]

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.

[ulope]

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.

[pitrou]

Ok, thanks. The tests are running ok on my Windows VM.

[serhiy-storchaka]

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

[gvanrossum]

I just ran into this issue. What's keeping it from being committed, except perhaps that Antoine decided to leave core developments?

[gvanrossum]

I'm just going to commit this.