Skip to content

Pathlib join pure windows path and pure posix path changes it behaviour on 3.13 #125069

New issue
New issue
Closed
Closed
Pathlib join pure windows path and pure posix path changes it behaviour on 3.13#125069
Labels
3.13only security fixestopic-pathlibtype-bugAn unexpected behavior, bug, or error

Description

@pohmelie
pohmelie
opened on Oct 7, 2024

Bug report

Bug description:

3.12.7

Python 3.12.7 (main, Oct  8 2024, 00:39:14) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pathlib; pathlib.PureWindowsPath("C:\\A") / pathlib.PurePosixPath("/foo/C:\\B").relative_to("/")
PureWindowsPath('C:/B')

3.13.0rc3

Python 3.13.0rc3 (main, Oct  7 2024, 23:18:57) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pathlib; pathlib.PureWindowsPath("C:\\A") / pathlib.PurePosixPath("/foo/C:\\B").relative_to("/")
PureWindowsPath('C:/A/foo/C:/B')

Not sure if it is a bug, or a fixed behaviour

CPython versions tested on:

3.12, 3.13

Operating systems tested on:

Linux

Linked PRs

Activity

  1. pohmelie
    added
    type-bugAn unexpected behavior, bug, or error
    on Oct 7, 2024
  2. ZeroIntensity
    added
    topic-pathlib
    on Oct 7, 2024
  3. AA-Turner
    added
    3.13only security fixes
    on Oct 7, 2024

  4. AA-Turner commented on Oct 7, 2024

    @AA-Turner
    AA-Turner
    on Oct 7, 2024
    Member

    cc @barneygale, though the reproducer does look a bit off, with multiple drive roots.

    @pohmelie can you reproduce this using only os.path?

    A

  5. pohmelie commented on Oct 7, 2024

    @pohmelie
    pohmelie
    on Oct 7, 2024
    Author

    @AA-Turner
    If you provide a code with os.path module, then I can test it. But, AFAIK, os.path have no ability to manage windows and posix paths.

  6. terryjreedy commented on Oct 7, 2024

    @terryjreedy
    terryjreedy
    on Oct 7, 2024
    Member

    Checking 3.13 What's New or the linked log file might reveal if intentional change.

  7. imfeh2 commented on Oct 8, 2024

    @imfeh2
    imfeh2
    on Oct 8, 2024
    Contributor

    It happens after this change.
    #111431

  8. ZeroIntensity commented on Oct 8, 2024

    @ZeroIntensity
    ZeroIntensity
    on Oct 8, 2024
    Member

    cc @barneygale, apparently that PR introduced a breaking change.

    Edit: oh wait, Adam already pinged, sorry!

  9. barneygale commented on Oct 8, 2024

    @barneygale
    barneygale
    on Oct 8, 2024
    Contributor

    Thanks for reporting this! There's a few things going on here. I think it comes down to pathlib's special handling of PurePath(PurePath(...)) and our efforts to delay joining of *pathsegments. I believe the 3.13 behaviour is correct, and that we should fix 3.12. PR coming up...

  10. barneygale commented on Oct 8, 2024

    @barneygale
    barneygale
    on Oct 8, 2024
    Contributor

    Another repro, this one works in 3.13 and 3.14:

    >>> a = PureWindowsPath('a')
