sys.audit() is called multiple times for glob.glob()

[serhiy-storchaka]

BPO	38149
Nosy	@serhiy-storchaka, @zooba, @miss-islington, @tirkarthi
PRs	bpo-38149: Call sys.audit() only once per call for glob.glob(). #18360 [3.8] bpo-38149: Call sys.audit() only once per call for glob.glob(). (GH-18360) #18373

^{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 2020-02-06.11:08:20.855>
created_at = <Date 2019-09-13.09:07:51.017>
labels = ['3.8', 'library', '3.9']
title = 'sys.audit() is called multiple times for glob.glob()'
updated_at = <Date 2020-02-06.11:08:20.853>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2020-02-06.11:08:20.853>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2020-02-06.11:08:20.855>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2019-09-13.09:07:51.017>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 38149
keywords = ['patch']
message_count = 9.0
messages = ['352248', '352252', '352264', '352269', '352297', '352303', '352304', '361474', '361475']
nosy_count = 4.0
nosy_names = ['serhiy.storchaka', 'steve.dower', 'miss-islington', 'xtreak']
pr_nums = ['18360', '18373']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue38149'
versions = ['Python 3.8', 'Python 3.9']

[serhiy-storchaka]

sys.audit() for "glob.glob" is called in a recursive function _iglob(). So in process of executing glob.glob() it can be called multiple times, with the original pattern and with patterns for parent directories, while there are metacharacters in them.

I do not think it was intentional.

[zooba]

Provided it's called with different arguments each time (which it is), there isn't a problem here. Audit hooks are supposed to be informative, not definitive (that is, you almost always need to take the surrounding context into consideration, which is why they are better for logging actions rather than blocking actions).

Though it might be unintentional that glob.glob() recursively handles each path segment when it could more efficiently work forward resolving each wildcard segment as it goes. Is that what you mean?

[serhiy-storchaka]

Using recursion is rather an implementation detail, because splitpath() splits a path from the end (Path.glob() does it from other side). Also, it may be a tiny bit more efficient if the pattern contains a long literal prefix.

[serhiy-storchaka]

Ah, and when added sys.audit() for glob.glob(), should not it be added for Path.glob() too? And for os.walk()?

[zooba]

when added sys.audit() for glob.glob(), should not it be added for Path.glob() too? And for os.walk()?

Sure, those would make sense.

They all go via scandir() which has its own event too (and will do for each directory), but being able to see that it came from a glob or walk call could be useful information.

Maybe moving the sys.audit call in glob.glob further down in the function would avoid some of the recursion? I don't want to duplicate where it's raised too much, as that provides opportunities to bypass it, but this one is only slightly informative - os.scandir is the real one that you'd worry about seeing.

[serhiy-storchaka]

I think it would be better to call sys.audit() in glob.iglob(), before calling the top-level _iglob(). It will give you a general context, and at every recursion level sys.audit() will be called for os.scandir().

[serhiy-storchaka]

It is harder to avoid repeating calls in os.walk(), because it is recursive itself. But you can introduce a recursive helper _walk() and make os.walk() just calling sys.audit() and _walk().

[serhiy-storchaka]

New changeset 54b4f14 by Serhiy Storchaka in branch 'master':
bpo-38149: Call sys.audit() only once per call for glob.glob(). (GH-18360)
54b4f14

[miss-islington]

New changeset 708f472 by Miss Islington (bot) in branch '3.8':
bpo-38149: Call sys.audit() only once per call for glob.glob(). (GH-18360)
708f472