Skip to content

Python 3.10/3.8: shutil.rmtree(None, ignore_errors=True) behaves differently between Windows and *nix platforms #94692

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

Closed
RamseyK opened this issue Jul 8, 2022 · 8 comments
Labels
docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@RamseyK
Copy link

RamseyK commented Jul 8, 2022

Bug report

Expected behavior: Passing None to shutil.rmtree's path argument should not yield an exception when ignore_errors=True.

Behavior on MacOS Python 3.10 (MacPorts) - Correct behavior:

>>> import shutil
>>> shutil.rmtree(None, ignore_errors=True)
>>> shutil.rmtree(None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/shutil.py", line 712, in rmtree
    onerror(os.lstat, path, sys.exc_info())
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/shutil.py", line 710, in rmtree
    orig_st = os.lstat(path)
TypeError: lstat: path should be string, bytes or os.PathLike, not NoneType

The above occurs on RedHat Linux 8 Python 3.8 and Python 3.10 as well.

Behavior on Windows (Incorrect/differs from MacOS/Linux):

>>> import shutil
>>> shutil.rmtree(None, ignore_errors=True)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python310\lib\shutil.py", line 733, in rmtree
    onerror(os.lstat, path, sys.exc_info())
  File "C:\Python310\lib\shutil.py", line 577, in rmtree
    orig_st = os.lstat(path)
TypeError: lstat: path should be string, bytes or os.PathLike, not NoneType
>>> shutil.rmtree(None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python310\lib\shutil.py", line 733, in rmtree
    onerror(os.lstat, path, sys.exc_info())
  File "C:\Python310\lib\shutil.py", line 577, in rmtree
    orig_st = os.lstat(path)
TypeError: lstat: path should be string, bytes or os.PathLike, not NoneType

Environment

  • CPython Versions Tested: 3.8.x, 3.10.x
  • Operating Systems: Windows 10 21H2, MacOS, RedHat Linux 8 (x86-64)

Linked PRs

@RamseyK RamseyK added the type-bug An unexpected behavior, bug, or error label Jul 8, 2022
@RamseyK RamseyK changed the title shutil.rmtree(None) with ignore_errors behaves differently shutil.rmtree(None) with ignore_errors behaves differently between platforms Jul 8, 2022
@RamseyK RamseyK changed the title shutil.rmtree(None) with ignore_errors behaves differently between platforms shutil.rmtree(None) with ignore_errors behaves differently between Windows and *nix platforms Jul 8, 2022
@RamseyK RamseyK changed the title shutil.rmtree(None) with ignore_errors behaves differently between Windows and *nix platforms shutil.rmtree(None, ignore_errors=True) behaves differently between Windows and *nix platforms Jul 8, 2022
@RamseyK RamseyK changed the title shutil.rmtree(None, ignore_errors=True) behaves differently between Windows and *nix platforms Python 3.10: shutil.rmtree(None, ignore_errors=True) behaves differently between Windows and *nix platforms Jul 8, 2022
@RamseyK RamseyK changed the title Python 3.10: shutil.rmtree(None, ignore_errors=True) behaves differently between Windows and *nix platforms Python 3.10/3.8: shutil.rmtree(None, ignore_errors=True) behaves differently between Windows and *nix platforms Jul 8, 2022
@ericvsmith
Copy link
Member

Expected behavior: Passing None to shutil.rmtree's path argument should not yield an exception when ignore_errors=True.

I disagree with this. The intent isn't to hide all errors, but rather filesystem errors.

The only change here might be to not ignore the TypeError on macOS.

@hauntsaninja
Copy link
Contributor

+1 to Eric. It's conceivable that these except Exception should be made into except OSError:

cpython/Lib/shutil.py

Lines 721 to 727 in 4bed0db

except Exception:
onerror(os.lstat, path, sys.exc_info())
return
try:
fd = os.open(path, os.O_RDONLY, dir_fd=dir_fd)
fd_closed = False
except Exception:
(as is the case in several of the rmtree helpers). It looks like those lines date pretty far back, so not clear that except Exception was intentional.

@ericvsmith
Copy link
Member

Yeah, it's unclear if maybe macOS raises some other non-OSError exception. It would take a lot of research, and more knowledge of macOS than I'll ever have. So I suspect the best we could do is document this and leave it be. Unless @RamseyK has some specific case where this is causing a real problem, I don't think we'll make a change here.

@eryksun
Copy link
Contributor

eryksun commented Jul 10, 2022

Functions in the os module such as os.lstat() and os.open() are supposed to raise OSError for errors due to system calls. The exact errno (or winerror) values may vary across platforms. A Python runtime error such as TypeError, ValueError (e.g. embedded null in a string), or ArithmeticError (e.g. fd value exceeds INT_MAX) is a script logic error, not a "failed removal" at the filesystem level.

@RamseyK
Copy link
Author

RamseyK commented Jul 10, 2022

Thanks for the clarification @ericvsmith @eryksun . I think documenting specifically that ignore_errors does not trap all exceptions as I (and others on my team) had naively assumed a different behavior of ignore_errors. Perhaps except Exception @hauntsaninja pointed out should be except OSError so the behavior is a bit more consistent across platforms.

@iritkatriel iritkatriel added stdlib Python modules in the Lib dir docs Documentation in the Doc dir labels Nov 25, 2023
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Dec 5, 2023
Previously a symlink attack resistant version of shutil.rmtree() could ignore
or pass to the error handler arbitrary exception when invalid arguments
were provided.
@serhiy-storchaka
Copy link
Member

I am wondering whether this change should be considered a bug fix (and backported) or a new feature (and only applied to main).

@ambv
Copy link
Contributor

ambv commented Dec 5, 2023

Since this isn't a new regression, it is properly worked around by people who noticed, and by those who didn't, it's a behavioral change. Let's treat as feature.

ambv pushed a commit that referenced this issue Dec 5, 2023
Previously a symlink attack resistant version of shutil.rmtree() could ignore
or pass to the error handler arbitrary exception when invalid arguments
were provided.
@serhiy-storchaka
Copy link
Member

Then perhaps add versionchanged and tweak the vague documentation?

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Dec 7, 2023
serhiy-storchaka added a commit that referenced this issue Dec 8, 2023
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
Previously a symlink attack resistant version of shutil.rmtree() could ignore
or pass to the error handler arbitrary exception when invalid arguments
were provided.
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
Previously a symlink attack resistant version of shutil.rmtree() could ignore
or pass to the error handler arbitrary exception when invalid arguments
were provided.
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

7 participants