Skip to content

Fix Windows media source paths for Flysystem - #16995

Open
Ibochkarev wants to merge 1 commit into
modxcms:3.xfrom
Ibochkarev:fix/issue-15438-windows-path-separator
Open

Fix Windows media source paths for Flysystem#16995
Ibochkarev wants to merge 1 commit into
modxcms:3.xfrom
Ibochkarev:fix/issue-15438-windows-path-separator

Conversation

@Ibochkarev

Copy link
Copy Markdown
Collaborator

What changed and why

On Windows, Flysystem object keys use /, while media source code joined and split paths with DIRECTORY_SEPARATOR (\). Nested rename dropped the parent directory and moved the object to the wrong place (often under the site core path). #15440 only cleared PHP “passed by reference” notices; path creation stayed broken (confirmed in issue comments).

This change treats Flysystem object paths as always /: sanitizePath / postfixSlash, helpers for root / join / rename, and the same join logic for create, move, list ids, and S3.

How to test

  1. On Windows (or with backslash-style inputs): create a nested folder under a media source, rename it, confirm it stays under the same parent.
  2. Rename a nested file the same way.
  3. Create a folder/file under a nested path and move an object between folders.
  4. Gate E:
    • php -l core/src/Revolution/Sources/modMediaSource.php → exit 0
    • php -l core/src/Revolution/Sources/modS3MediaSource.php → exit 0
    • composer run-script phpunit -- --filter modMediaSourceTest → OK (4 tests, 15 assertions)

Related issue(s)/PR(s)

Resolves #15438

Compatibility notes

Affects local and S3 media sources on all platforms. Behavior on Unix stays the same (/ already matched DIRECTORY_SEPARATOR). Windows file manager path joins should now match Flysystem keys. Local OS path handling in prepareSrcForThumb still uses DIRECTORY_SEPARATOR for phpThumb/file_exists checks.

Breaking change assessment

No public method signatures changed. Path strings returned from media source APIs may normalize separators to / more consistently (Flysystem-compatible). Safe for patch consumers that already treated Flysystem paths as /.

Test coverage

_build/test/Tests/Model/Sources/modMediaSourceTest.php: sanitize/postfix with \, getRenamedPath parent retention, root detection after postfixSlash('')/, joinFilesystemPaths for move/create-style joins.

Contributors

Thanks @sergant210 for the report, @wax100 for the Windows/OpenServer reproduction, and @Mark-H for confirming #15440 did not fix path creation.

AI tool use

Cursor agent assisted implementation, tests, and review loops. Human review still required before merge.

Flysystem object keys always use "/", so DIRECTORY_SEPARATOR broke
rename/create/move path joins on Windows. Normalize virtual paths to
forward slashes and keep parent directories on rename.
@Ibochkarev
Ibochkarev force-pushed the fix/issue-15438-windows-path-separator branch from a1269a0 to da88351 Compare August 15, 2026 02:04
@Ibochkarev Ibochkarev added the bug The issue in the code or project, which should be addressed. label Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug The issue in the code or project, which should be addressed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MODX file manager in Windows systems works incorrect

1 participant