Skip to content

Util: fix is_path_below() when parent is root path#28

Open
Ahelsamahy wants to merge 1 commit intoedgewall:trunkfrom
Ahelsamahy:fix/is-path-below-root-parent
Open

Util: fix is_path_below() when parent is root path#28
Ahelsamahy wants to merge 1 commit intoedgewall:trunkfrom
Ahelsamahy:fix/is-path-below-root-parent

Conversation

@Ahelsamahy
Copy link
Copy Markdown

Summary

This PR fixes an edge case in trac.util.is_path_below() when the parent path is a filesystem root.

Problem

The previous implementation used string prefix matching:

  • path == parent or path.startswith(parent + os.sep)

This fails for root parents (for example / on POSIX, and drive roots on Windows), because appending os.sep to a root path does not produce a reliable boundary check.

Solution

Replaced the prefix check with os.path.commonpath(...).

Tests

Added a regression test in PathTestCase:

@Ahelsamahy
Copy link
Copy Markdown
Author

I did open a ticket on the website related to the current PR on https://trac.edgewall.org/ticket/13896

Comment thread trac/util/__init__.py
return path == parent or path.startswith(parent + os.sep)
try:
return os.path.commonpath((path, parent)) == parent
except ValueError:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why catching ValueError?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Comment thread .gitignore
.project
.pydevproject
.settings
.DS_Store
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is not related with this PR.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agree, I did add it for my side as I'm using macOS and this is how it keeps trace of the dir layout. Should I remove it? My system will auto generate it again when i open finder on this repo locally

@Ahelsamahy Ahelsamahy requested a review from jun66j5 April 17, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants