Skip to content

gh-81793: Always call linkat() from os.link(), if available #132517

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
1 change: 1 addition & 0 deletions Doc/library/os.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2338,6 +2338,7 @@ features:
This function can support specifying *src_dir_fd* and/or *dst_dir_fd* to
supply :ref:`paths relative to directory descriptors <dir_fd>`, and :ref:`not
following symlinks <follow_symlinks>`.
The default value of *follow_symlinks* is ``False`` on Windows.

.. audit-event:: os.link src,dst,src_dir_fd,dst_dir_fd os.link

Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_inspect/test_inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -5844,7 +5844,7 @@ def test_operator_module_has_signatures(self):
self._test_module_has_signatures(operator)

def test_os_module_has_signatures(self):
unsupported_signature = {'chmod', 'utime'}
unsupported_signature = {'chmod', 'link', 'utime'}
unsupported_signature |= {name for name in
['get_terminal_size', 'posix_spawn', 'posix_spawnp',
'register_at_fork', 'startfile']
Expand Down
44 changes: 44 additions & 0 deletions Lib/test/test_posix.py
Original file line number Diff line number Diff line change
Expand Up @@ -1521,6 +1521,50 @@ def test_pidfd_open(self):
self.assertEqual(cm.exception.errno, errno.EINVAL)
os.close(os.pidfd_open(os.getpid(), 0))

@unittest.skipUnless(hasattr(os, "link"), "test needs os.link()")
def test_link_follow_symlinks(self):
default_follow = sys.platform.startswith(
('darwin', 'freebsd', 'netbsd', 'openbsd', 'dragonfly', 'sunos5'))
default_no_follow = sys.platform.startswith(('win32', 'linux'))
orig = os_helper.TESTFN
symlink = orig + 'symlink'
posix.symlink(orig, symlink)
self.addCleanup(os_helper.unlink, symlink)

with self.subTest('no follow_symlinks'):
# no follow_symlinks -> platform depending
link = orig + 'link'
posix.link(symlink, link)
self.addCleanup(os_helper.unlink, link)
if os.link in os.supports_follow_symlinks or default_follow:
self.assertEqual(posix.lstat(link), posix.lstat(orig))
elif default_no_follow:
self.assertEqual(posix.lstat(link), posix.lstat(symlink))

with self.subTest('follow_symlinks=False'):
# follow_symlinks=False -> duplicate the symlink itself
link = orig + 'link_nofollow'
try:
posix.link(symlink, link, follow_symlinks=False)
except NotImplementedError:
if os.link in os.supports_follow_symlinks or default_no_follow:
raise
else:
self.addCleanup(os_helper.unlink, link)
self.assertEqual(posix.lstat(link), posix.lstat(symlink))

with self.subTest('follow_symlinks=True'):
# follow_symlinks=True -> duplicate the target file
link = orig + 'link_following'
try:
posix.link(symlink, link, follow_symlinks=True)
except NotImplementedError:
if os.link in os.supports_follow_symlinks or default_follow:
raise
else:
self.addCleanup(os_helper.unlink, link)
self.assertEqual(posix.lstat(link), posix.lstat(orig))


# tests for the posix *at functions follow
class TestPosixDirFd(unittest.TestCase):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Fix :func:`os.link` on platforms (like Linux) where the
system :c:func:`!link` function does not follow symlinks. On Linux,
it now follows symlinks by default or if
``follow_symlinks=True`` is specified. On Windows, it now raises an error if
``follow_symlinks=True`` is passed. On macOS, it now raises an error if
``follow_symlinks=False`` is passed and the system :c:func:`!linkat`
function is not available at runtime.
6 changes: 3 additions & 3 deletions Modules/clinic/posixmodule.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

105 changes: 43 additions & 62 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,11 @@ extern char *ctermid_r(char *);
# define HAVE_FACCESSAT_RUNTIME 1
# define HAVE_FCHMODAT_RUNTIME 1
# define HAVE_FCHOWNAT_RUNTIME 1
#ifdef __wasi__
# define HAVE_LINKAT_RUNTIME 0
# else
# define HAVE_LINKAT_RUNTIME 1
# endif
# define HAVE_FDOPENDIR_RUNTIME 1
# define HAVE_MKDIRAT_RUNTIME 1
# define HAVE_RENAMEAT_RUNTIME 1
Expand Down Expand Up @@ -4346,7 +4350,7 @@ os.link
*
src_dir_fd : dir_fd = None
dst_dir_fd : dir_fd = None
follow_symlinks: bool = True
follow_symlinks: bool(c_default="-1", py_default="(os.name != 'nt')") = PLACEHOLDER

Create a hard link to a file.

Expand All @@ -4364,31 +4368,46 @@ src_dir_fd, dst_dir_fd, and follow_symlinks may not be implemented on your
static PyObject *
os_link_impl(PyObject *module, path_t *src, path_t *dst, int src_dir_fd,
int dst_dir_fd, int follow_symlinks)
/*[clinic end generated code: output=7f00f6007fd5269a input=b0095ebbcbaa7e04]*/
/*[clinic end generated code: output=7f00f6007fd5269a input=1d5e602d115fed7b]*/
{
#ifdef MS_WINDOWS
BOOL result = FALSE;
#else
int result;
#endif
#if defined(HAVE_LINKAT)
int linkat_unavailable = 0;
#endif

#ifndef HAVE_LINKAT
if ((src_dir_fd != DEFAULT_DIR_FD) || (dst_dir_fd != DEFAULT_DIR_FD)) {
argument_unavailable_error("link", "src_dir_fd and dst_dir_fd");
return NULL;
#ifdef HAVE_LINKAT
if (HAVE_LINKAT_RUNTIME) {
if (follow_symlinks < 0) {
follow_symlinks = 1;
}
}
else
#endif

#ifndef MS_WINDOWS
if ((src->narrow && dst->wide) || (src->wide && dst->narrow)) {
PyErr_SetString(PyExc_NotImplementedError,
"link: src and dst must be the same type");
return NULL;
}
{
if ((src_dir_fd != DEFAULT_DIR_FD) || (dst_dir_fd != DEFAULT_DIR_FD)) {
argument_unavailable_error("link", "src_dir_fd and dst_dir_fd");
return NULL;
}
/* See issue 85527: link() on Linux works like linkat without AT_SYMLINK_FOLLOW,
but on Mac it works like linkat *with* AT_SYMLINK_FOLLOW. */
#if defined(MS_WINDOWS) || defined(__linux__)
if (follow_symlinks == 1) {
argument_unavailable_error("link", "follow_symlinks=True");
return NULL;
}
#elif defined(__APPLE__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) || (defined(__sun) && defined(__SVR4))
if (follow_symlinks == 0) {
argument_unavailable_error("link", "follow_symlinks=False");
return NULL;
}
#else
if (follow_symlinks >= 0) {
argument_unavailable_error("link", "follow_symlinks");
return NULL;
}
#endif
}

if (PySys_Audit("os.link", "OOii", src->object, dst->object,
src_dir_fd == DEFAULT_DIR_FD ? -1 : src_dir_fd,
Expand All @@ -4406,44 +4425,18 @@ os_link_impl(PyObject *module, path_t *src, path_t *dst, int src_dir_fd,
#else
Py_BEGIN_ALLOW_THREADS
#ifdef HAVE_LINKAT
if ((src_dir_fd != DEFAULT_DIR_FD) ||
(dst_dir_fd != DEFAULT_DIR_FD) ||
(!follow_symlinks)) {

if (HAVE_LINKAT_RUNTIME) {

result = linkat(src_dir_fd, src->narrow,
dst_dir_fd, dst->narrow,
follow_symlinks ? AT_SYMLINK_FOLLOW : 0);

}
#ifdef __APPLE__
else {
if (src_dir_fd == DEFAULT_DIR_FD && dst_dir_fd == DEFAULT_DIR_FD) {
/* See issue 41355: This matches the behaviour of !HAVE_LINKAT */
result = link(src->narrow, dst->narrow);
} else {
linkat_unavailable = 1;
}
}
#endif
if (HAVE_LINKAT_RUNTIME) {
result = linkat(src_dir_fd, src->narrow,
dst_dir_fd, dst->narrow,
follow_symlinks ? AT_SYMLINK_FOLLOW : 0);
}
else
#endif /* HAVE_LINKAT */
#endif
{
/* linkat not available */
result = link(src->narrow, dst->narrow);
Py_END_ALLOW_THREADS

#ifdef HAVE_LINKAT
if (linkat_unavailable) {
/* Either or both dir_fd arguments were specified */
if (src_dir_fd != DEFAULT_DIR_FD) {
argument_unavailable_error("link", "src_dir_fd");
} else {
argument_unavailable_error("link", "dst_dir_fd");
}
return NULL;
}
#endif
Py_END_ALLOW_THREADS

if (result)
return path_error2(src, dst);
Expand Down Expand Up @@ -5935,12 +5928,6 @@ internal_rename(path_t *src, path_t *dst, int src_dir_fd, int dst_dir_fd, int is
return path_error2(src, dst);

#else
if ((src->narrow && dst->wide) || (src->wide && dst->narrow)) {
PyErr_Format(PyExc_ValueError,
"%s: src and dst must be the same type", function_name);
return NULL;
}

Py_BEGIN_ALLOW_THREADS
#ifdef HAVE_RENAMEAT
if (dir_fd_specified) {
Expand Down Expand Up @@ -10613,12 +10600,6 @@ os_symlink_impl(PyObject *module, path_t *src, path_t *dst,

#else

if ((src->narrow && dst->wide) || (src->wide && dst->narrow)) {
PyErr_SetString(PyExc_ValueError,
"symlink: src and dst must be the same type");
return NULL;
}

Py_BEGIN_ALLOW_THREADS
#ifdef HAVE_SYMLINKAT
if (dir_fd != DEFAULT_DIR_FD) {
Expand Down
Loading