From 63bbbf29f4b8fc49faf8aafd7ebf27a12e892a06 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 29 Apr 2016 12:42:37 +0200 Subject: [PATCH 1/5] Add wrapper for NFD encoding workaround --- apps/files_trashbin/lib/Storage.php | 2 +- .../Files/Storage/Wrapper/Encoding.php | 568 ++++++++++++++++++ lib/private/legacy/util.php | 9 + tests/lib/files/storage/wrapper/Encoding.php | 154 +++++ 4 files changed, 732 insertions(+), 1 deletion(-) create mode 100644 lib/private/Files/Storage/Wrapper/Encoding.php create mode 100644 tests/lib/files/storage/wrapper/Encoding.php diff --git a/apps/files_trashbin/lib/Storage.php b/apps/files_trashbin/lib/Storage.php index c4c523810a..b621c511db 100644 --- a/apps/files_trashbin/lib/Storage.php +++ b/apps/files_trashbin/lib/Storage.php @@ -150,7 +150,7 @@ class Storage extends Wrapper { return false; } - $normalized = Filesystem::normalizePath($this->mountPoint . '/' . $path); + $normalized = Filesystem::normalizePath($this->mountPoint . '/' . $path, true, false, true); $result = true; $view = Filesystem::getView(); if (!isset($this->deletedFiles[$normalized]) && $view instanceof View) { diff --git a/lib/private/Files/Storage/Wrapper/Encoding.php b/lib/private/Files/Storage/Wrapper/Encoding.php new file mode 100644 index 0000000000..0696f97368 --- /dev/null +++ b/lib/private/Files/Storage/Wrapper/Encoding.php @@ -0,0 +1,568 @@ + + * + * @copyright Copyright (c) 2016, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Files\Storage\Wrapper; + +use OCP\ICache; +use OC\Cache\CappedMemoryCache; + +/** + * Encoding wrapper that deals with file names that use unsupported encodings like NFD. + * + * When applied and a UTF-8 path name was given, the wrapper will first attempt to access + * the actual given name and then try its NFD form. + */ +class Encoding extends Wrapper { + + /** + * @var ICache + */ + private $namesCache; + + /** + * @param array $parameters + */ + public function __construct($parameters) { + $this->storage = $parameters['storage']; + $this->namesCache = new CappedMemoryCache(); + } + + /** + * Returns whether the given string is only made of ASCII characters + * + * @param string $str string + * + * @return bool true if the string is all ASCII, false otherwise + */ + private function isAscii($str) { + foreach (str_split($str) as $s) { + if (ord($s) > 127) { + return false; + } + } + return true; + } + + /** + * Checks whether the given path exists in NFC or NFD form after checking + * each form for each path section and returns the correct form. + * If no existing path found, returns the path as it was given. + * + * @param string $fullPath path to check + * + * @return string original or converted path + */ + private function findPathToUse($fullPath) { + $cachedPath = $this->namesCache[$fullPath]; + if ($cachedPath !== null) { + return $cachedPath; + } + + $sections = explode('/', $fullPath); + $path = ''; + foreach ($sections as $section) { + $path .= $section; + $convertedPath = $this->findPathToUseLastSection($path); + if ($convertedPath === null) { + // no point in continuing if the section was not found, use original path + return $fullPath; + } + $path = $convertedPath . '/'; + } + $path = rtrim($path, '/'); + $this->namesCache[$fullPath] = $path; + return $path; + } + + /** + * Checks whether the last path section of the given path exists in NFC or NFD form + * and returns the correct form. If no existing path found, returns null. + * + * @param string $fullPath path to check + * + * @return string|null original or converted path, or null if none of the forms was found + */ + private function findPathToUseLastSection($fullPath) { + $cachedPath = $this->namesCache[$fullPath]; + if ($cachedPath !== null) { + return $cachedPath; + } + + $sections = explode('/', $fullPath); + $lastSection = array_pop($sections); + $basePath = ltrim(implode('/', $sections) . '/', '/'); + + if ($lastSection === '' || $this->isAscii($lastSection)) { + $this->namesCache[$fullPath] = $fullPath; + return $fullPath; + } + + $result = $this->storage->file_exists($fullPath); + if ($result) { + $this->namesCache[$fullPath] = $fullPath; + return $fullPath; + } + // swap encoding + if (\Normalizer::isNormalized($lastSection, \Normalizer::FORM_C)) { + $otherFormPath = \Normalizer::normalize($lastSection, \Normalizer::FORM_D); + } else { + $otherFormPath = \Normalizer::normalize($lastSection, \Normalizer::FORM_C); + } + if ($this->storage->file_exists($basePath . $otherFormPath)) { + $this->namesCache[$fullPath] = $basePath . $otherFormPath; + return $basePath . $otherFormPath; + } + + // return original path, file did not exist at all + $this->namesCache[$fullPath] = $fullPath; + return null; + } + + /** + * see http://php.net/manual/en/function.mkdir.php + * + * @param string $path + * @return bool + */ + public function mkdir($path) { + // note: no conversion here, method should not be called with non-NFC names! + $result = $this->storage->mkdir($path); + if ($result) { + unset($this->namesCache[$path]); + } + return $result; + } + + /** + * see http://php.net/manual/en/function.rmdir.php + * + * @param string $path + * @return bool + */ + public function rmdir($path) { + $result = $this->storage->rmdir($this->findPathToUse($path)); + if ($result) { + unset($this->namesCache[$path]); + } + return $result; + } + + /** + * see http://php.net/manual/en/function.opendir.php + * + * @param string $path + * @return resource + */ + public function opendir($path) { + return $this->storage->opendir($this->findPathToUse($path)); + } + + /** + * see http://php.net/manual/en/function.is_dir.php + * + * @param string $path + * @return bool + */ + public function is_dir($path) { + return $this->storage->is_dir($this->findPathToUse($path)); + } + + /** + * see http://php.net/manual/en/function.is_file.php + * + * @param string $path + * @return bool + */ + public function is_file($path) { + return $this->storage->is_file($this->findPathToUse($path)); + } + + /** + * see http://php.net/manual/en/function.stat.php + * only the following keys are required in the result: size and mtime + * + * @param string $path + * @return array + */ + public function stat($path) { + return $this->storage->stat($this->findPathToUse($path)); + } + + /** + * see http://php.net/manual/en/function.filetype.php + * + * @param string $path + * @return bool + */ + public function filetype($path) { + return $this->storage->filetype($this->findPathToUse($path)); + } + + /** + * see http://php.net/manual/en/function.filesize.php + * The result for filesize when called on a folder is required to be 0 + * + * @param string $path + * @return int + */ + public function filesize($path) { + return $this->storage->filesize($this->findPathToUse($path)); + } + + /** + * check if a file can be created in $path + * + * @param string $path + * @return bool + */ + public function isCreatable($path) { + return $this->storage->isCreatable($this->findPathToUse($path)); + } + + /** + * check if a file can be read + * + * @param string $path + * @return bool + */ + public function isReadable($path) { + return $this->storage->isReadable($this->findPathToUse($path)); + } + + /** + * check if a file can be written to + * + * @param string $path + * @return bool + */ + public function isUpdatable($path) { + return $this->storage->isUpdatable($this->findPathToUse($path)); + } + + /** + * check if a file can be deleted + * + * @param string $path + * @return bool + */ + public function isDeletable($path) { + return $this->storage->isDeletable($this->findPathToUse($path)); + } + + /** + * check if a file can be shared + * + * @param string $path + * @return bool + */ + public function isSharable($path) { + return $this->storage->isSharable($this->findPathToUse($path)); + } + + /** + * get the full permissions of a path. + * Should return a combination of the PERMISSION_ constants defined in lib/public/constants.php + * + * @param string $path + * @return int + */ + public function getPermissions($path) { + return $this->storage->getPermissions($this->findPathToUse($path)); + } + + /** + * see http://php.net/manual/en/function.file_exists.php + * + * @param string $path + * @return bool + */ + public function file_exists($path) { + return $this->storage->file_exists($this->findPathToUse($path)); + } + + /** + * see http://php.net/manual/en/function.filemtime.php + * + * @param string $path + * @return int + */ + public function filemtime($path) { + return $this->storage->filemtime($this->findPathToUse($path)); + } + + /** + * see http://php.net/manual/en/function.file_get_contents.php + * + * @param string $path + * @return string + */ + public function file_get_contents($path) { + return $this->storage->file_get_contents($this->findPathToUse($path)); + } + + /** + * see http://php.net/manual/en/function.file_put_contents.php + * + * @param string $path + * @param string $data + * @return bool + */ + public function file_put_contents($path, $data) { + $result = $this->storage->file_put_contents($this->findPathToUse($path), $data); + if ($result) { + unset($this->namesCache[$path]); + } + return $result; + } + + /** + * see http://php.net/manual/en/function.unlink.php + * + * @param string $path + * @return bool + */ + public function unlink($path) { + $result = $this->storage->unlink($this->findPathToUse($path)); + if ($result) { + unset($this->namesCache[$path]); + } + return $result; + } + + /** + * see http://php.net/manual/en/function.rename.php + * + * @param string $path1 + * @param string $path2 + * @return bool + */ + public function rename($path1, $path2) { + // second name always NFC + $result = $this->storage->rename($this->findPathToUse($path1), $path2); + if ($result) { + unset($this->namesCache[$path1]); + unset($this->namesCache[$path2]); + } + return $result; + } + + /** + * see http://php.net/manual/en/function.copy.php + * + * @param string $path1 + * @param string $path2 + * @return bool + */ + public function copy($path1, $path2) { + $result = $this->storage->copy($this->findPathToUse($path1), $path2); + if ($result) { + unset($this->namesCache[$path2]); + } + return $result; + } + + /** + * see http://php.net/manual/en/function.fopen.php + * + * @param string $path + * @param string $mode + * @return resource + */ + public function fopen($path, $mode) { + $result = $this->storage->fopen($this->findPathToUse($path), $mode); + if ($result && $mode !== 'r' && $mode !== 'rb') { + unset($this->namesCache[$path]); + } + return $result; + } + + /** + * get the mimetype for a file or folder + * The mimetype for a folder is required to be "httpd/unix-directory" + * + * @param string $path + * @return string + */ + public function getMimeType($path) { + return $this->storage->getMimeType($this->findPathToUse($path)); + } + + /** + * see http://php.net/manual/en/function.hash.php + * + * @param string $type + * @param string $path + * @param bool $raw + * @return string + */ + public function hash($type, $path, $raw = false) { + return $this->storage->hash($type, $this->findPathToUse($path), $raw); + } + + /** + * see http://php.net/manual/en/function.free_space.php + * + * @param string $path + * @return int + */ + public function free_space($path) { + return $this->storage->free_space($this->findPathToUse($path)); + } + + /** + * search for occurrences of $query in file names + * + * @param string $query + * @return array + */ + public function search($query) { + return $this->storage->search($query); + } + + /** + * see http://php.net/manual/en/function.touch.php + * If the backend does not support the operation, false should be returned + * + * @param string $path + * @param int $mtime + * @return bool + */ + public function touch($path, $mtime = null) { + $result = $this->storage->touch($this->findPathToUse($path), $mtime); + if ($result) { + unset($this->namesCache[$path]); + } + return $result; + } + + /** + * get the path to a local version of the file. + * The local version of the file can be temporary and doesn't have to be persistent across requests + * + * @param string $path + * @return string + */ + public function getLocalFile($path) { + return $this->storage->getLocalFile($this->findPathToUse($path)); + } + + /** + * check if a file or folder has been updated since $time + * + * @param string $path + * @param int $time + * @return bool + * + * hasUpdated for folders should return at least true if a file inside the folder is add, removed or renamed. + * returning true for other changes in the folder is optional + */ + public function hasUpdated($path, $time) { + return $this->storage->hasUpdated($this->findPathToUse($path), $time); + } + + /** + * get a cache instance for the storage + * + * @param string $path + * @param \OC\Files\Storage\Storage (optional) the storage to pass to the cache + * @return \OC\Files\Cache\Cache + */ + public function getCache($path = '', $storage = null) { + if (!$storage) { + $storage = $this; + } + return $this->storage->getCache($this->findPathToUse($path), $storage); + } + + /** + * get a scanner instance for the storage + * + * @param string $path + * @param \OC\Files\Storage\Storage (optional) the storage to pass to the scanner + * @return \OC\Files\Cache\Scanner + */ + public function getScanner($path = '', $storage = null) { + if (!$storage) { + $storage = $this; + } + return $this->storage->getScanner($this->findPathToUse($path), $storage); + } + + /** + * get the ETag for a file or folder + * + * @param string $path + * @return string + */ + public function getETag($path) { + return $this->storage->getETag($this->findPathToUse($path)); + } + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + if ($sourceStorage === $this) { + return $this->copy($this->findPathToUse($sourceInternalPath), $targetInternalPath); + } + + $result = $this->storage->copyFromStorage($sourceStorage, $this->findPathToUse($sourceInternalPath), $targetInternalPath); + if ($result) { + unset($this->namesCache[$targetInternalPath]); + } + return $result; + } + + /** + * @param \OCP\Files\Storage $sourceStorage + * @param string $sourceInternalPath + * @param string $targetInternalPath + * @return bool + */ + public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + if ($sourceStorage === $this) { + $result = $this->rename($this->findPathToUse($sourceInternalPath), $targetInternalPath); + if ($result) { + unset($this->namesCache[$sourceInternalPath]); + unset($this->namesCache[$targetInternalPath]); + } + return $result; + } + + $result = $this->storage->moveFromStorage($sourceStorage, $this->findPathToUse($sourceInternalPath), $targetInternalPath); + if ($result) { + unset($this->namesCache[$sourceInternalPath]); + unset($this->namesCache[$targetInternalPath]); + } + return $result; + } + + /** + * @param string $path + * @return array + */ + public function getMetaData($path) { + return $this->storage->getMetaData($this->findPathToUse($path)); + } +} diff --git a/lib/private/legacy/util.php b/lib/private/legacy/util.php index 4f7a8668df..4196aa6637 100644 --- a/lib/private/legacy/util.php +++ b/lib/private/legacy/util.php @@ -172,6 +172,15 @@ class OC_Util { return $storage; }); + // install storage availability wrapper, before most other wrappers + \OC\Files\Filesystem::addStorageWrapper('oc_encoding', function ($mountPoint, $storage) { + // TODO: only do this opt-in if the mount option is specified + if (!$storage->instanceOfStorage('\OC\Files\Storage\Shared') && !$storage->isLocal()) { + return new \OC\Files\Storage\Wrapper\Encoding(['storage' => $storage]); + } + return $storage; + }); + \OC\Files\Filesystem::addStorageWrapper('oc_quota', function ($mountPoint, $storage) { // set up quota for home storages, even for other users // which can happen when using sharing diff --git a/tests/lib/files/storage/wrapper/Encoding.php b/tests/lib/files/storage/wrapper/Encoding.php new file mode 100644 index 0000000000..db0dfa6665 --- /dev/null +++ b/tests/lib/files/storage/wrapper/Encoding.php @@ -0,0 +1,154 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace Test\Files\Storage\Wrapper; + +class Encoding extends \Test\Files\Storage\Storage { + + const NFD_NAME = 'ümlaut'; + const NFC_NAME = 'ümlaut'; + + /** + * @var \OC\Files\Storage\Temporary + */ + private $sourceStorage; + + public function setUp() { + parent::setUp(); + $this->sourceStorage = new \OC\Files\Storage\Temporary([]); + $this->instance = new \OC\Files\Storage\Wrapper\Encoding([ + 'storage' => $this->sourceStorage + ]); + } + + public function tearDown() { + $this->sourceStorage->cleanUp(); + parent::tearDown(); + } + + public function directoryProvider() { + $a = parent::directoryProvider(); + $a[] = [self::NFD_NAME]; + return $a; + } + + public function fileNameProvider() { + $a = parent::fileNameProvider(); + $a[] = [self::NFD_NAME . '.txt']; + return $a; + } + + public function copyAndMoveProvider() { + $a = parent::copyAndMoveProvider(); + $a[] = [self::NFD_NAME . '.txt', self::NFC_NAME . '-renamed.txt']; + return $a; + } + + public function accessNameProvider() { + return [ + [self::NFD_NAME], + [self::NFC_NAME], + ]; + } + + /** + * @dataProvider accessNameProvider + */ + public function testFputEncoding($accessName) { + $this->sourceStorage->file_put_contents(self::NFD_NAME, 'bar'); + $this->assertEquals('bar', $this->instance->file_get_contents($accessName)); + } + + /** + * @dataProvider accessNameProvider + */ + public function testFopenReadEncoding($accessName) { + $this->sourceStorage->file_put_contents(self::NFD_NAME, 'bar'); + $fh = $this->instance->fopen($accessName, 'r'); + $data = fgets($fh); + fclose($fh); + $this->assertEquals('bar', $data); + } + + /** + * @dataProvider accessNameProvider + */ + public function testFopenOverwriteEncoding($accessName) { + $this->sourceStorage->file_put_contents(self::NFD_NAME, 'bar'); + $fh = $this->instance->fopen($accessName, 'w'); + $data = fputs($fh, 'test'); + fclose($fh); + $data = $this->sourceStorage->file_get_contents(self::NFD_NAME); + $this->assertEquals('test', $data); + $this->assertFalse($this->sourceStorage->file_exists(self::NFC_NAME)); + } + + /** + * @dataProvider accessNameProvider + */ + public function testFileExistsEncoding($accessName) { + $this->sourceStorage->file_put_contents(self::NFD_NAME, 'bar'); + $this->assertTrue($this->instance->file_exists($accessName)); + } + + /** + * @dataProvider accessNameProvider + */ + public function testUnlinkEncoding($accessName) { + $this->sourceStorage->file_put_contents(self::NFD_NAME, 'bar'); + $this->assertTrue($this->instance->unlink($accessName)); + $this->assertFalse($this->sourceStorage->file_exists(self::NFC_NAME)); + $this->assertFalse($this->sourceStorage->file_exists(self::NFD_NAME)); + } + + public function testNfcHigherPriority() { + $this->sourceStorage->file_put_contents(self::NFC_NAME, 'nfc'); + $this->sourceStorage->file_put_contents(self::NFD_NAME, 'nfd'); + $this->assertEquals('nfc', $this->instance->file_get_contents(self::NFC_NAME)); + } + + public function encodedDirectoriesProvider() { + return [ + [self::NFD_NAME, self::NFC_NAME], + [self::NFD_NAME . '/' . self::NFD_NAME, self::NFC_NAME . '/' . self::NFC_NAME], + [self::NFD_NAME . '/' . self::NFC_NAME . '/' .self::NFD_NAME, self::NFC_NAME . '/' . self::NFC_NAME . '/' . self::NFC_NAME], + ]; + } + + /** + * @dataProvider encodedDirectoriesProvider + */ + public function testOperationInsideDirectory($sourceDir, $accessDir) { + $this->sourceStorage->mkdir($sourceDir); + $this->instance->file_put_contents($accessDir . '/test.txt', 'bar'); + $this->assertTrue($this->instance->file_exists($accessDir . '/test.txt')); + $this->assertEquals('bar', $this->instance->file_get_contents($accessDir . '/test.txt')); + + $this->sourceStorage->file_put_contents($sourceDir . '/' . self::NFD_NAME, 'foo'); + $this->assertTrue($this->instance->file_exists($accessDir . '/' . self::NFC_NAME)); + $this->assertEquals('foo', $this->instance->file_get_contents($accessDir . '/' . self::NFC_NAME)); + + // try again to make it use cached path + $this->assertEquals('bar', $this->instance->file_get_contents($accessDir . '/test.txt')); + $this->assertTrue($this->instance->file_exists($accessDir . '/test.txt')); + $this->assertEquals('foo', $this->instance->file_get_contents($accessDir . '/' . self::NFC_NAME)); + $this->assertTrue($this->instance->file_exists($accessDir . '/' . self::NFC_NAME)); + } + + public function testCacheExtraSlash() { + $this->sourceStorage->file_put_contents(self::NFD_NAME, 'foo'); + $this->assertEquals(3, $this->instance->file_put_contents(self::NFC_NAME, 'bar')); + $this->assertEquals('bar', $this->instance->file_get_contents(self::NFC_NAME)); + clearstatcache(); + $this->assertEquals(5, $this->instance->file_put_contents('/' . self::NFC_NAME, 'baric')); + $this->assertEquals('baric', $this->instance->file_get_contents(self::NFC_NAME)); + clearstatcache(); + $this->assertEquals(8, $this->instance->file_put_contents('/' . self::NFC_NAME, 'barbaric')); + $this->assertEquals('barbaric', $this->instance->file_get_contents('//' . self::NFC_NAME)); + } +} From db4c7fe743d7cd2019612c9acd732ca0ec4467ea Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 2 May 2016 17:34:24 +0200 Subject: [PATCH 2/5] Add encoding wrapper as opt-in mount option The encoding wrapper is now only applied when the mount option is set, disabled by default. --- apps/files_external/js/settings.js | 32 +++++++++++++------- apps/files_external/templates/settings.php | 1 + apps/files_external/tests/js/settingsSpec.js | 3 +- lib/private/legacy/util.php | 6 ++-- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 55c12cc0ac..91f5f8d811 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -33,8 +33,12 @@ var MOUNT_OPTIONS_DROPDOWN_TEMPLATE = ' ' + ' ' + ' ' + + '
' + + ' ' + + ' ' + + '
' + ''; - + /** * Returns the selection of applicable users in the given configuration row * @@ -476,9 +480,9 @@ MountOptionsDropdown.prototype = { * * @param {Object} $container container * @param {Object} mountOptions mount options - * @param {Array} enabledOptions enabled mount options + * @param {Array} visibleOptions enabled mount options */ - show: function($container, mountOptions, enabledOptions) { + show: function($container, mountOptions, visibleOptions) { if (MountOptionsDropdown._last) { MountOptionsDropdown._last.hide(); } @@ -492,7 +496,7 @@ MountOptionsDropdown.prototype = { var $el = $(template()); this.$el = $el; - this.setOptions(mountOptions, enabledOptions); + this.setOptions(mountOptions, visibleOptions); this.$el.appendTo($container); MountOptionsDropdown._last = this; @@ -538,9 +542,9 @@ MountOptionsDropdown.prototype = { * Sets the mount options to the dropdown controls * * @param {Object} options mount options - * @param {Array} enabledOptions enabled mount options + * @param {Array} visibleOptions enabled mount options */ - setOptions: function(options, enabledOptions) { + setOptions: function(options, visibleOptions) { var $el = this.$el; _.each(options, function(value, key) { var $optionEl = $el.find('input, select').filterAttr('name', key); @@ -556,7 +560,7 @@ MountOptionsDropdown.prototype = { $el.find('.optionRow').each(function(i, row){ var $row = $(row); var optionId = $row.find('input, select').attr('name'); - if (enabledOptions.indexOf(optionId) === -1) { + if (visibleOptions.indexOf(optionId) === -1) { $row.hide(); } else { $row.show(); @@ -883,7 +887,8 @@ MountConfigListView.prototype = _.extend({ 'encrypt': true, 'previews': true, 'enable_sharing': false, - 'filesystem_check_changes': 1 + 'filesystem_check_changes': 1, + 'encoding_compatibility': false })); } @@ -1253,11 +1258,16 @@ MountConfigListView.prototype = _.extend({ var storage = this.getStorageConfig($tr); var $toggle = $tr.find('.mountOptionsToggle'); var dropDown = new MountOptionsDropdown(); - var enabledOptions = ['previews', 'filesystem_check_changes', 'enable_sharing']; + var visibleOptions = [ + 'previews', + 'filesystem_check_changes', + 'enable_sharing', + 'encoding_compatibility' + ]; if (this._encryptionEnabled) { - enabledOptions.push('encrypt'); + visibleOptions.push('encrypt'); } - dropDown.show($toggle, storage.mountOptions || [], enabledOptions); + dropDown.show($toggle, storage.mountOptions || [], visibleOptions); $('body').on('mouseup.mountOptionsDropdown', function(event) { var $target = $(event.target); if ($toggle.has($target).length) { diff --git a/apps/files_external/templates/settings.php b/apps/files_external/templates/settings.php index c9cc40b0ba..16e0e629bd 100644 --- a/apps/files_external/templates/settings.php +++ b/apps/files_external/templates/settings.php @@ -10,6 +10,7 @@ $l->t("Check for changes"); $l->t("Never"); $l->t("Once every direct access"); + $l->t("Enable encoding compatibility (decreases performance)"); script('files_external', 'settings'); style('files_external', 'settings'); diff --git a/apps/files_external/tests/js/settingsSpec.js b/apps/files_external/tests/js/settingsSpec.js index 462407e954..7aa49b2c82 100644 --- a/apps/files_external/tests/js/settingsSpec.js +++ b/apps/files_external/tests/js/settingsSpec.js @@ -370,7 +370,8 @@ describe('OCA.External.Settings tests', function() { encrypt: true, previews: true, enable_sharing: false, - filesystem_check_changes: 0 + filesystem_check_changes: 0, + encoding_compatibility: false }); }); }); diff --git a/lib/private/legacy/util.php b/lib/private/legacy/util.php index 4196aa6637..de97a76224 100644 --- a/lib/private/legacy/util.php +++ b/lib/private/legacy/util.php @@ -172,10 +172,8 @@ class OC_Util { return $storage; }); - // install storage availability wrapper, before most other wrappers - \OC\Files\Filesystem::addStorageWrapper('oc_encoding', function ($mountPoint, $storage) { - // TODO: only do this opt-in if the mount option is specified - if (!$storage->instanceOfStorage('\OC\Files\Storage\Shared') && !$storage->isLocal()) { + \OC\Files\Filesystem::addStorageWrapper('oc_encoding', function ($mountPoint, \OCP\Files\Storage $storage, \OCP\Files\Mount\IMountPoint $mount) { + if ($mount->getOption('encoding_compatibility', true) && !$storage->instanceOfStorage('\OC\Files\Storage\Shared') && !$storage->isLocal()) { return new \OC\Files\Storage\Wrapper\Encoding(['storage' => $storage]); } return $storage; From f8b2b954084c247d2edd70b56b17238fe7a8024e Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 3 May 2016 16:41:54 +0200 Subject: [PATCH 3/5] Scanner must normalize new children names for cache diff Since new children from the storage might contain NFD entries, these must be normalized to NFC to be properly diff'ed with the cache contents which is always NFC. This fixes an issue where NFD entries would disappear from the cache after rescannng for children. --- lib/private/Files/Cache/Scanner.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php index 03aca1924b..3191ba15ed 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -341,7 +341,7 @@ class Scanner extends BasicEmitter implements IScanner { if (is_resource($dh)) { while (($file = readdir($dh)) !== false) { if (!Filesystem::isIgnoredDir($file)) { - $children[] = $file; + $children[] = trim(\OC\Files\Filesystem::normalizePath($file), '/'); } } } From e8d082208d241b3e25150222cacb8d1b4a5b6f12 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 19 May 2016 16:38:28 +0200 Subject: [PATCH 4/5] Fixes for encoding wrapper Improved label Fixed rename/copy/moveFromStorage/copyFromStorage and added tests Improved findPathToUse algo --- apps/files_external/js/settings.js | 6 +- apps/files_external/templates/settings.php | 1 - .../Files/Storage/Wrapper/Encoding.php | 60 ++++++------------- tests/lib/files/storage/wrapper/Encoding.php | 49 +++++++++++++++ 4 files changed, 72 insertions(+), 44 deletions(-) diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 91f5f8d811..8d2cb52d67 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -35,7 +35,7 @@ var MOUNT_OPTIONS_DROPDOWN_TEMPLATE = ' ' + '
' + ' ' + - ' ' + + ' ' + '
' + ''; @@ -493,7 +493,9 @@ MountOptionsDropdown.prototype = { MountOptionsDropdown._template = template; } - var $el = $(template()); + var $el = $(template({ + mountOptionsEncodingLabel: t('files_external', 'Compatibility with Mac NFD encoding (slow)') + })); this.$el = $el; this.setOptions(mountOptions, visibleOptions); diff --git a/apps/files_external/templates/settings.php b/apps/files_external/templates/settings.php index 16e0e629bd..c9cc40b0ba 100644 --- a/apps/files_external/templates/settings.php +++ b/apps/files_external/templates/settings.php @@ -10,7 +10,6 @@ $l->t("Check for changes"); $l->t("Never"); $l->t("Once every direct access"); - $l->t("Enable encoding compatibility (decreases performance)"); script('files_external', 'settings'); style('files_external', 'settings'); diff --git a/lib/private/Files/Storage/Wrapper/Encoding.php b/lib/private/Files/Storage/Wrapper/Encoding.php index 0696f97368..b35773b199 100644 --- a/lib/private/Files/Storage/Wrapper/Encoding.php +++ b/lib/private/Files/Storage/Wrapper/Encoding.php @@ -53,12 +53,7 @@ class Encoding extends Wrapper { * @return bool true if the string is all ASCII, false otherwise */ private function isAscii($str) { - foreach (str_split($str) as $s) { - if (ord($s) > 127) { - return false; - } - } - return true; + return (bool) !preg_match('/[\\x80-\\xff]+/', $str); } /** @@ -79,8 +74,7 @@ class Encoding extends Wrapper { $sections = explode('/', $fullPath); $path = ''; foreach ($sections as $section) { - $path .= $section; - $convertedPath = $this->findPathToUseLastSection($path); + $convertedPath = $this->findPathToUseLastSection($path, $section); if ($convertedPath === null) { // no point in continuing if the section was not found, use original path return $fullPath; @@ -88,7 +82,6 @@ class Encoding extends Wrapper { $path = $convertedPath . '/'; } $path = rtrim($path, '/'); - $this->namesCache[$fullPath] = $path; return $path; } @@ -96,39 +89,28 @@ class Encoding extends Wrapper { * Checks whether the last path section of the given path exists in NFC or NFD form * and returns the correct form. If no existing path found, returns null. * - * @param string $fullPath path to check + * @param string $basePath base path to check + * @param string $lastSection last section of the path to check for NFD/NFC variations * * @return string|null original or converted path, or null if none of the forms was found */ - private function findPathToUseLastSection($fullPath) { - $cachedPath = $this->namesCache[$fullPath]; - if ($cachedPath !== null) { - return $cachedPath; - } - - $sections = explode('/', $fullPath); - $lastSection = array_pop($sections); - $basePath = ltrim(implode('/', $sections) . '/', '/'); - - if ($lastSection === '' || $this->isAscii($lastSection)) { + private function findPathToUseLastSection($basePath, $lastSection) { + $fullPath = $basePath . $lastSection; + if ($lastSection === '' || $this->isAscii($lastSection) || $this->storage->file_exists($fullPath)) { $this->namesCache[$fullPath] = $fullPath; return $fullPath; } - $result = $this->storage->file_exists($fullPath); - if ($result) { - $this->namesCache[$fullPath] = $fullPath; - return $fullPath; - } // swap encoding if (\Normalizer::isNormalized($lastSection, \Normalizer::FORM_C)) { $otherFormPath = \Normalizer::normalize($lastSection, \Normalizer::FORM_D); } else { $otherFormPath = \Normalizer::normalize($lastSection, \Normalizer::FORM_C); } - if ($this->storage->file_exists($basePath . $otherFormPath)) { - $this->namesCache[$fullPath] = $basePath . $otherFormPath; - return $basePath . $otherFormPath; + $otherFullPath = $basePath . $otherFormPath; + if ($this->storage->file_exists($otherFullPath)) { + $this->namesCache[$fullPath] = $otherFullPath; + return $otherFullPath; } // return original path, file did not exist at all @@ -146,7 +128,7 @@ class Encoding extends Wrapper { // note: no conversion here, method should not be called with non-NFC names! $result = $this->storage->mkdir($path); if ($result) { - unset($this->namesCache[$path]); + $this->namesCache[$path] = $path; } return $result; } @@ -326,11 +308,7 @@ class Encoding extends Wrapper { * @return bool */ public function file_put_contents($path, $data) { - $result = $this->storage->file_put_contents($this->findPathToUse($path), $data); - if ($result) { - unset($this->namesCache[$path]); - } - return $result; + return $this->storage->file_put_contents($this->findPathToUse($path), $data); } /** @@ -356,7 +334,7 @@ class Encoding extends Wrapper { */ public function rename($path1, $path2) { // second name always NFC - $result = $this->storage->rename($this->findPathToUse($path1), $path2); + $result = $this->storage->rename($this->findPathToUse($path1), $this->findPathToUse($path2)); if ($result) { unset($this->namesCache[$path1]); unset($this->namesCache[$path2]); @@ -372,7 +350,7 @@ class Encoding extends Wrapper { * @return bool */ public function copy($path1, $path2) { - $result = $this->storage->copy($this->findPathToUse($path1), $path2); + $result = $this->storage->copy($this->findPathToUse($path1), $this->findPathToUse($path2)); if ($result) { unset($this->namesCache[$path2]); } @@ -524,10 +502,10 @@ class Encoding extends Wrapper { */ public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { if ($sourceStorage === $this) { - return $this->copy($this->findPathToUse($sourceInternalPath), $targetInternalPath); + return $this->copy($sourceInternalPath, $this->findPathToUse($targetInternalPath)); } - $result = $this->storage->copyFromStorage($sourceStorage, $this->findPathToUse($sourceInternalPath), $targetInternalPath); + $result = $this->storage->copyFromStorage($sourceStorage, $sourceInternalPath, $this->findPathToUse($targetInternalPath)); if ($result) { unset($this->namesCache[$targetInternalPath]); } @@ -542,7 +520,7 @@ class Encoding extends Wrapper { */ public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { if ($sourceStorage === $this) { - $result = $this->rename($this->findPathToUse($sourceInternalPath), $targetInternalPath); + $result = $this->rename($sourceInternalPath, $this->findPathToUse($targetInternalPath)); if ($result) { unset($this->namesCache[$sourceInternalPath]); unset($this->namesCache[$targetInternalPath]); @@ -550,7 +528,7 @@ class Encoding extends Wrapper { return $result; } - $result = $this->storage->moveFromStorage($sourceStorage, $this->findPathToUse($sourceInternalPath), $targetInternalPath); + $result = $this->storage->moveFromStorage($sourceStorage, $sourceInternalPath, $this->findPathToUse($targetInternalPath)); if ($result) { unset($this->namesCache[$sourceInternalPath]); unset($this->namesCache[$targetInternalPath]); diff --git a/tests/lib/files/storage/wrapper/Encoding.php b/tests/lib/files/storage/wrapper/Encoding.php index db0dfa6665..79b7a3bfb6 100644 --- a/tests/lib/files/storage/wrapper/Encoding.php +++ b/tests/lib/files/storage/wrapper/Encoding.php @@ -151,4 +151,53 @@ class Encoding extends \Test\Files\Storage\Storage { $this->assertEquals(8, $this->instance->file_put_contents('/' . self::NFC_NAME, 'barbaric')); $this->assertEquals('barbaric', $this->instance->file_get_contents('//' . self::NFC_NAME)); } + + public function sourceAndTargetDirectoryProvider() { + return [ + [self::NFC_NAME . '1', self::NFC_NAME . '2'], + [self::NFD_NAME . '1', self::NFC_NAME . '2'], + [self::NFC_NAME . '1', self::NFD_NAME . '2'], + [self::NFD_NAME . '1', self::NFD_NAME . '2'], + ]; + } + + /** + * @dataProvider sourceAndTargetDirectoryProvider + */ + public function testCopyAndMoveEncodedFolder($sourceDir, $targetDir) { + $this->sourceStorage->mkdir($sourceDir); + $this->sourceStorage->mkdir($targetDir); + $this->sourceStorage->file_put_contents($sourceDir . '/test.txt', 'bar'); + $this->assertTrue($this->instance->copy(self::NFC_NAME . '1/test.txt', self::NFC_NAME . '2/test.txt')); + + $this->assertTrue($this->instance->file_exists(self::NFC_NAME . '1/test.txt')); + $this->assertTrue($this->instance->file_exists(self::NFC_NAME . '2/test.txt')); + $this->assertEquals('bar', $this->instance->file_get_contents(self::NFC_NAME . '2/test.txt')); + + $this->assertTrue($this->instance->rename(self::NFC_NAME . '1/test.txt', self::NFC_NAME . '2/test2.txt')); + $this->assertFalse($this->instance->file_exists(self::NFC_NAME . '1/test.txt')); + $this->assertTrue($this->instance->file_exists(self::NFC_NAME . '2/test2.txt')); + + $this->assertEquals('bar', $this->instance->file_get_contents(self::NFC_NAME . '2/test2.txt')); + } + + /** + * @dataProvider sourceAndTargetDirectoryProvider + */ + public function testCopyAndMoveFromStorageEncodedFolder($sourceDir, $targetDir) { + $this->sourceStorage->mkdir($sourceDir); + $this->sourceStorage->mkdir($targetDir); + $this->sourceStorage->file_put_contents($sourceDir . '/test.txt', 'bar'); + $this->assertTrue($this->instance->copyFromStorage($this->instance, self::NFC_NAME . '1/test.txt', self::NFC_NAME . '2/test.txt')); + + $this->assertTrue($this->instance->file_exists(self::NFC_NAME . '1/test.txt')); + $this->assertTrue($this->instance->file_exists(self::NFC_NAME . '2/test.txt')); + $this->assertEquals('bar', $this->instance->file_get_contents(self::NFC_NAME . '2/test.txt')); + + $this->assertTrue($this->instance->moveFromStorage($this->instance, self::NFC_NAME . '1/test.txt', self::NFC_NAME . '2/test2.txt')); + $this->assertFalse($this->instance->file_exists(self::NFC_NAME . '1/test.txt')); + $this->assertTrue($this->instance->file_exists(self::NFC_NAME . '2/test2.txt')); + + $this->assertEquals('bar', $this->instance->file_get_contents(self::NFC_NAME . '2/test2.txt')); + } } From bac8e13324f888e3d23851528943c9ae9f34cf12 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 19 May 2016 16:49:44 +0200 Subject: [PATCH 5/5] Remove unneeded unsets in encoding wrapper --- .../Files/Storage/Wrapper/Encoding.php | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/lib/private/Files/Storage/Wrapper/Encoding.php b/lib/private/Files/Storage/Wrapper/Encoding.php index b35773b199..0171dfe19b 100644 --- a/lib/private/Files/Storage/Wrapper/Encoding.php +++ b/lib/private/Files/Storage/Wrapper/Encoding.php @@ -334,12 +334,7 @@ class Encoding extends Wrapper { */ public function rename($path1, $path2) { // second name always NFC - $result = $this->storage->rename($this->findPathToUse($path1), $this->findPathToUse($path2)); - if ($result) { - unset($this->namesCache[$path1]); - unset($this->namesCache[$path2]); - } - return $result; + return $this->storage->rename($this->findPathToUse($path1), $this->findPathToUse($path2)); } /** @@ -350,11 +345,7 @@ class Encoding extends Wrapper { * @return bool */ public function copy($path1, $path2) { - $result = $this->storage->copy($this->findPathToUse($path1), $this->findPathToUse($path2)); - if ($result) { - unset($this->namesCache[$path2]); - } - return $result; + return $this->storage->copy($this->findPathToUse($path1), $this->findPathToUse($path2)); } /** @@ -424,11 +415,7 @@ class Encoding extends Wrapper { * @return bool */ public function touch($path, $mtime = null) { - $result = $this->storage->touch($this->findPathToUse($path), $mtime); - if ($result) { - unset($this->namesCache[$path]); - } - return $result; + return $this->storage->touch($this->findPathToUse($path), $mtime); } /**