From eee2091201b5b76b7b5fddc20092ae69167fbb30 Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Sat, 10 May 2014 15:02:10 +0200 Subject: [PATCH 1/4] [ticket/11226] filespec::move_file() should error correctly PHPBB3-11226 --- phpBB/includes/functions_upload.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/phpBB/includes/functions_upload.php b/phpBB/includes/functions_upload.php index b4e165502b..e7a4c23ba4 100644 --- a/phpBB/includes/functions_upload.php +++ b/phpBB/includes/functions_upload.php @@ -308,6 +308,8 @@ class filespec if (file_exists($this->destination_file) && !$overwrite) { @unlink($this->filename); + $this->error[] = sprintf($user->lang[$this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR'], $this->destination_file); + return false; } else { From cee9b1d85686407276092748b93fc69cb12ddde1 Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Sat, 10 May 2014 19:47:32 +0200 Subject: [PATCH 2/4] [ticket/11226] Use $user->lang() PHPBB3-11226 --- phpBB/includes/functions_upload.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/functions_upload.php b/phpBB/includes/functions_upload.php index e7a4c23ba4..16a05a0a56 100644 --- a/phpBB/includes/functions_upload.php +++ b/phpBB/includes/functions_upload.php @@ -308,7 +308,7 @@ class filespec if (file_exists($this->destination_file) && !$overwrite) { @unlink($this->filename); - $this->error[] = sprintf($user->lang[$this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR'], $this->destination_file); + $this->error[] = $user->lang($this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR', $this->destination_file); return false; } else From 6a3b343dfce3b0c758f94b8768f5a767a95f69ef Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Wed, 28 May 2014 10:53:19 +0200 Subject: [PATCH 3/4] [ticket/11226] Add tests PHPBB3-11226 --- tests/upload/fileupload_test.php | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/upload/fileupload_test.php b/tests/upload/fileupload_test.php index 8b9df33a63..b30bfe59b4 100644 --- a/tests/upload/fileupload_test.php +++ b/tests/upload/fileupload_test.php @@ -103,6 +103,30 @@ class phpbb_fileupload_test extends phpbb_test_case unlink($this->path . 'jpg.jpg'); } + public function test_move_existent_file() + { + $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)); + $file->move_file('../tests/upload/fixture'); + $this->assertEquals(1, sizeof($file->error)); + } + + public function test_move_existent_file_overwrite() + { + $upload = new fileupload('', array('jpg'), 1000); + + copy($this->path . 'jpg', $this->path . 'jpg.jpg'); + copy($this->path . 'jpg', $this->path . 'copies/jpg.jpg'); + $file = $upload->local_upload($this->path . 'jpg.jpg'); + $this->assertEquals(0, sizeof($file->error)); + $file->move_file('../tests/upload/fixture/copies', true); + $this->assertEquals(0, sizeof($file->error)); + unlink($this->path . 'copies/jpg.jpg'); + } + public function test_valid_dimensions() { $upload = new fileupload('', false, false, 1, 1, 100, 100); From b75fb96bab92952011f796cf29611c6bff09dd37 Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Wed, 28 May 2014 22:34:10 +0200 Subject: [PATCH 4/4] [ticket/11226] Explicity set file_moved to false PHPBB3-11226 --- phpBB/includes/functions_upload.php | 1 + tests/upload/fileupload_test.php | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/functions_upload.php b/phpBB/includes/functions_upload.php index 16a05a0a56..21ee9a7119 100644 --- a/phpBB/includes/functions_upload.php +++ b/phpBB/includes/functions_upload.php @@ -309,6 +309,7 @@ class filespec { @unlink($this->filename); $this->error[] = $user->lang($this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR', $this->destination_file); + $this->file_moved = false; return false; } else diff --git a/tests/upload/fileupload_test.php b/tests/upload/fileupload_test.php index b30bfe59b4..c362c3461e 100644 --- a/tests/upload/fileupload_test.php +++ b/tests/upload/fileupload_test.php @@ -110,7 +110,8 @@ class phpbb_fileupload_test extends phpbb_test_case copy($this->path . 'jpg', $this->path . 'jpg.jpg'); $file = $upload->local_upload($this->path . 'jpg.jpg'); $this->assertEquals(0, sizeof($file->error)); - $file->move_file('../tests/upload/fixture'); + $this->assertFalse($file->move_file('../tests/upload/fixture')); + $this->assertFalse($file->file_moved); $this->assertEquals(1, sizeof($file->error)); }