From 39db7005cd0bc6f17837ce7db469fb715984a785 Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 14 Nov 2021 00:43:14 +0700 Subject: [PATCH 1/2] [ticket/16910] Fix PHP warnings on uploading orphaned files to posts PHPBB3-16910 --- phpBB/adm/style/acp_attachments.html | 2 +- phpBB/includes/acp/acp_attachments.php | 54 +++++++++++++++++--------- phpBB/language/en/acp/attachments.php | 1 + 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/phpBB/adm/style/acp_attachments.html b/phpBB/adm/style/acp_attachments.html index a8d6f92d71..a2cfe8f11e 100644 --- a/phpBB/adm/style/acp_attachments.html +++ b/phpBB/adm/style/acp_attachments.html @@ -29,7 +29,7 @@ :: {upload.FILE_INFO}
- {upload.DENIED}{upload.ERROR_MSG}{L_SUCCESSFULLY_UPLOADED} + {upload.L_DENIED}{upload.ERROR_MSG}{L_SUCCESSFULLY_UPLOADED}

diff --git a/phpBB/includes/acp/acp_attachments.php b/phpBB/includes/acp/acp_attachments.php index b666d0b8d9..6b7d9f7b44 100644 --- a/phpBB/includes/acp/acp_attachments.php +++ b/phpBB/includes/acp/acp_attachments.php @@ -1000,29 +1000,45 @@ class acp_attachments $result = $db->sql_query($sql); $files_added = $space_taken = 0; + $error_msg = ''; + $upload_row = []; while ($row = $db->sql_fetchrow($result)) { - $post_row = $post_info[$upload_list[$row['attach_id']]]; + $upload_row = [ + 'FILE_INFO' => $user->lang('UPLOADING_FILE_TO', $row['real_filename'], $upload_list[$row['attach_id']]), + ]; - $template->assign_block_vars('upload', array( - 'FILE_INFO' => sprintf($user->lang['UPLOADING_FILE_TO'], $row['real_filename'], $post_row['post_id']), - 'S_DENIED' => (!$auth->acl_get('f_attach', $post_row['forum_id'])) ? true : false, - 'L_DENIED' => (!$auth->acl_get('f_attach', $post_row['forum_id'])) ? sprintf($user->lang['UPLOAD_DENIED_FORUM'], $forum_names[$row['forum_id']]) : '') - ); + if (isset($post_info[$upload_list[$row['attach_id']]])) + { + $post_row = $post_info[$upload_list[$row['attach_id']]]; + $upload_row = array_merge($upload_row, [ + 'S_DENIED' => !$auth->acl_get('f_attach', $post_row['forum_id']), + 'L_DENIED' => !$auth->acl_get('f_attach', $post_row['forum_id']) ? $user->lang('UPLOAD_DENIED_FORUM', $forum_names[$row['forum_id']]) : '', + ]); + } + else + { + $error_msg = $user->lang('UPLOAD_POST_NOT_EXIST', $row['real_filename'], $upload_list[$row['attach_id']]); + $upload_row = array_merge($upload_row, [ + 'ERROR_MSG' => $error_msg, + ]); + }; - if (!$auth->acl_get('f_attach', $post_row['forum_id'])) + $template->assign_block_vars('upload', $upload_row); + + if ($error_msg || !$auth->acl_get('f_attach', $post_row['forum_id'])) { continue; } // Adjust attachment entry - $sql_ary = array( + $sql_ary = [ 'in_message' => 0, 'is_orphan' => 0, 'poster_id' => $post_row['poster_id'], 'post_msg_id' => $post_row['post_id'], 'topic_id' => $post_row['topic_id'], - ); + ]; $sql = 'UPDATE ' . ATTACHMENTS_TABLE . ' SET ' . $db->sql_build_array('UPDATE', $sql_ary) . ' @@ -1042,7 +1058,7 @@ class acp_attachments $space_taken += $row['filesize']; $files_added++; - $phpbb_log->add('admin', $user->data['user_id'], $user->ip, 'LOG_ATTACH_FILEUPLOAD', false, array($post_row['post_id'], $row['real_filename'])); + $phpbb_log->add('admin', $user->data['user_id'], $user->ip, 'LOG_ATTACH_FILEUPLOAD', false, [$post_row['post_id'], $row['real_filename']]); } $db->sql_freeresult($result); @@ -1054,9 +1070,9 @@ class acp_attachments } } - $template->assign_vars(array( - 'S_ORPHAN' => true) - ); + $template->assign_vars([ + 'S_ORPHAN' => true, + ]); $attachments_per_page = (int) $config['topics_per_page']; @@ -1084,15 +1100,15 @@ class acp_attachments while ($row = $db->sql_fetchrow($result)) { - $template->assign_block_vars('orphan', array( + $template->assign_block_vars('orphan', [ 'FILESIZE' => get_formatted_filesize($row['filesize']), 'FILETIME' => $user->format_date($row['filetime']), 'REAL_FILENAME' => utf8_basename($row['real_filename']), 'PHYSICAL_FILENAME' => utf8_basename($row['physical_filename']), 'ATTACH_ID' => $row['attach_id'], - 'POST_IDS' => (!empty($post_ids[$row['attach_id']])) ? $post_ids[$row['attach_id']] : '', - 'U_FILE' => append_sid($phpbb_root_path . 'download/file.' . $phpEx, 'mode=view&id=' . $row['attach_id'])) - ); + 'POST_ID' => (!empty($post_ids[$row['attach_id']])) ? $post_ids[$row['attach_id']] : '', + 'U_FILE' => append_sid($phpbb_root_path . 'download/file.' . $phpEx, 'mode=view&id=' . $row['attach_id']), + ]); } $db->sql_freeresult($result); @@ -1105,10 +1121,10 @@ class acp_attachments $start ); - $template->assign_vars(array( + $template->assign_vars([ 'TOTAL_FILES' => $num_files, 'TOTAL_SIZE' => get_formatted_filesize($total_size), - )); + ]); break; diff --git a/phpBB/language/en/acp/attachments.php b/phpBB/language/en/acp/attachments.php index 5e0332462a..19fc6b4422 100644 --- a/phpBB/language/en/acp/attachments.php +++ b/phpBB/language/en/acp/attachments.php @@ -170,4 +170,5 @@ $lang = array_merge($lang, array( 'UPLOAD_DIR_EXPLAIN' => 'Storage path for attachments. Please note that if you change this directory while already having uploaded attachments you need to manually copy the files to their new location.', 'UPLOAD_ICON' => 'Upload icon', 'UPLOAD_NOT_DIR' => 'The upload location you specified does not appear to be a directory.', + 'UPLOAD_POST_NOT_EXIST' => 'File “%1$s” can not be uploaded to post number %2$d as the post does not exist.', )); From fecf3306f3febd77b1c5f53e3f86e99ad910d3fb Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 14 Nov 2021 18:53:07 +0700 Subject: [PATCH 2/2] [ticket/16910] Add test PHPBB3-16910 --- tests/functional/acp_attachments_test.php | 118 ++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 tests/functional/acp_attachments_test.php diff --git a/tests/functional/acp_attachments_test.php b/tests/functional/acp_attachments_test.php new file mode 100644 index 0000000000..8b1fb2cc6d --- /dev/null +++ b/tests/functional/acp_attachments_test.php @@ -0,0 +1,118 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +/** + * @group functional + */ +class phpbb_functional_acp_attachments_test extends phpbb_functional_test_case +{ + private $path; + + protected function setUp(): void + { + parent::setUp(); + $this->path = __DIR__ . '/fixtures/files/'; + $this->add_lang('posting'); + } + + protected function tearDown(): void + { + $iterator = new DirectoryIterator(__DIR__ . '/../../phpBB/files/'); + foreach ($iterator as $fileinfo) + { + if ( + $fileinfo->isDot() + || $fileinfo->isDir() + || $fileinfo->getFilename() === 'index.htm' + || $fileinfo->getFilename() === '.htaccess' + ) + { + continue; + } + + unlink($fileinfo->getPathname()); + } + } + + private function upload_file($filename, $mimetype) + { + $crawler = self::$client->request( + 'GET', + 'posting.php?mode=reply&f=2&t=1&sid=' . $this->sid + ); + + $file_form_data = array_merge(['add_file' => $this->lang('ADD_FILE')], $this->get_hidden_fields($crawler, 'posting.php?mode=reply&f=2&t=1&sid=' . $this->sid)); + + $file = array( + 'tmp_name' => $this->path . $filename, + 'name' => $filename, + 'type' => $mimetype, + 'size' => filesize($this->path . $filename), + 'error' => UPLOAD_ERR_OK, + ); + + $crawler = self::$client->request( + 'POST', + 'posting.php?mode=reply&t=1&sid=' . $this->sid, + $file_form_data, + array('fileupload' => $file) + ); + + return $crawler; + } + + public function test_orphaned_attachments() + { + $this->login(); + $this->add_lang(['common', 'acp/common', 'acp/attachments']); + $crawler = $this->upload_file('valid.jpg', 'image/jpeg'); + + // Ensure there was no error message rendered + $this->assertStringNotContainsString('

' . $this->lang('INFORMATION') . '

', $this->get_content()); + + // Also the file name should be in the first row of the files table + $this->assertEquals('valid.jpg', $crawler->filter('span.file-name > a')->text()); + + $attach_link = $crawler->filter('span.file-name > a')->attr('href'); + $attach_id = $this->get_parameter_from_link($attach_link, 'id'); + + // Set file time older than 3 hours to consider it orphan + $sql = 'UPDATE ' . ATTACHMENTS_TABLE . ' + SET filetime = filetime - ' . 4*60*60 . ' + WHERE attach_id = ' . (int) $attach_id; + $this->db->sql_query($sql); + + $this->admin_login(); + $crawler = self::request('GET', 'adm/index.php?sid=' . $this->sid . '&i=acp_attachments&mode=orphan'); + $this->assertContainsLang('ACP_ORPHAN_ATTACHMENTS_EXPLAIN', $this->get_content()); + $this->assertStringContainsString('valid.jpg', $crawler->filter('tbody a')->text()); + + $form = $crawler->selectButton($this->lang('SUBMIT'))->form([ + "post_id[$attach_id]" => 99999, // Random + ]); + $form["add[$attach_id]"]->tick(); + $crawler = self::submit($form); + + $this->assertContainsLang('UPLOADING_FILES', $this->get_content()); + $this->assertStringContainsString($this->lang('UPLOADING_FILE_TO', 'valid.jpg', 99999), $this->get_content()); + $this->assertStringContainsString($this->lang('UPLOAD_POST_NOT_EXIST', 'valid.jpg', 99999), $crawler->filter('span[class="error"]')->text()); + + // Delete the file + $form = $crawler->selectButton($this->lang('SUBMIT'))->form(); + $form["delete[$attach_id]"]->tick(); + $crawler = self::submit($form); + + $this->assertContainsLang('NOTIFY', $crawler->filter('.successbox')->text()); + $this->assertStringContainsString(strip_tags($this->lang('LOG_ATTACH_ORPHAN_DEL', 'valid.jpg')), $crawler->filter('.successbox > p')->text()); + } +}