Merge pull request #3459 from owncloud/fix_for_2377

fix problems with german "Umlaut" in folder name

3rdparty

@@ -1 +1 @@
-Subproject commit a13af72fbe8983686fc47489a750e60319f68ac2
+Subproject commit e312294ef62873df2b8c02e774f9dfe1b7fbc38d

lib/files/cache/cache.php

@@ -100,6 +100,9 @@ class Cache {
 	 */
 	public function get($file) {
 		if (is_string($file) or $file == '') {
+			// normalize file
+			$file = $this->normalize($file);
+
 			$where = 'WHERE `storage` = ? AND `path_hash` = ?';
 			$params = array($this->getNumericStorageId(), md5($file));
 		} else { //file id
@@ -179,6 +182,9 @@ class Cache {
 			$this->update($id, $data);
 			return $id;
 		} else {
+			// normalize file
+			$file = $this->normalize($file);
+
 			if (isset($this->partial[$file])) { //add any saved partial data
 				$data = array_merge($this->partial[$file], $data);
 				unset($this->partial[$file]);
@@ -220,6 +226,17 @@ class Cache {
 	 * @param array $data
 	 */
 	public function update($id, array $data) {
+
+		if(isset($data['path'])) {
+			// normalize path
+			$data['path'] = $this->normalize($data['path']);
+		}
+
+		if(isset($data['name'])) {
+			// normalize path
+			$data['name'] = $this->normalize($data['name']);
+		}
+
 		list($queryParts, $params) = $this->buildParts($data);
 		$params[] = $id;
 
@@ -267,6 +284,9 @@ class Cache {
 	 * @return int
 	 */
 	public function getId($file) {
+		// normalize file
+		$file = $this->normalize($file);
+
 		$pathHash = md5($file);
 
 		$query = \OC_DB::prepare('SELECT `fileid` FROM `*PREFIX*filecache` WHERE `storage` = ? AND `path_hash` = ?');
@@ -334,6 +354,10 @@ class Cache {
 	 * @param string $target
 	 */
 	public function move($source, $target) {
+		// normalize source and target
+		$source = $this->normalize($source);
+		$target = $this->normalize($target);
+
 		$sourceData = $this->get($source);
 		$sourceId = $sourceData['fileid'];
 		$newParentId = $this->getParentId($target);
@@ -374,6 +398,9 @@ class Cache {
 	 * @return int, Cache::NOT_FOUND, Cache::PARTIAL, Cache::SHALLOW or Cache::COMPLETE
 	 */
 	public function getStatus($file) {
+		// normalize file
+		$file = $this->normalize($file);
+
 		$pathHash = md5($file);
 		$query = \OC_DB::prepare('SELECT `size` FROM `*PREFIX*filecache` WHERE `storage` = ? AND `path_hash` = ?');
 		$result = $query->execute(array($this->getNumericStorageId(), $pathHash));
@@ -402,6 +429,10 @@ class Cache {
 	 * @return array of file data
 	 */
 	public function search($pattern) {
+
+		// normalize pattern
+		$pattern = $this->normalize($pattern);
+
 		$query = \OC_DB::prepare('
 			SELECT `fileid`, `storage`, `path`, `parent`, `name`, `mimetype`, `mimepart`, `size`, `mtime`, `encrypted`, `unencrypted_size`, `etag`
 			FROM `*PREFIX*filecache` WHERE `name` LIKE ? AND `storage` = ?'
@@ -551,4 +582,14 @@ class Cache {
 			return null;
 		}
 	}
+
+	/**
+	 * normalize the given path
+	 * @param $path
+	 * @return string
+	 */
+	public function normalize($path) {
+
+		return \OC_Util::normalizeUnicode($path);
+	}
 }

lib/files/filesystem.php

@@ -631,9 +631,8 @@ class Filesystem {
 			$path = substr($path, 0, -1);
 		}
 		//normalize unicode if possible
-		if (class_exists('Normalizer')) {
-			$path = \Normalizer::normalize($path);
-		}
+		$path = \OC_Util::normalizeUnicode($path);
+
 		return $path;
 	}
 

lib/util.php

@@ -1,4 +1,7 @@
 <?php
+
+require_once 'Patchwork/PHP/Shim/Normalizer.php';
+
 /**
  * Class for utility functions
  *
@@ -823,5 +826,21 @@ class OC_Util {
 		return $theme;
 	}
 
+	/**
+	 * Normalize a unicode string
+	 * @param string $value a not normalized string
+	 * @return bool|string
+	 */
+	public static function normalizeUnicode($value) {
+		if(class_exists('Patchwork\PHP\Shim\Normalizer')) {
+			$normalizedValue = \Patchwork\PHP\Shim\Normalizer::normalize($value);
+			if($normalizedValue === false) {
+				\OC_Log::write( 'core', 'normalizing failed for "' . $value . '"', \OC_Log::WARN);
+			} else {
+				$value = $normalizedValue;
+			}
+		}
 
+		return $value;
+	}
 }

tests/lib/files/cache/cache.php

@@ -8,6 +8,8 @@
 
 namespace Test\Files\Cache;
 
+use PHPUnit_Framework_MockObject_MockObject;
+
 class LongId extends \OC\Files\Storage\Temporary {
 	public function getId() {
 		return 'long:' . str_repeat('foo', 50) . parent::getId();
@@ -262,6 +264,89 @@ class Cache extends \PHPUnit_Framework_TestCase {
 		$this->assertEquals(array(md5($storageId), 'foo'), \OC\Files\Cache\Cache::getById($id));
 	}
 
+	/**
+	 * @brief this test show the bug resulting if we have no normalizer installed
+	 */
+	public function testWithoutNormalizer() {
+		// folder name "Schön" with U+00F6 (normalized)
+		$folderWith00F6 = "\x53\x63\x68\xc3\xb6\x6e";
+
+		// folder name "Schön" with U+0308 (un-normalized)
+		$folderWith0308 = "\x53\x63\x68\x6f\xcc\x88\x6e";
+
+		/**
+		 * @var \OC\Files\Cache\Cache | PHPUnit_Framework_MockObject_MockObject $cacheMock
+		 */
+		$cacheMock = $this->getMock('\OC\Files\Cache\Cache', array('normalize'), array($this->storage), '', true);
+
+		$cacheMock->expects($this->any())
+			->method('normalize')
+			->will($this->returnArgument(0));
+
+		$data = array('size' => 100, 'mtime' => 50, 'mimetype' => 'httpd/unix-directory');
+
+		// put root folder
+		$this->assertFalse($cacheMock->get('folder'));
+		$this->assertGreaterThan(0, $cacheMock->put('folder', $data));
+
+		// put un-normalized folder
+		$this->assertFalse($cacheMock->get('folder/' .$folderWith0308));
+		$this->assertGreaterThan(0, $cacheMock->put('folder/' .$folderWith0308, $data));
+
+		// get un-normalized folder by name
+		$unNormalizedFolderName = $cacheMock->get('folder/' .$folderWith0308);
+
+		// check if database layer normalized the folder name (this should not happen)
+		$this->assertEquals($folderWith0308, $unNormalizedFolderName['name']);
+
+		// put normalized folder
+		$this->assertFalse($cacheMock->get('folder/' . $folderWith00F6));
+		$this->assertGreaterThan(0, $cacheMock->put('folder/' .$folderWith00F6, $data));
+
+		// this is our bug, we have two different hashes with the same name (Schön)
+		$this->assertEquals(2, count($cacheMock->getFolderContents('folder')));
+	}
+
+	/**
+	 * @brief this test shows that there is no bug if we use the normalizer
+	 */
+	public function testWithNormalizer() {
+
+		if(!class_exists('Patchwork\PHP\Shim\Normalizer')) {
+			$this->markTestSkipped('The 3rdparty Normalizer extension is not available.');
+			return;
+		}
+
+		// folder name "Schön" with U+00F6 (normalized)
+		$folderWith00F6 = "\x53\x63\x68\xc3\xb6\x6e";
+
+		// folder name "Schön" with U+0308 (un-normalized)
+		$folderWith0308 = "\x53\x63\x68\x6f\xcc\x88\x6e";
+
+		$data = array('size' => 100, 'mtime' => 50, 'mimetype' => 'httpd/unix-directory');
+
+		// put root folder
+		$this->assertFalse($this->cache->get('folder'));
+		$this->assertGreaterThan(0, $this->cache->put('folder', $data));
+
+		// put un-normalized folder
+		$this->assertFalse($this->cache->get('folder/' .$folderWith0308));
+		$this->assertGreaterThan(0, $this->cache->put('folder/' .$folderWith0308, $data));
+
+		// get un-normalized folder by name
+		$unNormalizedFolderName = $this->cache->get('folder/' .$folderWith0308);
+
+		// check if folder name was normalized
+		$this->assertEquals($folderWith00F6, $unNormalizedFolderName['name']);
+
+		// put normalized folder
+		$this->assertTrue(is_array($this->cache->get('folder/' . $folderWith00F6)));
+		$this->assertGreaterThan(0, $this->cache->put('folder/' .$folderWith00F6, $data));
+
+		// at this point we should have only one folder named "Schön"
+		$this->assertEquals(1, count($this->cache->getFolderContents('folder')));
+	}
+
 	public function tearDown() {
 		$this->cache->clear();
 	}

tests/lib/files/filesystem.php

@@ -72,7 +72,7 @@ class Filesystem extends \PHPUnit_Framework_TestCase {
 		$this->assertEquals('/path', \OC\Files\Filesystem::normalizePath('\path'));
 		$this->assertEquals('/foo/bar', \OC\Files\Filesystem::normalizePath('/foo//bar/'));
 		$this->assertEquals('/foo/bar', \OC\Files\Filesystem::normalizePath('/foo////bar'));
-		if (class_exists('Normalizer')) {
+		if (class_exists('Patchwork\PHP\Shim\Normalizer')) {
 			$this->assertEquals("/foo/bar\xC3\xBC", \OC\Files\Filesystem::normalizePath("/foo/baru\xCC\x88"));
 		}
 	}