Repository navigation
Clarify the documentation of pathlib.Path.is_relative_to() #99334
Description
Activity
Pathlib never elides
../segments unless you callresolve()I'm not a big fan of
is_relative_to(). And I agree that users may expect a slightly more rigorous check that automatically handles../. Once #99031 lands, the method will be equivalent tob == a or b in a.parents, which IMO is too trivial to justify its existence. We could look at deprecating it?Reacted by muesloIt's probably too late to bother deprecating the function when the implementation is going to become trivial, but the docs could be clarified to effectively state the one-liner to clarify what the function represents.
I would say, if we're going as far as thinking about deprecation, I'd rather keep the function and have it do more:
class PurePath: def is_relative_to(self, other, strict=False): # I've adapted my exemple to the 3.14 implementation, # not the current one (i.e.: `other` vs `*other`) a, b = self, other if strict: # I've read about security concerns around `normpath`. # I think limiting its usage to `PurePath` is the way to # avoid them here (and I think `PurePath` is exactly were # one would want this behavior), but I'm not an expert # on the matter. a = Path(os.path.normpath(a)) b = Path(os.path.normpath(b)) return b == a or b in a.parents class Path(PurePath): def is_relative_to(self, other, strict=False): a, b = self, other if strict: a = a.resolve(strict=True) b = b.resolve(strict=True) return b == a or b in a.parents Path('/var/../../opt').is_relative_to(Path('/var')) >>> True # we keep the current behavior as default Path('/var/../../opt').is_relative_to(Path('/var'), strict=True) >>> False
Although, I have to admit to not being a huge fan of
resolvein this case. Sometimes, what one wants is just to ensure that a path is a child of another path, regardless of any symlink along the way. These cases would not be directly covered by this proposed solution, but we could dopath = PurePath(ROOT_PATH, user_input_rel_path) if path.is_relative_to(ROOT_PATH, strict=True): Path(path).unlink() else: raise PermissionError('Nope!')
Not ideal, I'll admit, but close enough, I think.
But, if I'm being honest, I don't understand when the current behavior is actually useful. In which case would one want
Path('/var/log/../../opt').is_relative_to('/var/log')to return True? I'm assuming it's useful to some, otherwise it wouldn't have been done that way, but I don't see it, personally. So maybe, the actual solution is even simpler:class PurePath: def is_relative_to(self, other): a = Path(os.path.normpath(self)) b = Path(os.path.normpath(other)) return b == a or b in a.parents class Path(PurePath): ... Path('/var/../../opt').is_relative_to(Path('/var')) >>> False path = Path(ROOT_PATH, user_input_rel_path) # use .resolve() if you need symlink resolution if path.is_relative_to(ROOT_PATH): Path.unlink() else: raise PermissionError('Nope!')
This is the more elegant solution but it is a breaking change.
- added a commit that references this issue
on Jan 23, 2024 - added 4 commits that reference this issue
on Jan 23, 2024 Tis done! Thanks @thomas-mckay for reporting, hopefully the new text makes sense to you.
Reacted by Thomas- added a commit that references this issue
on Feb 11, 2024
Hi,
Currently (python 3.10.6 & 3.11.0):
Once you know
is_relative_tousesrelative_to, this makes more sense but it's not obvious from the documentation and the examples given. Also it can easily lead to code that looks secure but isn't. Case in point, I was tasked with reviewing this code today (simplified for illustration purposes):I was unsure if I should open a bug or not because one could easily argue it isn't a bug. I do believe however that a warning in the documentation could save a few devs from making a mistake.
Linked PRs
PurePath.is_relative_to()is purely lexical. #114031PurePath.is_relative_to()is purely lexical. (GH-114031) #114460PurePath.is_relative_to()is purely lexical. (GH-114031). #114461