Skip to content

fix: Modernize Child Path Detection#497

Open
whoisj wants to merge 5 commits into
mainfrom
jwyman/tri-1023
Open

fix: Modernize Child Path Detection#497
whoisj wants to merge 5 commits into
mainfrom
jwyman/tri-1023

Conversation

@whoisj
Copy link
Copy Markdown
Contributor

@whoisj whoisj commented May 12, 2026

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
Copy link
Copy Markdown
Contributor Author

whoisj commented May 13, 2026

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 Outdated
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
/// Check if a character is a path separator.
/// \param c The character to check.
/// \return `true` when the character is a path separator, otherwise `false`.
bool IsPathSeparator(char c);
Copy link
Copy Markdown
Contributor

@yinggeh yinggeh May 14, 2026

Choose a reason for hiding this comment

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

Suggested change
bool IsPathSeparator(char c);
inline bool IsPathSeparator(char c);

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.

why? it's not like the value could be modified by the function. 😕

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