diff --git a/includes/message_parser.php b/includes/message_parser.php index 74b8e764..07774c86 100644 --- a/includes/message_parser.php +++ b/includes/message_parser.php @@ -1849,83 +1849,59 @@ class parse_message extends bbcode_firstpass $attachment_data = $request->variable('attachment_data', array(0 => array('' => '')), true, \phpbb\request\request_interface::POST); $this->attachment_data = array(); - $check_user_id = ($check_user_id === false) ? $user->data['user_id'] : $check_user_id; - if (!count($attachment_data)) { - return; + return true; } - $not_orphan = $orphan = array(); + $positions = array(); foreach ($attachment_data as $pos => $var_ary) { - if ($var_ary['is_orphan']) - { - $orphan[(int) $var_ary['attach_id']] = $pos; - } - else - { - $not_orphan[(int) $var_ary['attach_id']] = $pos; - } - } - - // Regenerate already posted attachments - if (count($not_orphan)) - { - // Get the attachment data, based on the poster id... - $sql = 'SELECT attach_id, is_orphan, real_filename, attach_comment, filesize - FROM ' . ATTACHMENTS_TABLE . ' - WHERE ' . $db->sql_in_set('attach_id', array_keys($not_orphan)) . ' - AND poster_id = ' . $check_user_id; - $result = $db->sql_query($sql); - - while ($row = $db->sql_fetchrow($result)) - { - $pos = $not_orphan[$row['attach_id']]; - $this->attachment_data[$pos] = $row; - $this->attachment_data[$pos]['attach_comment'] = $attachment_data[$pos]['attach_comment']; - - unset($not_orphan[$row['attach_id']]); - } - $db->sql_freeresult($result); - } - - if (count($not_orphan)) - { - trigger_error('NO_ACCESS_ATTACHMENT', E_USER_ERROR); + $positions[(int) $var_ary['attach_id']] = $pos; } - // Regenerate newly uploaded attachments - if (count($orphan)) - { - $sql = 'SELECT attach_id, is_orphan, real_filename, attach_comment, filesize - FROM ' . ATTACHMENTS_TABLE . ' - WHERE ' . $db->sql_in_set('attach_id', array_keys($orphan)) . ' - AND poster_id = ' . $user->data['user_id'] . ' - AND is_orphan = 1'; - $result = $db->sql_query($sql); + $check_user_id = ($check_user_id === false) ? $user->data['user_id'] : $check_user_id; - while ($row = $db->sql_fetchrow($result)) + // Get the attachment data, based on the poster id... + $sql = 'SELECT attach_id, is_orphan, real_filename, attach_comment, filesize + FROM ' . ATTACHMENTS_TABLE . ' + WHERE ' . $db->sql_in_set('attach_id', array_keys($positions)) . ' + AND ( + (poster_id = ' . $check_user_id . ' AND is_orphan = 0) + OR + (poster_id = ' . $user->data['user_id'] . ' AND is_orphan = 1) + )'; + $result = $db->sql_query($sql); + + while ($row = $db->sql_fetchrow($result)) + { + $pos = $positions[$row['attach_id']]; + + // If someone double-submits a form, the second submission will + // appear to have valid attachment IDs even though they were already + // consumed by the first submission. To avoid this, match the orphan + // states so that the files are excluded from the next submission. + // Without this check, the second post will have its + // `post_attachments` metadata field set because + // `$this->attachment_data` will not be empty, but it *won’t* have + // attachments since `submit_post` will not update database records + // which were already consumed. + if ($row['is_orphan'] == $attachment_data[$pos]['is_orphan']) { - $pos = $orphan[$row['attach_id']]; $this->attachment_data[$pos] = $row; if (isset($attachment_data[$pos]['attach_comment'])) { $this->attachment_data[$pos]['attach_comment'] = $attachment_data[$pos]['attach_comment']; } - - unset($orphan[$row['attach_id']]); + unset($positions[$row['attach_id']]); } - $db->sql_freeresult($result); - } - - if (count($orphan)) - { - trigger_error('NO_ACCESS_ATTACHMENT', E_USER_ERROR); } + $db->sql_freeresult($result); ksort($this->attachment_data); + + return count($positions) === 0; } /** diff --git a/includes/ucp/ucp_pm_compose.php b/includes/ucp/ucp_pm_compose.php index 1a12fcc1..4b376896 100644 --- a/includes/ucp/ucp_pm_compose.php +++ b/includes/ucp/ucp_pm_compose.php @@ -602,7 +602,10 @@ function compose_pm($id, $mode, $action, $user_folders = array()) // Always check if the submitted attachment data is valid and belongs to the user. // Further down (especially in submit_post()) we do not check this again. - $message_parser->get_submitted_attachment_data(); + if (!$message_parser->get_submitted_attachment_data()) + { + $error[] = $user->lang('NO_ACCESS_ATTACHMENT'); + } if ($message_attachment && !$submit && !$refresh && !$preview && $action == 'edit') { diff --git a/language/en/common.php b/language/en/common.php index fe4f0419..dd83ac17 100644 --- a/language/en/common.php +++ b/language/en/common.php @@ -492,7 +492,7 @@ $lang = array_merge($lang, array( ), 'NOTIFY_ADMIN' => 'Please notify the board administrator or webmaster.', 'NOTIFY_ADMIN_EMAIL' => 'Please notify the board administrator or webmaster: %1$s', - 'NO_ACCESS_ATTACHMENT' => 'You are not allowed to access this file.', + 'NO_ACCESS_ATTACHMENT' => 'Some attachments are no longer available. Please check and re-attach any missing files.', 'NO_ACTION' => 'No action specified.', 'NO_ADMINISTRATORS' => 'There are no administrators.', 'NO_AUTH_ADMIN' => 'Access to the Administration Control Panel is not allowed as you do not have administrative permissions.', diff --git a/posting.php b/posting.php index b8b18b9e..c7437204 100644 --- a/posting.php +++ b/posting.php @@ -641,7 +641,10 @@ unset($uninit); // Always check if the submitted attachment data is valid and belongs to the user. // Further down (especially in submit_post()) we do not check this again. -$message_parser->get_submitted_attachment_data($post_data['poster_id']); +if (!$message_parser->get_submitted_attachment_data($post_data['poster_id'])) +{ + $error[] = $language->lang('NO_ACCESS_ATTACHMENT'); +} if ($post_data['post_attachment'] && !$submit && !$refresh && !$preview && $mode == 'edit') {