[issue33010] os.path.isdir() returns True for broken directory symlinks or junctions

[Steve Dower]

Steve Dower  added the comment:

Fixed in 3.8 by changes to os.stat implementation.

--
resolution:  -> fixed
stage:  -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33010] os.path.isdir() returns True for broken directory symlinks or junctions

[Jeffrey Kintscher]

Change by Jeffrey Kintscher :


--
nosy: +Jeffrey.Kintscher

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33010] os.path.isdir() returns True for broken directory symlinks or junctions

[Eryk Sun]

Eryk Sun  added the comment:

Thanks for the quick feedback and pushing back on this. We just have a broken 
mount point, unlike in Unix where it's a regular directory. so you're right to 
just query the basic info for all directory reparse points, without 
special-casing mount points.

However, I do think a valid mount point is a directory. I don't see why 
os.path.ismount() returns true for a non-existent mount point, drive letter, or 
share. Is this just to avoid network access delays with unavailable mapped 
drives and shares? Also, because of the hard-coded UNC check in ismount(), we 
aren't supporting mount points in directories on shares. I think this needs 
work.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33010] os.path.isdir() returns True for broken directory symlinks or junctions

[Alexey Izbyshev]

Alexey Izbyshev  added the comment:

Hmm, actually, my NFS example is probably bad. I've run an experiment, and 
stat() simply hangs in the case if the NFS server is down.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33010] os.path.isdir() returns True for broken directory symlinks or junctions

[Alexey Izbyshev]

Alexey Izbyshev  added the comment:

Thank you for the detailed response, Eryk!

> A mount point is always a directory, even if the volume isn't currently 
> available.

Do I understand correctly that you propose to additionally change 
os.path.exists() to return True for mount points with unavailable volumes? 
Сurrently, os.path.exists() (i.e, the underlying os.stat()) attempts to 
traverse them, and this would be consistent with os.path.isdir() if the latter 
were changed to traverse directory reparse points too (both would return False 
for such mount points). Is your idea to change the behavior to match POSIX in a 
similar case when, for example, the remote NFS server is down but stat() still 
works on the local mount point? If so, is this a new idea compared to the first 
paragraph of [1] where you say that non-link reparse points should always be 
traversed?

[1] https://github.com/python/cpython/pull/5998#discussion_r172402233

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33010] os.path.isdir() returns True for broken directory symlinks or junctions

[Eryk Sun]

Eryk Sun  added the comment:

Yes, it makes sense to call GetFileAttributes as the fast path, and then fall 
back on a slow path (i.e. create, query, close) for a directory reparse point. 
With GetFileInformationByHandleEx, the equivalent query is FileBasicInfo, which 
is available starting with Vista, so this could be backported to 3.5 if 
necessary. 

However, to be more consistent with POSIX, we need to first query 
FileAttributeTagInfo on the reparse point to get the reparse tag. If it's 
IO_REPARSE_TAG_MOUNT_POINT (junction) and the target is a volume name (i.e. 
"Volume{GUID}"), then we should return true. This can also be checked via 
GetVolumePathName, after normalizing the path, like how os.path.ismount() works 
on Windows (ntpath.py). A mount point is always a directory, even if the volume 
isn't currently available. OTOH, sometimes junctions are used as links instead, 
which is being addressed more generally by Vidar Fauske in issue 31226. There's 
potentially overlap here with Vidar's proposed _Py_is_reparse_link function.

More info on GetFileAttributes, if you're curious:

GetFileAttributes and GetFileAttributesEx are relatively cheap I/O calls. 
They're implemented by translating to a native NT path and calling 
NtQueryAttributesFile and NtQueryFullAttributesFile, respectively. These two 
system calls are optimized for network access. They use an open packet that 
that's query-only without reparsing. The I/O Manager's parse routine can thus 
use a local (fake) File object, the file system's corresponding fast I/O 
routine, and skip the normal cruft of working with an I/O request (i.e. 
IRP_MJ_CREATE, IRP_MJ_QUERY_INFORMATION, IRP_MJ_CLEANUP, and IRP_MJ_CLOSE). In 
the sample fastfat driver, these routines are respectively 
FatFastQueryBasicInfo [1] and FatFastQueryNetworkOpenInfo [2].

[1]: 
https://github.com/Microsoft/Windows-driver-samples/blob/aa6e0b36eb932099fa4eb950a6f5e289a23b6d6e/filesys/fastfat/fatdata.c#L933

[2]: 
https://github.com/Microsoft/Windows-driver-samples/blob/aa6e0b36eb932099fa4eb950a6f5e289a23b6d6e/filesys/fastfat/fatdata.c#L1272

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33010] os.path.isdir() returns True for broken directory symlinks or junctions

[Alexey Izbyshev]

New submission from Alexey Izbyshev :

os.path.isdir() violates its own documentation by returning True for broken 
directory symlinks or junctions, for which os.path.exists() returns False:

>>> os.mkdir('b')
>>> import _winapi
>>> _winapi.CreateJunction('b', 'a')
>>> os.rmdir('b')
>>> os.path.exists('a')
False
>>> os.path.isdir('a')
True

The underlying problem is that os.path.isdir() uses GetFileAttributes, which is 
documented not to follow symlinks.

Eryk, is there a cheaper way to check FILE_ATTRIBUTE_DIRECTORY on a path while 
following reparse points apart from 
CreateFile/GetFileInformationByHandleEx/CloseFile?

Also, does it make sense to use GetFileAttributes as a fast path and use 
something like above as a fallback only if FILE_ATTRIBUTE_REPARSE_POINT is set, 
or does GetFileAttributes do something equivalently expensive under the hood?

--
components: Extension Modules, Windows
messages: 313314
nosy: eryksun, izbyshev, paul.moore, steve.dower, tim.golden, zach.ware
priority: normal
severity: normal
status: open
title: os.path.isdir() returns True for broken directory symlinks or junctions
type: behavior
versions: Python 3.6, Python 3.7, Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com