GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-106037: Disarm `os.PathLike` foot-shotgun in `pathlib.PurePath` user subclasses by barneygale · Pull Request #106043 · python/cpython

Conversation

@barneygale

@barneygale barneygale commented Jun 23, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

We made it possible to subclass pathlib.PurePath in a68e585, which landed in 3.12. However, user subclasses automatically inherit an __fspath__() method, which may not be appropriate. For example, a user subclass may implement a "virtual" filesystem to provide access to a .zip file or FTP server. But it would be highly surprising if open(FTPPath(...)) attempted to open a local file.

This patch makes the os.PathLike interface opt-in for pathlib.PurePath subclasses. In pathlib itself, we opt into the os.PathLike interface for PurePosixPath, PureWindowsPath and Path. As PurePath is not instantiable (you always get a PurePosixPath or PureWindowsPath object back), this is backwards compatible with 3.11.

  • Issue: User subclasses of pathlib.PurePath are os.PathLike #106037

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@barneygale
pythonGH-106037: Allow subclasses of pathlib.PurePath to opt into `…
cba36a3 
…os.PathLike`

We made it possible to subclass `pathlib.PurePath` in a68e585, which landed
in 3.12. However, user subclasses automatically inherit an `__fspath__()`
method, which may not be appropriate. For example, a user subclass may
implement a "virtual" filesystem to provide access to a `.zip` file or FTP
server. But it would be highly surprising if `open(FTPPath(...))` attempted
to open a *local* file.

This patch makes the `os.PathLike` interface opt-in. In pathlib itself, we
opt into the `os.PathLike` interface for `PurePosixPath`, `PureWindowsPath`
and `Path`. As `PurePath` is not instantiable (you always get a
`PurePosixPath` or `PureWindowsPath` object back), this is backwards
compatible with 3.11, but not with earlier 3.12 betas.
@barneygale barneygale added topic-pathlib needs backport to 3.12 only security fixes labels Jun 23, 2023
@bedevere-bot bedevere-bot added the awaiting core review label Jun 23, 2023
@bedevere-bot bedevere-bot mentioned this pull request Jun 23, 2023
User subclasses of pathlib.PurePath are os.PathLike #106037
Closed
@barneygale barneygale mentioned this pull request Jun 23, 2023
GH-89812: Add pathlib._PurePathExt #104810
Closed
@AlexWaygood
AlexWaygood self-requested a review June 23, 2023 18:03
AlexWaygood
AlexWaygood reviewed Jun 23, 2023
View reviewed changes

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Nice idea. Are we sure it's backwards compatible to remove __fspath__ from PurePath? Looks like there are a few user subclasses of it in the wild: https://github.com/pytest-dev/pyfakefs/blob/b576d65bbd9a7508c90e6b00aa3b9eccf4a86fa1/pyfakefs/fake_pathlib.py#L796-L805

One possible alternative solution might be to implement #106046, and then just document that users should set __fspath__ to None if they don't want their path subclasses to be considered subtypes of os.PathLike...?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
barneygale reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@barneygale

barneygale commented Jun 23, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

That would at least allow me to set __fspath__ = None in AbstractPath, which would lessen the impact significantly.

Looks like there are a few user subclasses of it in the wild

That example you gave was only recently updated for 3.12 beta 1. There shouldn't be many more out there, and as I consider this a bugfix, it's reasonable to change behaviour anyway I think
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@barneygale

barneygale commented Jun 23, 2023

Copy link
Copy Markdown
Contributor Author

Alex, I've played around with just setting __fspath__ = None where I need, and it works nicely. I mean, the exception message is bogus, but that's still a simpler fix than this PR or the previous _BasePurePath PR. Thanks so much for the pointer. I'll close this but keep the bug open (it's technically valid, but I have a workaround for AbstractPath so less pressing)
AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@barneygale barneygale closed this Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting core review needs backport to 3.12 only security fixes topic-pathlib

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@barneygale @AlexWaygood @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.