Skip to content

fix: Modernize Child Path Detection#497

Merged
whoisj merged 6 commits into
mainfrom
jwyman/tri-1023
May 18, 2026
Merged

fix: Modernize Child Path Detection#497
whoisj merged 6 commits into
mainfrom
jwyman/tri-1023

Conversation

@whoisj

@whoisj whoisj commented May 12, 2026

Copy link
Copy Markdown
Contributor

This change updates the algorithm used to detect if a 'child path' escapes its parent.

@whoisj whoisj requested review from mudit-eng, pskiran1 and yinggeh May 12, 2026 18:44
@whoisj whoisj added bug Something isn't working PR: fix A bug fix labels May 12, 2026
Comment thread src/filesystem/api.cc Outdated
Comment thread src/filesystem/api.cc Outdated
Comment thread src/filesystem/api.cc Outdated
Comment thread src/filesystem/api.cc Outdated
@mudit-eng

Copy link
Copy Markdown
Contributor

How do we validate this code works?

whoisj added 2 commits May 12, 2026 20:39
This change updates the alogrithm used to detect if a 'child path' escapes its parent.
@whoisj whoisj force-pushed the jwyman/tri-1023 branch from ec8dc5e to 4399e50 Compare May 13, 2026 00:39
Comment thread src/filesystem/api.cc Outdated
Comment thread src/model_repository_manager/model_repository_manager.cc Outdated
@whoisj whoisj force-pushed the jwyman/tri-1023 branch from 7936ea5 to 6774c6d Compare May 13, 2026 16:17
@whoisj

whoisj commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

How do we validate this code works?

One would assume our CI tests cover most if not all cases. I'll see to it that a new one with the case that this PR fixes is added.

@whoisj whoisj requested review from mudit-eng and yinggeh May 13, 2026 16:19
Comment thread src/filesystem/api.cc
Comment thread src/filesystem/api.cc Outdated
@whoisj whoisj requested a review from yinggeh May 14, 2026 18:06
Comment thread src/filesystem/api.h Outdated
@whoisj whoisj force-pushed the jwyman/tri-1023 branch from 9994e0f to 9d3fc7d Compare May 15, 2026 17:42
Comment thread src/filesystem/api.h Outdated
/// \param c The character to check.
/// \return `true` when the character is a path separator, otherwise `false`.
bool IsPathSeparator(char c);
inline bool

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe the function should be in anonymous namespace in api.cc instead of making a public interface of class LocalizedPath.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if there is a need of this function given that Triton does not have Windows support.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just removed it.

@whoisj whoisj requested a review from yinggeh May 18, 2026 19:34
@whoisj whoisj merged commit dcb315d into main May 18, 2026
1 check passed
@whoisj whoisj deleted the jwyman/tri-1023 branch May 18, 2026 22:23
@yinggeh yinggeh mentioned this pull request May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working PR: fix A bug fix

Development

Successfully merging this pull request may close these issues.

3 participants