diff --git a/_build/test/Tests/Model/Sources/modMediaSourceTest.php b/_build/test/Tests/Model/Sources/modMediaSourceTest.php index 7108a144057..32fb9f0b885 100644 --- a/_build/test/Tests/Model/Sources/modMediaSourceTest.php +++ b/_build/test/Tests/Model/Sources/modMediaSourceTest.php @@ -1,4 +1,5 @@ source = $this->modx->newObject(modMediaSource::class); + $this->source = $this->modx->newObject(modFileMediaSource::class); $this->source->fromArray([ 'name' => 'UnitTestSource', 'description' => '', 'class_key' => modFileMediaSource::class, 'properties' => [], - ],'',true); + ], '', true); } /** * Tear down fixtures after each test. * * @after */ - public function tearDownFixtures() { + public function tearDownFixtures() + { parent::tearDownFixtures(); $this->source = null; } - public function testExample() { - $this->assertTrue(true); + /** + * Flysystem object paths must stay on forward slashes, even when the OS uses "\". + */ + public function testSanitizePathUsesForwardSlash() + { + $this->assertSame('folder/sub', $this->source->sanitizePath('folder\\sub')); + $this->assertSame('folder/sub', $this->source->sanitizePath('folder//sub')); + $this->assertSame('folder/sub/', $this->source->postfixSlash('folder\\sub')); + $this->assertSame('/', $this->source->postfixSlash('')); + } + + /** + * Renaming a nested path must keep the parent directory (#15438 Windows regression). + */ + public function testGetRenamedPathKeepsParentWithMixedSeparators() + { + $method = new ReflectionMethod(modMediaSource::class, 'getRenamedPath'); + + $this->assertSame('parent/new-name', $method->invoke($this->source, 'parent/old-name', 'new-name')); + $this->assertSame('parent/new-name', $method->invoke($this->source, 'parent\\old-name', 'new-name')); + $this->assertSame('a/b/renamed', $method->invoke($this->source, 'a/b/c', 'renamed')); + $this->assertSame('renamed', $method->invoke($this->source, 'old-name', 'renamed')); + } + + /** + * Root detection must treat "/" as root after postfixSlash (Windows DIRECTORY_SEPARATOR is "\"). + */ + public function testIsFilesystemRootPathAfterPostfix() + { + $method = new ReflectionMethod(modMediaSource::class, 'isFilesystemRootPath'); + + $this->assertTrue($method->invoke($this->source, '/')); + $this->assertTrue($method->invoke($this->source, '\\')); + $this->assertTrue($method->invoke($this->source, '')); + $this->assertFalse($method->invoke($this->source, 'assets/')); + } + + /** + * Move/create path joins must use "/" so Windows hosts match Flysystem keys. + */ + public function testJoinFilesystemPathsUsesForwardSlash() + { + $method = new ReflectionMethod(modMediaSource::class, 'joinFilesystemPaths'); + + $this->assertSame('folder/sub/file.txt', $method->invoke($this->source, 'folder/sub/', 'file.txt')); + $this->assertSame('folder/sub/file.txt', $method->invoke($this->source, 'folder\\sub\\', 'file.txt')); + $this->assertSame('file.txt', $method->invoke($this->source, '/', 'file.txt')); } } diff --git a/core/src/Revolution/Sources/modMediaSource.php b/core/src/Revolution/Sources/modMediaSource.php index 2d43cf04911..a60ac2a96a4 100644 --- a/core/src/Revolution/Sources/modMediaSource.php +++ b/core/src/Revolution/Sources/modMediaSource.php @@ -72,6 +72,58 @@ abstract class modMediaSource extends modAccessibleSimpleObject implements modMe public const SOURCE_FILESYSTEM = 'Filesystem'; + /** + * Path separator for Flysystem virtual object paths. + * Flysystem normalizes paths to "/", including on Windows. + */ + public const FILESYSTEM_SEPARATOR = '/'; + + /** + * Whether a Flysystem object path is the media source root. + * + * @param string $path + * + * @return bool + */ + protected function isFilesystemRootPath($path) + { + $path = $this->sanitizePath((string)$path); + + return $path === '' || $path === self::FILESYSTEM_SEPARATOR; + } + + /** + * Trim Flysystem separators from both ends of an object path. + * + * @param string $path + * + * @return string + */ + protected function trimFilesystemPath($path) + { + return trim($this->sanitizePath((string)$path), self::FILESYSTEM_SEPARATOR); + } + + /** + * Join Flysystem object path segments with "/". + * + * @param string ...$segments + * + * @return string + */ + protected function joinFilesystemPaths(...$segments) + { + $parts = []; + foreach ($segments as $segment) { + $segment = $this->trimFilesystemPath($segment); + if ($segment !== '') { + $parts[] = $segment; + } + } + + return implode(self::FILESYSTEM_SEPARATOR, $parts); + } + /** * Get the default MODX filesystem source * @@ -289,11 +341,11 @@ public function getBases($path = '') $bases['pathAbsolute'] = $bases['path']; } - $bases['pathAbsoluteWithPath'] = $bases['pathAbsolute'] . ltrim($path, DIRECTORY_SEPARATOR); + $bases['pathAbsoluteWithPath'] = $bases['pathAbsolute'] . ltrim($path, self::FILESYSTEM_SEPARATOR); if (is_dir($bases['pathAbsoluteWithPath'])) { $bases['pathAbsoluteWithPath'] = $this->postfixSlash($bases['pathAbsoluteWithPath']); } - $bases['pathRelative'] = ltrim($path, DIRECTORY_SEPARATOR); + $bases['pathRelative'] = ltrim($path, self::FILESYSTEM_SEPARATOR); // get relative url $bases['urlIsRelative'] = false; @@ -305,8 +357,8 @@ public function getBases($path = '') $bases['urlAbsolute'] = $bases['url']; } - $bases['urlAbsoluteWithPath'] = $bases['urlAbsolute'] . ltrim($path, DIRECTORY_SEPARATOR); - $bases['urlRelative'] = ltrim($path, DIRECTORY_SEPARATOR); + $bases['urlAbsoluteWithPath'] = $bases['urlAbsolute'] . ltrim($path, self::FILESYSTEM_SEPARATOR); + $bases['urlRelative'] = ltrim($path, self::FILESYSTEM_SEPARATOR); return $bases; } @@ -351,7 +403,7 @@ public function getContainerList($path) { $properties = $this->getPropertyListWithDefaults(); $path = $this->postfixSlash($path); - if ($path == DIRECTORY_SEPARATOR || $path == '\\') { + if ($this->isFilesystemRootPath($path)) { $path = ''; } @@ -402,7 +454,7 @@ public function getContainerList($path) $dirNames[] = strtoupper($file_name); $visibility = $this->visibility_dirs ? $this->getVisibility($object['path']) : false; $directories[$file_name] = [ - 'id' => rawurlencode(rtrim($object['path'], DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR), + 'id' => rawurlencode(rtrim($object['path'], self::FILESYSTEM_SEPARATOR) . self::FILESYSTEM_SEPARATOR), 'sid' => $this->get('id'), 'text' => $file_name, 'cls' => implode(' ', $cls), @@ -470,7 +522,7 @@ public function getObjectsInContainer($path) $fullPath = $path; if (!empty($bases['pathAbsolute'])) { - $fullPath = $bases['pathAbsolute'] . ltrim($path, DIRECTORY_SEPARATOR); + $fullPath = $bases['pathAbsolute'] . ltrim($path, self::FILESYSTEM_SEPARATOR); } $imageExtensions = explode(',', $properties['imageExtensions']); @@ -480,7 +532,7 @@ public function getObjectsInContainer($path) $skipExtensions = $this->getSkipExtensionsArray($properties); $files = $fileNames = []; - if (!empty($path) && $path != DIRECTORY_SEPARATOR) { + if (!empty($path) && !$this->isFilesystemRootPath($path)) { try { $mimeType = $this->filesystem->mimeType($path); } catch (FilesystemException | UnableToRetrieveMetadata $e) { @@ -506,7 +558,7 @@ public function getObjectsInContainer($path) foreach ($contents as $object) { if ( in_array($object['path'], $skipFiles) || - in_array(trim($object['path'], DIRECTORY_SEPARATOR), $skipFiles) || + in_array($this->trimFilesystemPath($object['path']), $skipFiles) || in_array($fullPath . $object['path'], $skipFiles) ) { continue; @@ -594,7 +646,7 @@ public function getObjectContents($path) ); try { $fa = [ - 'name' => rtrim($path, DIRECTORY_SEPARATOR), + 'name' => rtrim($path, self::FILESYSTEM_SEPARATOR), 'basename' => basename($path), 'path' => $path, 'size' => $this->filesystem->fileSize($path), @@ -628,10 +680,10 @@ public function getObjectContents($path) */ public function createContainer($name, $parentContainer) { - if ($parentContainer == DIRECTORY_SEPARATOR) { + if ($this->isFilesystemRootPath($parentContainer)) { $parentContainer = ''; } - $path = $this->sanitizePath($parentContainer . DIRECTORY_SEPARATOR . ltrim($name, DIRECTORY_SEPARATOR)); + $path = $this->joinFilesystemPaths($parentContainer, $name); // Ensure directory doesn't already exist. try { @@ -691,12 +743,12 @@ public function checkFileExists() */ public function createObject($path, $name, $content) { - if ($path == DIRECTORY_SEPARATOR) { + if ($this->isFilesystemRootPath($path)) { $path = ''; } $path = !empty($path) - ? $this->sanitizePath($path . DIRECTORY_SEPARATOR . ltrim($name, DIRECTORY_SEPARATOR)) - : $name; + ? $this->joinFilesystemPaths($path, $name) + : $this->trimFilesystemPath($name); if (!$this->checkFileType($path)) { return false; @@ -756,7 +808,7 @@ public function moveObject($from, $to, $point = 'append', $to_source = 0) { $path = $this->postfixSlash($from); $to = $this->postfixSlash($to); - $newPath = rtrim($to, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR . basename($from); + $newPath = $this->joinFilesystemPaths($to, basename($this->sanitizePath($from))); // Ensure object can be read. try { @@ -800,8 +852,8 @@ public function moveObject($from, $to, $point = 'append', $to_source = 0) 'destination' => $destination, ]); $mountManager->move( - 'org://' . ltrim($path, DIRECTORY_SEPARATOR), - 'destination://' . ltrim($newPath, DIRECTORY_SEPARATOR) + 'org://' . ltrim($path, self::FILESYSTEM_SEPARATOR), + 'destination://' . ltrim($newPath, self::FILESYSTEM_SEPARATOR) ); } catch (FilesystemException | UnableToMoveFile $e) { $this->addError('source', $e->getMessage()); @@ -921,6 +973,28 @@ public function removeObject($path) } + /** + * Build a renamed object path while keeping the parent directory. + * Always uses "/" so Windows hosts match Flysystem path keys. + * + * @param string $oldPath Current object path from Flysystem + * @param string $newName New basename (file or folder name) + * + * @return string + */ + protected function getRenamedPath($oldPath, $newName) + { + $oldPath = $this->trimFilesystemPath($oldPath); + $newName = $this->trimFilesystemPath($newName); + $parent = dirname($oldPath); + if ($parent === '.' || $parent === self::FILESYSTEM_SEPARATOR) { + return $newName; + } + + return $this->joinFilesystemPaths($parent, $newName); + } + + /** * @param string $oldPath * @param string $newName @@ -929,16 +1003,9 @@ public function removeObject($path) */ public function renameContainer($oldPath, $newName) { - $oldPath = trim($oldPath, DIRECTORY_SEPARATOR); - if (strpos($oldPath, DIRECTORY_SEPARATOR)) { - $path = explode(DIRECTORY_SEPARATOR, $oldPath); - array_pop($path); - $newPath = implode(DIRECTORY_SEPARATOR, $path) . DIRECTORY_SEPARATOR . $newName; - } else { - $newPath = $newName; - } - $oldPath = $this->sanitizePath($oldPath) . DIRECTORY_SEPARATOR; - $newPath = $this->sanitizePath($newPath) . DIRECTORY_SEPARATOR; + $newPath = $this->getRenamedPath($oldPath, $newName); + $oldPath = $this->sanitizePath($oldPath) . self::FILESYSTEM_SEPARATOR; + $newPath = $this->sanitizePath($newPath) . self::FILESYSTEM_SEPARATOR; // Ensure current directory can be read. try { @@ -994,14 +1061,7 @@ public function renameContainer($oldPath, $newName) */ public function renameObject($oldPath, $newName) { - $oldPath = trim($oldPath, DIRECTORY_SEPARATOR); - if (strpos($oldPath, DIRECTORY_SEPARATOR)) { - $path = explode(DIRECTORY_SEPARATOR, $oldPath); - array_pop($path); - $newPath = implode(DIRECTORY_SEPARATOR, $path) . DIRECTORY_SEPARATOR . $newName; - } else { - $newPath = $newName; - } + $newPath = $this->getRenamedPath($oldPath, $newName); $oldPath = $this->sanitizePath($oldPath); $newPath = $this->sanitizePath($newPath); @@ -1113,7 +1173,7 @@ public function uploadObjectsToContainer($container, array $objects = []) $properties = $this->getPropertyList(); $visibility = $this->xpdo->getOption('visibility', $properties, Visibility::PUBLIC); - if ($container != DIRECTORY_SEPARATOR) { + if (!$this->isFilesystemRootPath($container)) { try { $this->filesystem->fileExists($container); } catch (FilesystemException | UnableToRetrieveMetadata $e) { @@ -1343,7 +1403,7 @@ public function getObjectUrl($object = '') $properties = $this->getPropertyList(); return !empty($properties['baseUrl']) - ? rtrim($properties['baseUrl'], DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR . $object + ? rtrim($properties['baseUrl'], self::FILESYSTEM_SEPARATOR) . self::FILESYSTEM_SEPARATOR . ltrim($object, self::FILESYSTEM_SEPARATOR) : false; } @@ -1593,7 +1653,7 @@ public function prepareSrcForThumb($src) $properties = $this->getPropertyList(); if (isset($properties['url'])) { - $src = $properties['url'] . DIRECTORY_SEPARATOR . ltrim($src, DIRECTORY_SEPARATOR); + $src = $properties['url'] . self::FILESYSTEM_SEPARATOR . ltrim($src, self::FILESYSTEM_SEPARATOR); } // don't strip stuff for absolute URLs @@ -1788,7 +1848,11 @@ public function clearCache(array $options = []) */ public function sanitizePath($path) { - return preg_replace(["/\.*[\/|\\\]/i", "/[\/|\\\]+/i"], [DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR], $path); + return preg_replace( + ['#\.*[/\\\\]#i', '#[/\\\\]+#'], + [self::FILESYSTEM_SEPARATOR, self::FILESYSTEM_SEPARATOR], + (string)$path + ); } @@ -1801,9 +1865,9 @@ public function sanitizePath($path) */ public function postfixSlash($path) { - $len = strlen($path); - if (substr($path, $len - 1, $len) != DIRECTORY_SEPARATOR) { - $path .= DIRECTORY_SEPARATOR; + $path = str_replace('\\', self::FILESYSTEM_SEPARATOR, (string)$path); + if (!str_ends_with($path, self::FILESYSTEM_SEPARATOR)) { + $path .= self::FILESYSTEM_SEPARATOR; } return $path; @@ -1875,7 +1939,7 @@ protected function buildFileList($path, $ext, $image_extensions, $bases, $proper $fullPath = $path; if (!empty($bases['pathAbsolute'])) { - $fullPath = $bases['pathAbsolute'] . ltrim($path, DIRECTORY_SEPARATOR); + $fullPath = $bases['pathAbsolute'] . ltrim($path, self::FILESYSTEM_SEPARATOR); } if (!empty($properties['currentFile']) && rawurldecode($properties['currentFile']) == $fullPath . $path && $properties['currentAction'] == $editAction) { @@ -1913,7 +1977,7 @@ protected function buildFileList($path, $ext, $image_extensions, $bases, $proper 'directory' => $bases['path'], 'url' => $bases['url'] . $path, 'urlExternal' => $this->getObjectUrl($path), - 'urlAbsolute' => $bases['urlAbsoluteWithPath'] . ltrim($file_name, DIRECTORY_SEPARATOR), + 'urlAbsolute' => $bases['urlAbsoluteWithPath'] . ltrim($file_name, self::FILESYSTEM_SEPARATOR), 'file' => rawurlencode($fullPath . $path), ]; if ($this->visibility_files && $visibility) { @@ -2015,8 +2079,8 @@ protected function buildFileBrowserViewList($path, $ext, $image_extensions, $bas 'thumb_height' => $thumb_image_info['height'], 'url' => $path, - 'relativeUrl' => ltrim($path, DIRECTORY_SEPARATOR), - 'fullRelativeUrl' => rtrim($bases['url']) . ltrim($path, DIRECTORY_SEPARATOR), + 'relativeUrl' => ltrim($path, self::FILESYSTEM_SEPARATOR), + 'fullRelativeUrl' => rtrim($bases['url']) . ltrim($path, self::FILESYSTEM_SEPARATOR), 'ext' => $ext, 'pathname' => $path, 'pathRelative' => rawurlencode($path), @@ -2284,7 +2348,7 @@ protected function buildManagerImagePreview($path, $ext, $width, $height, $bases $size = $this->getImageDimensions($path, $ext); if (is_array($size) && $size['width'] > 0 && $size['height'] > 0) { if ($ext == 'svg') { - $size['src'] = $bases['urlAbsolute'] . ltrim($path, DIRECTORY_SEPARATOR); + $size['src'] = $bases['urlAbsolute'] . ltrim($path, self::FILESYSTEM_SEPARATOR); return $size; } diff --git a/core/src/Revolution/Sources/modS3MediaSource.php b/core/src/Revolution/Sources/modS3MediaSource.php index c6db2ac5d21..769a86dd439 100644 --- a/core/src/Revolution/Sources/modS3MediaSource.php +++ b/core/src/Revolution/Sources/modS3MediaSource.php @@ -336,7 +336,7 @@ public function getContainerList($path) { $properties = $this->getPropertyListWithDefaults(); $path = $this->postfixSlash($path); - if ($path === DIRECTORY_SEPARATOR || $path === '\\') { + if ($this->isFilesystemRootPath($path)) { $path = ''; } @@ -371,9 +371,9 @@ public function getContainerList($path) } $re = '#^(.*?/|)(' . implode('|', array_map('preg_quote', $skipFiles)) . ')/?$#'; - $pathid = rawurlencode(rtrim($path, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR); + $pathid = rawurlencode(rtrim($path, self::FILESYSTEM_SEPARATOR) . self::FILESYSTEM_SEPARATOR); foreach ($contents as $object) { - $id = rawurlencode(rtrim($object['path'], DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR); + $id = rawurlencode(rtrim($object['path'], self::FILESYSTEM_SEPARATOR) . self::FILESYSTEM_SEPARATOR); if ($id === $pathid || preg_match($re, $object['path'])) { continue; } @@ -457,7 +457,7 @@ public function getObjectsInContainer($path) $fullPath = $path; if (!empty($bases['pathAbsolute'])) { - $fullPath = $bases['pathAbsolute'] . ltrim($path, DIRECTORY_SEPARATOR); + $fullPath = $bases['pathAbsolute'] . ltrim($path, self::FILESYSTEM_SEPARATOR); } $imageExtensions = explode(',', $properties['imageExtensions']); @@ -467,7 +467,7 @@ public function getObjectsInContainer($path) $files = $fileNames = []; - if (!empty($path) && $path !== DIRECTORY_SEPARATOR) { + if (!empty($path) && !$this->isFilesystemRootPath($path)) { try { $mimeType = $this->filesystem->mimeType($path); } catch (FilesystemException | UnableToRetrieveMetadata $e) { @@ -493,7 +493,7 @@ public function getObjectsInContainer($path) if ( (in_array($fullPath . $object['path'], $skipFiles, true)) || in_array($object['path'], $skipFiles, true) || - in_array(trim($object['path'], DIRECTORY_SEPARATOR), $skipFiles, true) + in_array($this->trimFilesystemPath($object['path']), $skipFiles, true) ) { continue; } @@ -582,16 +582,9 @@ public function removeContainer($path) */ public function renameContainer($oldPath, $newName) { - $oldPath = trim($oldPath, DIRECTORY_SEPARATOR); - if (strpos($oldPath, DIRECTORY_SEPARATOR)) { - $path = explode(DIRECTORY_SEPARATOR, $oldPath); - array_pop($path); - $newPath = implode(DIRECTORY_SEPARATOR, $path) . DIRECTORY_SEPARATOR . $newName; - } else { - $newPath = $newName; - } - $oldPath = $this->sanitizePath($oldPath) . DIRECTORY_SEPARATOR; - $newPath = $this->sanitizePath($newPath) . DIRECTORY_SEPARATOR; + $newPath = $this->getRenamedPath($oldPath, $newName); + $oldPath = $this->sanitizePath($oldPath) . self::FILESYSTEM_SEPARATOR; + $newPath = $this->sanitizePath($newPath) . self::FILESYSTEM_SEPARATOR; // Ensure current directory can be read. try { @@ -771,8 +764,8 @@ protected function buildFileBrowserViewList($path, $ext, $image_extensions, $bas 'thumb_height' => $thumb_height, 'url' => $path, - 'relativeUrl' => ltrim($path, DIRECTORY_SEPARATOR), - 'fullRelativeUrl' => rtrim($bases['url']) . ltrim($path, DIRECTORY_SEPARATOR), + 'relativeUrl' => ltrim($path, self::FILESYSTEM_SEPARATOR), + 'fullRelativeUrl' => rtrim($bases['url']) . ltrim($path, self::FILESYSTEM_SEPARATOR), 'ext' => $ext, 'pathname' => $path, 'pathRelative' => rawurlencode($path),