Clarify the documentation of pathlib.Path.is_relative_to()

[thomas-mckay]

Hi,

Currently (python 3.10.6 & 3.11.0):

from pathlib import Path

p = Path('/var/log/../../opt')
p.is_relative_to('/var/log')
>>> True

p = p.resolve()
p.is_relative_to('/var/log')
>>> False

Once you know is_relative_to uses relative_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):

path = Path(ROOT_PATH, user_input_rel_path)
if path.is_relative_to(ROOT_PATH):
    path.unlink()
else:
    raise PermissionError('Nope!')

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

• GH-99334: Explain that PurePath.is_relative_to() is purely lexical. #114031
• [3.12] GH-99334: Explain that PurePath.is_relative_to() is purely lexical. (GH-114031) #114460
• [3.11] GH-99334: Explain that PurePath.is_relative_to() is purely lexical. (GH-114031). #114461

[barneygale]

Pathlib never elides ../ segments unless you call resolve()

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 to b == a or b in a.parents, which IMO is too trivial to justify its existence. We could look at deprecating it?

[brettcannon]

It'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.

[thomas-mckay]

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 resolve in 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 do

path = 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.

[barneygale]

Tis done! Thanks @thomas-mckay for reporting, hopefully the new text makes sense to you.