Repository navigation
gh-157899: Add support for fds pointing to symlinks in os.readlink(). #157915
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
base: main
Are you sure you want to change the base?
Changes from all commits
2cbd30e
8bb6f09
8196b59
f96963a
7b566b7
253c3b4
20fa1e2
6c8f5f1
9bf9236
c8d85a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1881,6 +1881,74 @@ def test_readlink_dir_fd(self): | |
| self.addCleanup(posix.unlink, fullname) | ||
| self.assertEqual(posix.readlink(name, dir_fd=dir_fd), 'symlink') | ||
|
|
||
|
|
||
| _support_readlink_with_fd = ( | ||
| hasattr(os, 'readlink') \ | ||
| and os.readlink in os.supports_fd | ||
| ) | ||
|
|
||
| def _open_symlink_as_fd(self, path): | ||
| open_flags = os.O_RDONLY | ||
| if hasattr(os, "O_SYMLINK"): # macOS/iOS | ||
| open_flags |= os.O_SYMLINK | ||
| elif hasattr(os, "O_NOFOLLOW") and hasattr(os, "O_PATH"): # Linux/Android | ||
| open_flags |= os.O_NOFOLLOW | os.O_PATH | ||
| else: | ||
| self.fail("lacking open flags for this test") | ||
| return os.open(path, open_flags) | ||
|
|
||
| @unittest.skipUnless(_support_readlink_with_fd, | ||
| "feature not supported on this platform") | ||
| def test_readlink_with_fd(self): | ||
| with self.prepare() as (dir_fd, name, fullname): | ||
| os.symlink("symlink", fullname) | ||
| self.addCleanup(posix.unlink, fullname) | ||
| fd = self._open_symlink_as_fd(fullname) | ||
| self.addCleanup(os.close, fd) | ||
| self.assertEqual(os.readlink(fd), "symlink") | ||
|
|
||
| @unittest.skipUnless(_support_readlink_with_fd, | ||
| "feature not supported on this platform") | ||
| def test_readlink_with_fd_not_referring_to_symlink_throws(self): | ||
| with self.prepare_file() as (dir_fd, name, fullname): | ||
| fd = os.open(fullname, os.O_RDONLY) | ||
| self.addCleanup(os.close, fd) | ||
| # on Linux/Android, readlinkat("", fd, ...) fails with ENOENT, which | ||
| # Python translates to a FileNotFoundError, a subclass of OSError. | ||
| # On macOS, freadlink(fd, ...) fails with EINVAL, which gets raised | ||
| # as a OSError. | ||
| # So catching OSError here covers both cases. | ||
| with self.assertRaises(OSError): | ||
| os.readlink(fd) | ||
|
|
||
| @unittest.skipUnless(_support_readlink_with_fd, | ||
| "feature not supported on this platform") | ||
| def test_readlink_with_fd_throws_if_both_fd_and_dir_fd_are_given(self): | ||
| with self.prepare() as (dir_fd, name, fullname): | ||
| os.symlink("symlink", fullname) | ||
| self.addCleanup(posix.unlink, fullname) | ||
| fd = self._open_symlink_as_fd(fullname) | ||
| self.addCleanup(os.close, fd) | ||
| with self.assertRaises(ValueError): | ||
| os.readlink(fd, dir_fd=dir_fd) | ||
|
|
||
| @unittest.skipIf(_support_readlink_with_fd, | ||
| "feature is supported on this platform") | ||
| def test_readlink_with_fd_throws_not_implemented_error(self): | ||
| # on unsupported platforms, we may not even be able to get a | ||
| # file descriptor for a symlink, so use a fd for an ordinary file | ||
| os_helper.create_empty_file(os_helper.TESTFN) | ||
| self.addCleanup(os_helper.unlink, os_helper.TESTFN) | ||
| fd = os.open(os_helper.TESTFN, os.O_RDONLY) | ||
| self.addCleanup(os.close, fd) | ||
| with self.assertRaises(NotImplementedError): | ||
| os.readlink(fd) | ||
|
Comment on lines
+1937
to
+1945
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you also test that * (or someone porting to a new platform, after the tests warn them about the issue)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, from what I can see iOS has I also found that apparently iOS does have |
||
|
|
||
| @unittest.skipUnless(sys.platform in ["linux", "android", "darwin", "ios"], | ||
| "feature not supported on this platform") | ||
| def test_readlink_is_marked_as_supports_fd(self): | ||
| self.assertIn(os.readlink, os.supports_fd) | ||
|
|
||
| @unittest.skipUnless(os.rename in os.supports_dir_fd, "test needs dir_fd support in os.rename()") | ||
| def test_rename_dir_fd(self): | ||
| with self.prepare_file() as (dir_fd, name, fullname), \ | ||
|
|
@@ -2620,6 +2688,17 @@ def test_readlink(self): | |
| with self.assertRaisesRegex(NotImplementedError, "dir_fd unavailable"): | ||
| os.readlink("path", dir_fd=0) | ||
|
|
||
| def test_freadlink(self): | ||
| self._verify_available("HAVE_FREADLINK") | ||
| if self.mac_ver >= (13, 0): | ||
| self.assertIn("HAVE_FREADLINK", posix._have_functions) | ||
|
|
||
| else: | ||
| self.assertNotIn("HAVE_FREADLINK", posix._have_functions) | ||
| errmsg = "readlink cannot read file descriptors on this platform" | ||
| with self.assertRaisesRegex(NotImplementedError, errmsg): | ||
| os.readlink(0) | ||
|
|
||
| def test_symlink(self): | ||
| self._verify_available("HAVE_SYMLINKAT") | ||
| if self.mac_ver >= (10, 10): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| :func:`os.readlink` now accepts a file descriptor referring to a | ||
| symlink on Linux, Android, macOS and iOS. |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.