Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Doc/library/tarfile.rst
Original file line number Diff line number Diff line change
Expand Up @@ -942,9 +942,9 @@ reused in custom filters:
.. note::

Although the filter preserves all permission bits unchanged, the sticky
bit (:const:`~stat.S_ISVTX`) will not be set on extracted files when the
system does not allow it. For compatibility reasons, no error is raised
in this case.
bit (:const:`~stat.S_ISVTX`) will not be set on an extracted file if the
file system does not allow it. For compatibility reasons, no exception is
raised in this case.

.. function:: tar_filter(member, path)

Expand Down
33 changes: 33 additions & 0 deletions Lib/test/support/os_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,39 @@ def skip_unless_working_chmod(test):
return test if ok else unittest.skip(msg)(test)


_can_chmod_set_sticky_bit = None


def can_chmod_set_sticky_bit():
"""Return True if chmod allows to set the sticky bit on a regular file.

Return False if chmod fails with an error, or it succeeds but does not set
the sticky bit.
"""

def can_chmod_set_sticky_bit_inner():
if not can_chmod():
return False
filename = TESTFN
try:
with open(filename, "w") as f:
# assume it won't fail because can_chmod() returned True:
os.chmod(f.fileno(), 0o666)
try:
os.chmod(f.fileno(), 0o666 | stat.S_ISVTX)
except OSError:
return False
mode = os.stat(f.fileno()).st_mode
return bool(mode & stat.S_ISVTX)
finally:
unlink(filename)

global _can_chmod_set_sticky_bit
if _can_chmod_set_sticky_bit is None:
_can_chmod_set_sticky_bit = can_chmod_set_sticky_bit_inner()
Copy link
Member

Choose a reason for hiding this comment

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

Hum, I suggest you to mimick code around in os_helper.py: other "can" functions also have a cache (global variable), but they don't use an "inner" function.

return _can_chmod_set_sticky_bit


# Check whether the current effective user has the capability to override
# DAC (discretionary access control). Typically user root is able to
# bypass file read, write, and execute permission checks. The capability
Expand Down
35 changes: 5 additions & 30 deletions Lib/test/test_tarfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -3057,34 +3057,6 @@ def test_keyword_only(self, mock_geteuid):
tarfl.extract, filename_1, TEMPDIR, False, True)


_can_chmod_set_sticky = None


def can_chmod_set_sticky():
"""Can chmod set the sticky bit on a file?"""

def can_chmod_set_sticky_inner():
if not os_helper.can_chmod():
return False
filename = os_helper.TESTFN + "-chmod-sticky"
try:
with open(filename, "w") as f:
os.chmod(f.fileno(), 0o666)
try:
os.chmod(f.fileno(), 0o666 | stat.S_ISVTX)
except OSError:
return False
mode = os.stat(f.fileno()).st_mode
return bool(mode & stat.S_ISVTX)
finally:
os.unlink(filename)

global _can_chmod_set_sticky
if _can_chmod_set_sticky is None:
_can_chmod_set_sticky = can_chmod_set_sticky_inner()
return _can_chmod_set_sticky


@os_helper.skip_unless_working_chmod
class ExtractStickyFileTest(unittest.TestCase):

Expand All @@ -3108,7 +3080,10 @@ def extract_sticky_file(self, file_name="sticky"):
def test_extract_chmod_eftype_success(self):
Copy link
Member

Choose a reason for hiding this comment

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

On most platforms, this test willl be run without EFTYPE. I suggest to remove it:

Suggested change
def test_extract_chmod_eftype_success(self):
def test_extract_chmod_success(self):

# Extracting a file as non-root should skip the sticky bit (gh-108948)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Extracting a file as non-root should skip the sticky bit (gh-108948)
# gh-108948: Extracting a file as non-root should skip the sticky bit

# even on platforms where chmod fails with EFTYPE (i.e. FreeBSD)
expected_mode = self.sticky_mode if can_chmod_set_sticky() else self.non_sticky_mode
if os_helper.can_chmod_set_sticky_bit():
expected_mode = self.sticky_mode
else:
expected_mode = self.non_sticky_mode
self.extract_sticky_file()
got_mode = stat.filemode(os.stat(self.extracted_path).st_mode)
self.assertEqual(got_mode, expected_mode)
Expand Down Expand Up @@ -3906,7 +3881,7 @@ def test_modes(self):
with open(tmp_filename, 'w'):
pass
new_mode = os.stat(tmp_filename).st_mode | stat.S_ISGID | stat.S_ISUID
if can_chmod_set_sticky():
if os_helper.can_chmod_set_sticky_bit():
new_mode |= stat.S_ISVTX
os.chmod(tmp_filename, new_mode)
got_mode = os.stat(tmp_filename).st_mode
Expand Down