Merge pull request #10619 from owncloud/issue/6722

Add a test to break the slugifyPath() with folder and file afterwards

lib/private/files/mapper.php

@@ -183,8 +183,6 @@ class Mapper
 		$pathElements = explode('/', $path);
 		$sluggedElements = array();
 
-		$last= end($pathElements);
-		
 		foreach ($pathElements as $pathElement) {
 			// remove empty elements
 			if (empty($pathElement)) {
@@ -205,7 +203,6 @@ class Mapper
 				// if filename doesn't contain periods add index ofter the last char
 				array_push($sluggedElements, $last . '-' . $index);
 			}
-
 		}
 
 		$sluggedPath = $this->unchangedPhysicalRoot . implode('/', $sluggedElements);
@@ -218,8 +215,8 @@ class Mapper
 	 * @param string $text
 	 * @return string
 	 */
-	private function slugify($text)
-	{
+	private function slugify($text) {
+		$originalText = $text;
 		// replace non letter or digits or dots by -
 		$text = preg_replace('~[^\\pL\d\.]+~u', '-', $text);
 
@@ -241,7 +238,17 @@ class Mapper
 		$text = preg_replace('~\.+$~', '', $text);
 
 		if (empty($text)) {
-			return uniqid();
+			/**
+			 * Item slug would be empty. Previously we used uniqid() here.
+			 * However this means that the behaviour is not reproducible, so
+			 * when uploading files into a "empty" folder, the folders name is
+			 * different.
+			 *
+			 * If there would be a md5() hash collision, the deduplicate check
+			 * will spot this and append an index later, so this should not be
+			 * a problem.
+			 */
+			return md5($originalText);
 		}
 
 		return $text;

tests/lib/files/mapper.php

@@ -33,32 +33,47 @@ class Mapper extends \PHPUnit_Framework_TestCase {
 		$this->mapper = new \OC\Files\Mapper('D:/');
 	}
 
-	public function testSlugifyPath() {
+	public function slugifyPathData() {
+		return array(
 			// with extension
-		$this->assertEquals('D:/text.txt', $this->mapper->slugifyPath('D:/text.txt'));
-		$this->assertEquals('D:/text-2.txt', $this->mapper->slugifyPath('D:/text.txt', 2));
-		$this->assertEquals('D:/a/b/text.txt', $this->mapper->slugifyPath('D:/a/b/text.txt'));
+			array('D:/text.txt', 'D:/text.txt'),
+			array('D:/text-2.txt', 'D:/text.txt', 2),
+			array('D:/a/b/text.txt', 'D:/a/b/text.txt'),
 
 			// without extension
-		$this->assertEquals('D:/text', $this->mapper->slugifyPath('D:/text'));
-		$this->assertEquals('D:/text-2', $this->mapper->slugifyPath('D:/text', 2));
-		$this->assertEquals('D:/a/b/text', $this->mapper->slugifyPath('D:/a/b/text'));
+			array('D:/text', 'D:/text'),
+			array('D:/text-2', 'D:/text', 2),
+			array('D:/a/b/text', 'D:/a/b/text'),
 
 			// with double dot
-		$this->assertEquals('D:/text.text.txt', $this->mapper->slugifyPath('D:/text.text.txt'));
-		$this->assertEquals('D:/text.text-2.txt', $this->mapper->slugifyPath('D:/text.text.txt', 2));
-		$this->assertEquals('D:/a/b/text.text.txt', $this->mapper->slugifyPath('D:/a/b/text.text.txt'));
+			array('D:/text.text.txt', 'D:/text.text.txt'),
+			array('D:/text.text-2.txt', 'D:/text.text.txt', 2),
+			array('D:/a/b/text.text.txt', 'D:/a/b/text.text.txt'),
 
 			// foldername and filename with periods
-		$this->assertEquals('D:/folder.name.with.periods', $this->mapper->slugifyPath('D:/folder.name.with.periods'));
-		$this->assertEquals('D:/folder.name.with.periods/test-2.txt', $this->mapper->slugifyPath('D:/folder.name.with.periods/test.txt', 2));
-		$this->assertEquals('D:/folder.name.with.periods/test.txt', $this->mapper->slugifyPath('D:/folder.name.with.periods/test.txt'));
+			array('D:/folder.name.with.periods', 'D:/folder.name.with.periods'),
+			array('D:/folder.name.with.periods/test-2.txt', 'D:/folder.name.with.periods/test.txt', 2),
+			array('D:/folder.name.with.periods/test.txt', 'D:/folder.name.with.periods/test.txt'),
 
 			// foldername and filename with periods and spaces
-		$this->assertEquals('D:/folder.name.with.peri-ods', $this->mapper->slugifyPath('D:/folder.name.with.peri ods'));
-		$this->assertEquals('D:/folder.name.with.peri-ods/te-st-2.t-x-t', $this->mapper->slugifyPath('D:/folder.name.with.peri ods/te st.t x t', 2));
-		$this->assertEquals('D:/folder.name.with.peri-ods/te-st.t-x-t', $this->mapper->slugifyPath('D:/folder.name.with.peri ods/te st.t x t'));
+			array('D:/folder.name.with.peri-ods', 'D:/folder.name.with.peri ods'),
+			array('D:/folder.name.with.peri-ods/te-st-2.t-x-t', 'D:/folder.name.with.peri ods/te st.t x t', 2),
+			array('D:/folder.name.with.peri-ods/te-st.t-x-t', 'D:/folder.name.with.peri ods/te st.t x t'),
 
+			/**
+			 * If a foldername is empty, after we stripped out some unicode and other characters,
+			 * the resulting name must be reproducable otherwise uploading a file into that folder
+			 * will not write the file into the same folder.
+			 */
+			array('D:/' . md5('ありがとう'), 'D:/ありがとう'),
+			array('D:/' . md5('ありがとう') . '/issue6722.txt', 'D:/ありがとう/issue6722.txt'),
+		);
+	}
 
+	/**
+	 * @dataProvider slugifyPathData
+	 */
+	public function testSlugifyPath($slug, $path, $index = null) {
+		$this->assertEquals($slug, $this->mapper->slugifyPath($path, $index));
 	}
 }