[ticket/13904] Set visibility in files and improve test coverage

PHPBB3-13904

3 files changed, 64 insertions, 11 deletions

diff --git a/tests/files/upload_test.php b/tests/files/upload_test.php
index 057bb8a4f2..dc0080d25c 100644
--- a/tests/files/upload_test.php
+++ b/tests/files/upload_test.php
@@ -76,16 +76,19 @@ class phpbb_files_upload_test extends phpbb_test_case
 	public function test_set_disallowed_content()
 	{
 		$upload = new \phpbb\files\upload($this->filesystem, $this->factory, $this->language, $this->request, $this->phpbb_root_path);
+		$disallowed_content = new ReflectionProperty($upload, 'disallowed_content');
+		$disallowed_content->setAccessible(true);
+
 		$upload->set_disallowed_content(array('foo'));
-		$this->assertEquals(array('foo'), $upload->disallowed_content);
+		$this->assertEquals(array('foo'), $disallowed_content->getValue($upload));
 		$upload->set_disallowed_content(array('foo', 'bar', 'meh'));
-		$this->assertEquals(array('foo', 'bar', 'meh'), $upload->disallowed_content);
+		$this->assertEquals(array('foo', 'bar', 'meh'), $disallowed_content->getValue($upload));
 		$upload->set_disallowed_content('');
-		$this->assertEquals(array('foo', 'bar', 'meh'), $upload->disallowed_content);
+		$this->assertEquals(array('foo', 'bar', 'meh'), $disallowed_content->getValue($upload));
 		$this->assertINstanceOf('\phpbb\files\upload', $upload->set_disallowed_content(array()));
-		$this->assertEquals(array(), $upload->disallowed_content);
+		$this->assertEquals(array(), $disallowed_content->getValue($upload));
 		$upload->reset_vars();
-		$this->assertEquals(array(), $upload->disallowed_content);
+		$this->assertEquals(array(), $disallowed_content->getValue($upload));
 	}
 
 	public function test_is_valid()
diff --git a/tests/functional/fileupload_remote_test.php b/tests/functional/fileupload_remote_test.php
index a754d8c91d..a6aa233aaf 100644
--- a/tests/functional/fileupload_remote_test.php
+++ b/tests/functional/fileupload_remote_test.php
@@ -98,7 +98,7 @@ class phpbb_functional_fileupload_remote_test extends phpbb_functional_test_case
 			->set_max_filesize(1000);
 		$file = $upload->handle_upload('remote', self::$root_url . 'styles/prosilver/theme/images/forum_read.gif');
 		$this->assertEquals(0, sizeof($file->error));
-		$this->assertTrue(file_exists($file->filename));
+		$this->assertTrue(file_exists($file->get('filename')));
 	}
 
 	public function test_too_large()
diff --git a/tests/upload/filespec_test.php b/tests/upload/filespec_test.php
index b0df72a6ff..f885b1acfc 100644
--- a/tests/upload/filespec_test.php
+++ b/tests/upload/filespec_test.php
@@ -133,7 +133,7 @@ class phpbb_filespec_test extends phpbb_test_case
 	{
 		$upload = new phpbb_mock_fileupload();
 		$filespec = $this->get_filespec();
-		$filespec->upload = $upload;
+		$filespec->set_upload_namespace($upload);
 		$filespec->file_moved = true;
 		$filespec->filesize = $filespec->get_filesize($this->path . $filename);
 
@@ -174,6 +174,7 @@ class phpbb_filespec_test extends phpbb_test_case
 			array($chunks[2] . $chunks[9]),
 			array($chunks[3] . $chunks[4]),
 			array($chunks[5] . $chunks[6]),
+			array('foobar.png'),
 		);
 	}
 
@@ -185,7 +186,7 @@ class phpbb_filespec_test extends phpbb_test_case
 		$bad_chars = array("'", "\\", ' ', '/', ':', '*', '?', '"', '<', '>', '|');
 		$filespec = $this->get_filespec(array('name' => $filename));
 		$filespec->clean_filename('real', self::PREFIX);
-		$name = $filespec->realname;
+		$name = $filespec->get('realname');
 
 		$this->assertEquals(0, preg_match('/%(\w{2})/', $name));
 		foreach ($bad_chars as $char)
@@ -201,7 +202,7 @@ class phpbb_filespec_test extends phpbb_test_case
 		{
 			$filespec = $this->get_filespec();
 			$filespec->clean_filename('unique', self::PREFIX);
-			$name = $filespec->realname;
+			$name = $filespec->get('realname');
 
 			$this->assertEquals(strlen($name), 32 + strlen(self::PREFIX));
 			$this->assertRegExp('#^[A-Za-z0-9]+$#', substr($name, strlen(self::PREFIX)));
@@ -210,6 +211,55 @@ class phpbb_filespec_test extends phpbb_test_case
 		}
 	}
 
+	public function test_clean_filename_unique_ext()
+	{
+		$filenames = array();
+		for ($tests = 0; $tests < self::TEST_COUNT; $tests++)
+		{
+			$filespec = $this->get_filespec(array('name' => 'foobar.jpg'));
+			$filespec->clean_filename('unique_ext', self::PREFIX);
+			$name = $filespec->get('realname');
+
+			$this->assertEquals(strlen($name), 32 + strlen(self::PREFIX) + strlen('.jpg'));
+			$this->assertRegExp('#^[A-Za-z0-9]+\.jpg$#', substr($name, strlen(self::PREFIX)));
+			$this->assertFalse(isset($filenames[$name]));
+			$filenames[$name] = true;
+		}
+	}
+
+	public function data_clean_filename_avatar()
+	{
+		return array(
+			array(false, false, ''),
+			array('foobar.png', 'u5.png', 'avatar', 'u', 5),
+			array('foobar.png', 'g9.png', 'avatar', 'g', 9),
+
+		);
+	}
+
+	/**
+	 * @dataProvider data_clean_filename_avatar
+	 */
+	public function test_clean_filename_avatar($filename, $expected, $mode, $prefix = '', $user_id = '')
+	{
+		$filespec = new \phpbb\files\filespec($this->filesystem, $this->language, $this->phpbb_root_path, $this->mimetype_guesser);
+
+		if ($filename)
+		{
+			$upload_ary = array(
+				'name' => $filename,
+				'type' => '',
+				'size' => '',
+				'tmp_name' => '',
+				'error' => '',
+			);
+			$filespec->set_upload_ary($upload_ary);
+		}
+		$filespec->clean_filename($mode, $prefix, $user_id);
+
+		$this->assertSame($expected, $filespec->get('realname'));
+	}
+
 	public function get_extension_variables()
 	{
 		return array(
@@ -312,7 +362,7 @@ class phpbb_filespec_test extends phpbb_test_case
 			'type' => $mime_type,
 		));
 		$filespec->extension = $extension;
-		$filespec->upload = $upload;
+		$filespec->set_upload_namespace($upload);
 		$filespec->local = true;
 
 		$this->assertEquals($expected, $filespec->move_file($this->path . 'copies'));
@@ -336,6 +386,6 @@ class phpbb_filespec_test extends phpbb_test_case
 		$type_cast_helper->set_var($upload_name, $filename, 'string', true, true);
 		$filespec = $this->get_filespec(array('name'=> $upload_name));
 
-		$this->assertSame(trim(utf8_basename(htmlspecialchars($filename))), $filespec->uploadname);
+		$this->assertSame(trim(utf8_basename(htmlspecialchars($filename))), $filespec->get('uploadname'));
 	}
 }