Replace Path class with Symfony Path class

[emteknetnz]

Path could be possibly be replaced with the Path class in symfony/filesystem

Notes

• If we can simply swap it out and no unit tests break, then do the work directly in CMS 5 instead. Otherwise if we need to update the code that calls these methods, then target CMS 6

Acceptance criteria

• Determine if replacing the Silverstripe Path class is worthwhile and viable
• Deprecate and replace Path

CMS 5 PRs

• API Deprecate Path class #11381

CMS 6 PRs

Do not merge yet - after CMS 5 PR is merged, reassign to Guy to rebase.

• API Remove Path class in favour of Symfony's Path class #11380
• API Use Symfony's Path class silverstripe-assets#643
• API Use Symfony's Path class silverstripe-graphql#605
• API Use Symfony's Path class silverstripe-graphql-devtools#61
• DOC Document replacing Path class with Symfony's Path class developer-docs#586

[emteknetnz]

I don't think this is worthwhile as the Silverstripe Path methods having additional logic which symfony path does not

Silverstripe Path::join() includes protection against directly traversal i.e. not allowed .. and will throw an exception, whereas Symfony Path will not stop you from having a path with '..' and will simply return a path with the the .. resolved for you - "all "." and ".." segments are removed as far as possible"

The Silverstripe normalize() method also has an additional param $relative which means that the directory seperators will be rtrim()'ed off

If you agree with this assessment then just close this issue

[GuySartorelli]

Silverstripe Path::join() includes protection against directly traversal i.e. not allowed .. and will throw an exception, whereas Symfony Path will not stop you from having a path with '..' and will simply return a path with the the .. resolved for you - "all "." and ".." segments are removed as far as possible"

I honestly think the way symfony does that is better. Directory traversal protection doesn't belong in a "join these two paths together" method, it belongs in whatever is using that path - at which point you'd know where you are or aren't allowed to traverse to.
If I go Path::join('_resources/theme/simple/images/', '../css/'); I should be able to get '_resources/theme/simple/css/' as a result without an error thrown in my face for trying to get a perfectly valid path.

The Silverstripe normalize() method also has an additional param $relative which means that the directory seperators will be rtrim()'ed off

I think it's worth losing that little bit of functionality and having a few ltrim(Path::normalize($myPath), '/'); around the place for the sake of

1. not having any confusion at all about which Path class to use, because there's only one
2. IMO better join functionality
3. All of the additional methods symfony has on their Path class

And yes technically we can use the other Path class along side our one, but that just makes point 1 worse imo.

[michalkleiner]

I'd go with Symphony's Path class, too, so that we don't have the same conversation in a year why we didn't replace this component when we replaced others.

[GuySartorelli]

#11380 (review)
We're not doing it I guess