>>> b1 = PurePosixPath('b', 'c:\\d')
>>> b2 = PurePosixPath('b/c:\\d')
>>> b1 == b2
True
>>> a / b1
PureWindowsPath('c:/d')
>>> a / b2
PureWindowsPath('a/b/c:/d')
  11. barneygale
    added 2 commits that reference this issue on Oct 8, 2024

    pythonGH-125069: Fix inconsistent joining in `WindowsPath(PosixPath(.…

    6d90005

    Merge branch 'main' into pythongh-125069

    307f6d7

  12. barneygale commented on Oct 13, 2024

    @barneygale
    barneygale
    on Oct 13, 2024
    Contributor

    PR available, if anyone has a mo to review: #125156

  13. barneygale
    added 2 commits that reference this issue on Oct 13, 2024

    Merge branch 'main' into pythongh-125069

    4cfb6a1

    GH-125069: Fix inconsistent joining in `WindowsPath(PosixPath(...))` (#…

    cb8e599
  14. miss-islington
    added a commit that references this issue on Oct 13, 2024

    pythonGH-125069: Fix inconsistent joining in `WindowsPath(PosixPath(.…

    d87d7d3
  15. barneygale
    added 3 commits that reference this issue on Oct 13, 2024

    [3.12] pythonGH-125069: Fix inconsistent joining in `WindowsPath(Posi…

    ea2eafe

    [3.13] GH-125069: Fix inconsistent joining in `WindowsPath(PosixPath(…

    094d95f

    [3.12] GH-125069: Fix inconsistent joining in `WindowsPath(PosixPath(…

    f49221a

  16. barneygale commented on Oct 13, 2024

    @barneygale
    barneygale
    on Oct 13, 2024
    Contributor

    I've pushed a fix to 3.12, 3.13 and 3.14 (main).

    In the original repro case, the 3.12 behaviour was buggy and the 3.13 behaviour was correct. The optimisation in #111431 (3.13+) happened to fix the issue for paths generated by relative_to(). The change I've applied here should fix the problem at its source.

    Thanks for reporting :)

  17. barneygale
    closed this as completedon Oct 13, 2024

  18. pohmelie commented on Oct 13, 2024

    @pohmelie
    pohmelie
    on Oct 13, 2024
    Author

    @barneygale
    So, what should I do in my library code? Should I dispatch behaviour by python version? Will the fix lands on 3.12.8 and buggy behaviour still presents in 3.12.0 to 3.12.7 inclusive? 🤔 I want equal behaviour of my function on all supported python versions from 3.8 to 3.13.

  19. barneygale commented on Oct 13, 2024

    @barneygale
    barneygale
    on Oct 13, 2024
    Contributor

    @barneygale So, what should I do in my library code? Should I dispatch behaviour by python version? Will the fix lands on 3.12.8 and buggy behaviour still presents in 3.12.0 to 3.12.7 inclusive? 🤔 I want equal behaviour of my function on all supported python versions from 3.8 to 3.13.

    Untested, but I think this will work in all version:

    diff --git a/src/aioftp/server.py b/src/aioftp/server.py
index 1830d78..d31f40a 100644
--- a/src/aioftp/server.py
+++ b/src/aioftp/server.py
@@ -1031,7 +1031,7 @@ class Server:
             else:
                 resolved_virtual_path /= part
         base_path = connection.user.base_path
-        real_path = base_path / resolved_virtual_path.relative_to("/")
+        real_path = base_path / str(resolved_virtual_path.relative_to("/"))
         # replace with `is_relative_to` check after 3.9+ requirements lands
         try:
             real_path.relative_to(base_path)

  20. barneygale commented on Oct 14, 2024

    @barneygale
    barneygale
    on Oct 14, 2024
    Contributor

    Please use https://discuss.python.org/ for general discussion. If you're looking to convince other people to do work on your behalf (e.g. improving the pathlib codebase) then I suggest you strike a more constructive note. If you're planning to put in your own issues and PRs I'd be happy to review.

  21. pohmelie commented on Oct 14, 2024

    @pohmelie
    pohmelie
    on Oct 14, 2024
    Author

    @barneygale yep, looks like this str(...) trick works, thank you!

  22. pohmelie
    added a commit that references this issue on Oct 14, 2024

    force conversion to avoid buggy pathlib implementation for old python…

    e254c57

  23. barneygale commented on Oct 14, 2024

    @barneygale
    barneygale
    on Oct 14, 2024
    Contributor

    Wow what valuable feedback, thanks buddy! :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.13only security fixestopic-pathlibtype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions