Skip to content

bpo-34384: Fix os.readlink() on Windows #8740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Aug 15, 2018
4 changes: 3 additions & 1 deletion Doc/library/os.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2014,8 +2014,10 @@ features:
The *dir_fd* argument.

.. versionchanged:: 3.6
Accepts a :term:`path-like object`.
Accepts a :term:`path-like object` on Unix.

.. versionchanged:: 3.8
Accepts a :term:`path-like object` and a bytes object on Windows.

.. function:: remove(path, *, dir_fd=None)

Expand Down
47 changes: 47 additions & 0 deletions Lib/test/test_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -2039,6 +2039,53 @@ def test_listdir_extended_path(self):
[os.fsencode(path) for path in self.created_paths])


@unittest.skipUnless(hasattr(os, 'readlink'), 'needs os.readlink()')
class ReadlinkTests(unittest.TestCase):
filelink = 'readlinktest'
filelink_target = os.path.abspath(__file__)
filelinkb = os.fsencode(filelink)
filelinkb_target = os.fsencode(filelink_target)

def setUp(self):
self.assertTrue(os.path.exists(self.filelink_target))
self.assertTrue(os.path.exists(self.filelinkb_target))
self.assertFalse(os.path.exists(self.filelink))
self.assertFalse(os.path.exists(self.filelinkb))

def test_not_symlink(self):
filelink_target = FakePath(self.filelink_target)
self.assertRaises(OSError, os.readlink, self.filelink_target)
self.assertRaises(OSError, os.readlink, filelink_target)

def test_missing_link(self):
self.assertRaises(FileNotFoundError, os.readlink, 'missing-link')
self.assertRaises(FileNotFoundError, os.readlink,
FakePath('missing-link'))

@support.skip_unless_symlink
def test_pathlike(self):
os.symlink(self.filelink_target, self.filelink)
self.addCleanup(support.unlink, self.filelink)
filelink = FakePath(self.filelink)
self.assertEqual(os.readlink(filelink), self.filelink_target)

@support.skip_unless_symlink
def test_pathlike_bytes(self):
os.symlink(self.filelinkb_target, self.filelinkb)
self.addCleanup(support.unlink, self.filelinkb)
path = os.readlink(FakePath(self.filelinkb))
self.assertEqual(path, self.filelinkb_target)
self.assertIsInstance(path, bytes)

@support.skip_unless_symlink
def test_bytes(self):
os.symlink(self.filelinkb_target, self.filelinkb)
self.addCleanup(support.unlink, self.filelinkb)
path = os.readlink(self.filelinkb)
self.assertEqual(path, self.filelinkb_target)
self.assertIsInstance(path, bytes)


@unittest.skipUnless(sys.platform == "win32", "Win32 specific tests")
@support.skip_unless_symlink
class Win32SymlinkTests(unittest.TestCase):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
:func:`os.readlink` now accepts :term:`path-like <path-like object>` and
:class:`bytes` objects on Windows.
93 changes: 34 additions & 59 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -7414,18 +7414,24 @@ If dir_fd is not None, it should be a file descriptor open to a directory,\n\
and path should be relative; path will then be relative to that directory.\n\
dir_fd may not be implemented on your platform.\n\
If it is unavailable, using it will raise a NotImplementedError.");
#endif

#ifdef HAVE_READLINK
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#if defined(HAVE_READLINK) || defined(MS_WINDOWS)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already defined in line 7408.


/* AC 3.5: merge win32 and not together */
static PyObject *
posix_readlink(PyObject *self, PyObject *args, PyObject *kwargs)
{
path_t path;
int dir_fd = DEFAULT_DIR_FD;
#if defined(HAVE_READLINK)
char buffer[MAXPATHLEN+1];
ssize_t length;
#elif defined(MS_WINDOWS)
DWORD n_bytes_returned;
DWORD io_result;
HANDLE reparse_point_handle;

char target_buffer[_Py_MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
_Py_REPARSE_DATA_BUFFER *rdb = (_Py_REPARSE_DATA_BUFFER *)target_buffer;
const wchar_t *print_name;
#endif
PyObject *return_value = NULL;
static char *keywords[] = {"path", "dir_fd", NULL};

Expand All @@ -7436,6 +7442,7 @@ posix_readlink(PyObject *self, PyObject *args, PyObject *kwargs)
READLINKAT_DIR_FD_CONVERTER, &dir_fd))
return NULL;

#if defined(HAVE_READLINK)
Py_BEGIN_ALLOW_THREADS
#ifdef HAVE_READLINKAT
if (dir_fd != DEFAULT_DIR_FD)
Expand All @@ -7455,45 +7462,11 @@ posix_readlink(PyObject *self, PyObject *args, PyObject *kwargs)
return_value = PyUnicode_DecodeFSDefaultAndSize(buffer, length);
else
return_value = PyBytes_FromStringAndSize(buffer, length);
exit:
path_cleanup(&path);
return return_value;
}

#endif /* HAVE_READLINK */

#if !defined(HAVE_READLINK) && defined(MS_WINDOWS)

static PyObject *
win_readlink(PyObject *self, PyObject *args, PyObject *kwargs)
{
const wchar_t *path;
DWORD n_bytes_returned;
DWORD io_result;
PyObject *po, *result;
int dir_fd;
HANDLE reparse_point_handle;

char target_buffer[_Py_MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
_Py_REPARSE_DATA_BUFFER *rdb = (_Py_REPARSE_DATA_BUFFER *)target_buffer;
const wchar_t *print_name;

static char *keywords[] = {"path", "dir_fd", NULL};

if (!PyArg_ParseTupleAndKeywords(args, kwargs, "U|$O&:readlink", keywords,
&po,
dir_fd_unavailable, &dir_fd
))
return NULL;

path = _PyUnicode_AsUnicode(po);
if (path == NULL)
return NULL;

#elif defined(MS_WINDOWS)
/* First get a handle to the reparse point */
Py_BEGIN_ALLOW_THREADS
reparse_point_handle = CreateFileW(
path,
path.wide,
0,
0,
0,
Expand All @@ -7502,8 +7475,10 @@ win_readlink(PyObject *self, PyObject *args, PyObject *kwargs)
0);
Py_END_ALLOW_THREADS

if (reparse_point_handle==INVALID_HANDLE_VALUE)
return win32_error_object("readlink", po);
if (reparse_point_handle == INVALID_HANDLE_VALUE) {
return_value = path_error(&path);
goto exit;
}

Py_BEGIN_ALLOW_THREADS
/* New call DeviceIoControl to read the reparse point */
Expand All @@ -7518,26 +7493,31 @@ win_readlink(PyObject *self, PyObject *args, PyObject *kwargs)
CloseHandle(reparse_point_handle);
Py_END_ALLOW_THREADS

if (io_result==0)
return win32_error_object("readlink", po);
if (io_result == 0) {
return_value = path_error(&path);
goto exit;
}

if (rdb->ReparseTag != IO_REPARSE_TAG_SYMLINK)
{
PyErr_SetString(PyExc_ValueError,
"not a symbolic link");
return NULL;
goto exit;
}
print_name = (wchar_t *)((char*)rdb->SymbolicLinkReparseBuffer.PathBuffer +
rdb->SymbolicLinkReparseBuffer.PrintNameOffset);

result = PyUnicode_FromWideChar(print_name,
rdb->SymbolicLinkReparseBuffer.PrintNameLength / sizeof(wchar_t));
return result;
return_value = PyUnicode_FromWideChar(print_name,
rdb->SymbolicLinkReparseBuffer.PrintNameLength / sizeof(wchar_t));
if (path.narrow) {
Py_SETREF(return_value, PyUnicode_EncodeFSDefault(return_value));
}
#endif
exit:
path_cleanup(&path);
return return_value;
}

#endif /* !defined(HAVE_READLINK) && defined(MS_WINDOWS) */


#endif /* defined(HAVE_READLINK) || defined(MS_WINDOWS) */

#ifdef HAVE_SYMLINK

Expand Down Expand Up @@ -12950,16 +12930,11 @@ static PyMethodDef posix_methods[] = {
OS_GETPRIORITY_METHODDEF
OS_SETPRIORITY_METHODDEF
OS_POSIX_SPAWN_METHODDEF
#ifdef HAVE_READLINK
#if defined(HAVE_READLINK) || defined(MS_WINDOWS)
{"readlink", (PyCFunction)posix_readlink,
METH_VARARGS | METH_KEYWORDS,
readlink__doc__},
#endif /* HAVE_READLINK */
#if !defined(HAVE_READLINK) && defined(MS_WINDOWS)
{"readlink", (PyCFunction)win_readlink,
METH_VARARGS | METH_KEYWORDS,
readlink__doc__},
#endif /* !defined(HAVE_READLINK) && defined(MS_WINDOWS) */
#endif /* defined(HAVE_READLINK) || defined(MS_WINDOWS) */
OS_RENAME_METHODDEF
OS_REPLACE_METHODDEF
OS_RMDIR_METHODDEF
Expand Down