From 580cec619b399f710aedc1d3c2dd92c2287ebaff Mon Sep 17 00:00:00 2001 From: Fyorl Date: Sun, 8 Jul 2012 14:39:18 +0100 Subject: [ticket/10941] Added subdirectory for file operations Also removed common.php as it was unnecessary. PHPBB3-10941 --- tests/upload/filespec_test.php | 272 +++++++++++++++++++++++++++++++++++++++ tests/upload/fileupload_test.php | 116 +++++++++++++++++ tests/upload/fixture/gif | Bin 0 -> 35 bytes tests/upload/fixture/jpg | Bin 0 -> 519 bytes tests/upload/fixture/png | Bin 0 -> 69 bytes tests/upload/fixture/tif | Bin 0 -> 256 bytes tests/upload/fixture/txt | 1 + 7 files changed, 389 insertions(+) create mode 100644 tests/upload/filespec_test.php create mode 100644 tests/upload/fileupload_test.php create mode 100644 tests/upload/fixture/gif create mode 100644 tests/upload/fixture/jpg create mode 100644 tests/upload/fixture/png create mode 100644 tests/upload/fixture/tif create mode 100644 tests/upload/fixture/txt (limited to 'tests/upload') diff --git a/tests/upload/filespec_test.php b/tests/upload/filespec_test.php new file mode 100644 index 0000000000..7961e8ed64 --- /dev/null +++ b/tests/upload/filespec_test.php @@ -0,0 +1,272 @@ +lang = new phpbb_mock_lang(); + + $this->config = &$config; + $this->path = __DIR__ . '/fixture/'; + + // Create copies of the files for use in testing move_file + $iterator = new DirectoryIterator($this->path); + foreach ($iterator as $fileinfo) + { + if ($fileinfo->isDot() || $fileinfo->isDir()) + { + continue; + } + + copy($fileinfo->getPathname(), $this->path . 'copies/' . $fileinfo->getFilename() . '_copy'); + if ($fileinfo->getFilename() === 'txt') + { + copy($fileinfo->getPathname(), $this->path . 'copies/' . $fileinfo->getFilename() . '_copy_2'); + } + } + } + + private function get_filespec($override = array()) + { + // Initialise a blank filespec object for use with trivial methods + $upload_ary = array( + 'name' => '', + 'type' => '', + 'size' => '', + 'tmp_name' => '', + 'error' => '', + ); + + return new filespec(array_merge($upload_ary, $override), null); + } + + protected function tearDown() + { + global $user; + $this->config = array(); + $user = null; + + foreach (glob($this->path . 'copies/*') as $file) + { + unlink($file); + } + } + + public function additional_checks_variables() + { + // False here just indicates the file is too large and fails the + // filespec::additional_checks method because of it. All other code + // paths in that method are covered elsewhere. + return array( + array('gif', true), + array('jpg', false), + array('png', true), + array('tif', false), + array('txt', true), + ); + } + + /** + * @dataProvider additional_checks_variables + */ + public function test_additional_checks($filename, $expected) + { + $upload = new phpbb_mock_fileupload(); + $filespec = $this->get_filespec(array('tmp_name', $this->path . $filename)); + $filespec->upload = $upload; + $filespec->file_moved = true; + $filespec->filesize = $filespec->get_filesize($this->path . $filename); + + $this->assertEquals($expected, $filespec->additional_checks()); + } + + public function check_content_variables() + { + // False here indicates that a file is non-binary and contains + // disallowed content that makes IE report the mimetype incorrectly. + return array( + array('gif', true), + array('jpg', true), + array('png', true), + array('tif', true), + array('txt', false), + ); + } + + /** + * @dataProvider check_content_variables + */ + public function test_check_content($filename, $expected) + { + $disallowed_content = explode('|', $this->config['mime_triggers']); + $filespec = $this->get_filespec(array('tmp_name' => $this->path . $filename)); + $this->assertEquals($expected, $filespec->check_content($disallowed_content)); + } + + public function clean_filename_variables() + { + $chunks = str_split('abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ\'\\" /:*?<>|[];(){},#+=-_`', 8); + return array( + array($chunks[0] . $chunks[7]), + array($chunks[1] . $chunks[8]), + array($chunks[2] . $chunks[9]), + array($chunks[3] . $chunks[4]), + array($chunks[5] . $chunks[6]), + ); + } + + /** + * @dataProvider clean_filename_variables + */ + public function test_clean_filename_real($filename) + { + $bad_chars = array("'", "\\", ' ', '/', ':', '*', '?', '"', '<', '>', '|'); + $filespec = $this->get_filespec(array('name' => $filename)); + $filespec->clean_filename('real', self::PREFIX); + $name = $filespec->realname; + + $this->assertEquals(0, preg_match('/%(\w{2})/', $name)); + foreach ($bad_chars as $char) + { + $this->assertFalse(strpos($name, $char)); + } + } + + public function test_clean_filename_unique() + { + $filenames = array(); + for ($tests = 0; $tests < self::TEST_COUNT; $tests++) + { + $filespec = $this->get_filespec(); + $filespec->clean_filename('unique', self::PREFIX); + $name = $filespec->realname; + + $this->assertEquals(strlen($name), 32 + strlen(self::PREFIX)); + $this->assertRegExp('#^[A-Za-z0-9]+$#', substr($name, strlen(self::PREFIX))); + $this->assertFalse(isset($filenames[$name])); + $filenames[$name] = true; + } + } + + public function get_extension_variables() + { + return array( + array('file.png', 'png'), + array('file.phpbb.gif', 'gif'), + array('file..', ''), + array('.file..jpg.webp', 'webp'), + ); + } + + /** + * @dataProvider get_extension_variables + */ + public function test_get_extension($filename, $expected) + { + $filespec = $this->get_filespec(); + $this->assertEquals($expected, $filespec->get_extension($filename)); + } + + public function is_image_variables() + { + return array( + array('gif', 'image/gif', true), + array('jpg', 'image/jpg', true), + array('png', 'image/png', true), + array('tif', 'image/tif', true), + array('txt', 'text/plain', false), + ); + } + + /** + * @dataProvider is_image_variables + */ + public function test_is_image($filename, $mimetype, $expected) + { + $filespec = $this->get_filespec(array('tmp_name' => $this->path . $filename, 'type' => $mimetype)); + $this->assertEquals($expected, $filespec->is_image()); + } + + public function move_file_variables() + { + return array( + array('gif_copy', 'gif_moved', 'image/gif', 'gif', false, true), + array('non_existant', 'still_non_existant', 'text/plain', 'txt', 'GENERAL_UPLOAD_ERROR', false), + array('txt_copy', 'txt_as_img', 'image/jpg', 'txt', false, true), + array('txt_copy_2', 'txt_moved', 'text/plain', 'txt', false, true), + array('jpg_copy', 'jpg_moved', 'image/png', 'jpg', false, true), + array('png_copy', 'png_moved', 'image/png', 'jpg', 'IMAGE_FILETYPE_MISMATCH', true), + ); + } + + /** + * @dataProvider move_file_variables + */ + public function test_move_file($tmp_name, $realname, $mime_type, $extension, $error, $expected) + { + // Global $phpbb_root_path and $phpEx are required by phpbb_chmod + global $phpbb_root_path, $phpEx; + $phpbb_root_path = ''; + $phpEx = 'php'; + + $upload = new phpbb_mock_fileupload(); + $upload->max_filesize = self::UPLOAD_MAX_FILESIZE; + + $filespec = $this->get_filespec(array( + 'tmp_name' => $this->path . 'copies/' . $tmp_name, + 'name' => $realname, + 'type' => $mime_type, + )); + $filespec->extension = $extension; + $filespec->upload = $upload; + $filespec->local = true; + + $this->assertEquals($expected, $filespec->move_file($this->path . 'copies')); + $this->assertEquals($filespec->file_moved, file_exists($this->path . 'copies/' . $realname)); + if ($error) + { + $this->assertEquals($error, $filespec->error[0]); + } + + $phpEx = ''; + } +} diff --git a/tests/upload/fileupload_test.php b/tests/upload/fileupload_test.php new file mode 100644 index 0000000000..2b3c17b8e0 --- /dev/null +++ b/tests/upload/fileupload_test.php @@ -0,0 +1,116 @@ +lang = new phpbb_mock_lang(); + $this->path = __DIR__ . '/fixture/'; + } + + private function gen_valid_filespec() + { + $filespec = new phpbb_mock_filespec(); + $filespec->filesize = 1; + $filespec->extension = 'jpg'; + $filespec->realname = 'valid'; + $filespec->width = 2; + $filespec->height = 2; + + return $filespec; + } + + protected function tearDown() + { + // Clear globals + global $config, $user; + $config = array(); + $user = null; + } + + public function test_common_checks_invalid_extension() + { + $upload = new fileupload('', array('png'), 100); + $file = $this->gen_valid_filespec(); + $upload->common_checks($file); + $this->assertEquals('DISALLOWED_EXTENSION', $file->error[0]); + } + + public function test_common_checks_invalid_filename() + { + $upload = new fileupload('', array('jpg'), 100); + $file = $this->gen_valid_filespec(); + $file->realname = 'invalid?'; + $upload->common_checks($file); + $this->assertEquals('INVALID_FILENAME', $file->error[0]); + } + + public function test_common_checks_too_large() + { + $upload = new fileupload('', array('jpg'), 100); + $file = $this->gen_valid_filespec(); + $file->filesize = 1000; + $upload->common_checks($file); + $this->assertEquals('WRONG_FILESIZE', $file->error[0]); + } + + public function test_common_checks_valid_file() + { + $upload = new fileupload('', array('jpg'), 1000); + $file = $this->gen_valid_filespec(); + $upload->common_checks($file); + $this->assertEquals(0, sizeof($file->error)); + } + + public function test_local_upload() + { + $upload = new fileupload('', array('jpg'), 1000); + + copy($this->path . 'jpg', $this->path . 'jpg.jpg'); + $file = $upload->local_upload($this->path . 'jpg.jpg'); + $this->assertEquals(0, sizeof($file->error)); + unlink($this->path . 'jpg.jpg'); + } + + public function test_valid_dimensions() + { + $upload = new fileupload('', false, false, 1, 1, 100, 100); + + $file1 = $this->gen_valid_filespec(); + $file2 = $this->gen_valid_filespec(); + $file2->height = 101; + $file3 = $this->gen_valid_filespec(); + $file3->width = 0; + + $this->assertTrue($upload->valid_dimensions($file1)); + $this->assertFalse($upload->valid_dimensions($file2)); + $this->assertFalse($upload->valid_dimensions($file3)); + } +} diff --git a/tests/upload/fixture/gif b/tests/upload/fixture/gif new file mode 100644 index 0000000000..b636f4b8df Binary files /dev/null and b/tests/upload/fixture/gif differ diff --git a/tests/upload/fixture/jpg b/tests/upload/fixture/jpg new file mode 100644 index 0000000000..3cd5038e38 Binary files /dev/null and b/tests/upload/fixture/jpg differ diff --git a/tests/upload/fixture/png b/tests/upload/fixture/png new file mode 100644 index 0000000000..5514ad40e9 Binary files /dev/null and b/tests/upload/fixture/png differ diff --git a/tests/upload/fixture/tif b/tests/upload/fixture/tif new file mode 100644 index 0000000000..248b50f9cb Binary files /dev/null and b/tests/upload/fixture/tif differ diff --git a/tests/upload/fixture/txt b/tests/upload/fixture/txt new file mode 100644 index 0000000000..d3c40d2ea7 --- /dev/null +++ b/tests/upload/fixture/txt @@ -0,0 +1 @@ +mime trigger -- cgit v1.2.1 From 9f3a02d4755409407a26bf75324ac0a8a15d87e2 Mon Sep 17 00:00:00 2001 From: Fyorl Date: Mon, 9 Jul 2012 00:10:41 +0100 Subject: [ticket/10941] Removed manual includes of mock classes Also marked a test as incomplete even though this appears to be ignored when actually running the tests. PHPBB3-10941 --- tests/upload/filespec_test.php | 2 -- tests/upload/fileupload_test.php | 1 - 2 files changed, 3 deletions(-) (limited to 'tests/upload') diff --git a/tests/upload/filespec_test.php b/tests/upload/filespec_test.php index 7961e8ed64..fc6409999f 100644 --- a/tests/upload/filespec_test.php +++ b/tests/upload/filespec_test.php @@ -10,8 +10,6 @@ require_once __DIR__ . '/../../phpBB/includes/functions.php'; require_once __DIR__ . '/../../phpBB/includes/utf/utf_tools.php'; require_once __DIR__ . '/../../phpBB/includes/functions_upload.php'; -require_once __DIR__ . '/../mock/fileupload.php'; -require_once __DIR__ . '/../mock/request.php'; class phpbb_filespec_test extends phpbb_test_case { diff --git a/tests/upload/fileupload_test.php b/tests/upload/fileupload_test.php index 2b3c17b8e0..076855ab56 100644 --- a/tests/upload/fileupload_test.php +++ b/tests/upload/fileupload_test.php @@ -10,7 +10,6 @@ require_once __DIR__ . '/../../phpBB/includes/functions.php'; require_once __DIR__ . '/../../phpBB/includes/utf/utf_tools.php'; require_once __DIR__ . '/../../phpBB/includes/functions_upload.php'; -require_once __DIR__ . '/../mock/filespec.php'; class phpbb_fileupload_test extends phpbb_test_case { -- cgit v1.2.1 From ae9b642b815dce164d0675eee89627f63da97eaf Mon Sep 17 00:00:00 2001 From: Fyorl Date: Mon, 9 Jul 2012 01:48:34 +0100 Subject: [ticket/10941] Added tests/upload/fixture/copies to tracking PHPBB3-10941 --- tests/upload/fixture/copies/.gitkeep | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/upload/fixture/copies/.gitkeep (limited to 'tests/upload') diff --git a/tests/upload/fixture/copies/.gitkeep b/tests/upload/fixture/copies/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 -- cgit v1.2.1 From f470eee8abb18fffa834797300f458c1b6478937 Mon Sep 17 00:00:00 2001 From: Fyorl Date: Mon, 9 Jul 2012 01:55:11 +0100 Subject: [ticket/10941] Modified tearDown to use DirectoryIterator instead of glob PHPBB3-10941 --- tests/upload/filespec_test.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'tests/upload') diff --git a/tests/upload/filespec_test.php b/tests/upload/filespec_test.php index fc6409999f..5d75b5fabd 100644 --- a/tests/upload/filespec_test.php +++ b/tests/upload/filespec_test.php @@ -83,9 +83,14 @@ class phpbb_filespec_test extends phpbb_test_case $this->config = array(); $user = null; - foreach (glob($this->path . 'copies/*') as $file) + $iterator = new DirectoryIterator($this->path . 'copies'); + foreach ($iterator as $fileinfo) { - unlink($file); + $name = $fileinfo->getFilename(); + if ($name[0] !== '.') + { + unlink($fileinfo->getPathname()); + } } } -- cgit v1.2.1 From 1b3589aae81d44985b5731bf83cce41da6f713e1 Mon Sep 17 00:00:00 2001 From: Fyorl Date: Mon, 9 Jul 2012 02:02:10 +0100 Subject: [ticket/10941] Added a comment to ensure tags stay uppercase PHPBB3-10941 --- tests/upload/fixture/txt | 1 + 1 file changed, 1 insertion(+) (limited to 'tests/upload') diff --git a/tests/upload/fixture/txt b/tests/upload/fixture/txt index d3c40d2ea7..a78c858f5c 100644 --- a/tests/upload/fixture/txt +++ b/tests/upload/fixture/txt @@ -1 +1,2 @@ mime trigger +The HTML tags should remain uppercase so that case-insensitivity can be checked. -- cgit v1.2.1 From d046b25124e42f1fb99e1eb04b45420a3af3c4d2 Mon Sep 17 00:00:00 2001 From: Fyorl Date: Mon, 9 Jul 2012 02:50:31 +0100 Subject: [ticket/10941] File 'txt' now too big so changed tests to reflect that PHPBB3-10941 --- tests/upload/filespec_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests/upload') diff --git a/tests/upload/filespec_test.php b/tests/upload/filespec_test.php index 5d75b5fabd..34d561eaa0 100644 --- a/tests/upload/filespec_test.php +++ b/tests/upload/filespec_test.php @@ -104,7 +104,7 @@ class phpbb_filespec_test extends phpbb_test_case array('jpg', false), array('png', true), array('tif', false), - array('txt', true), + array('txt', false), ); } -- cgit v1.2.1 From 65d7aae2f306d1fed2f26c97caf8b15d178828ce Mon Sep 17 00:00:00 2001 From: Fyorl Date: Mon, 9 Jul 2012 02:51:30 +0100 Subject: [ticket/10941] Removed superfluous array PHPBB3-10941 --- tests/upload/filespec_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests/upload') diff --git a/tests/upload/filespec_test.php b/tests/upload/filespec_test.php index 34d561eaa0..c7ff2e78e0 100644 --- a/tests/upload/filespec_test.php +++ b/tests/upload/filespec_test.php @@ -114,7 +114,7 @@ class phpbb_filespec_test extends phpbb_test_case public function test_additional_checks($filename, $expected) { $upload = new phpbb_mock_fileupload(); - $filespec = $this->get_filespec(array('tmp_name', $this->path . $filename)); + $filespec = $this->get_filespec(); $filespec->upload = $upload; $filespec->file_moved = true; $filespec->filesize = $filespec->get_filesize($this->path . $filename); -- cgit v1.2.1