From 80404a8674da49e249c316a30932bed4a82e2dac Mon Sep 17 00:00:00 2001 From: Aleksander Machniak Date: Sun, 9 Oct 2022 11:35:14 +0200 Subject: Store uploads metadata in a separate sql database table instead of a session (#8415) --- CHANGELOG.md | 1 + SQL/mysql.initial.sql | 14 +- SQL/mysql/2022100100.sql | 9 + SQL/postgres.initial.sql | 17 +- SQL/postgres/2022100100.sql | 9 + SQL/sqlite.initial.sql | 16 +- SQL/sqlite/2022100100.sql | 9 + .../filesystem_attachments.php | 51 +--- plugins/help/help.php | 3 +- plugins/vcard_attachments/vcard_attachments.php | 4 +- program/actions/contacts/index.php | 2 +- program/actions/contacts/photo.php | 3 +- program/actions/contacts/save.php | 11 +- program/actions/contacts/upload_photo.php | 27 +- program/actions/mail/attachment_delete.php | 12 +- program/actions/mail/attachment_display.php | 6 +- program/actions/mail/attachment_rename.php | 11 +- program/actions/mail/attachment_upload.php | 44 ++-- program/actions/mail/compose.php | 81 ++---- program/actions/mail/send.php | 51 +--- program/actions/settings/index.php | 5 +- program/actions/settings/upload.php | 26 +- program/actions/settings/upload_display.php | 19 +- program/include/rcmail.php | 14 +- program/include/rcmail_action.php | 70 +---- program/include/rcmail_attachment_handler.php | 6 +- program/lib/Roundcube/rcube.php | 23 +- program/lib/Roundcube/rcube_plugin.php | 2 +- program/lib/Roundcube/rcube_uploads.php | 285 +++++++++++++++++++++ program/lib/Roundcube/rcube_user.php | 8 +- tests/ActionTestCase.php | 75 ++++++ tests/Actions/Contacts/UploadPhoto.php | 29 +-- tests/Actions/Mail/AttachmentDelete.php | 43 +++- tests/Actions/Mail/AttachmentRename.php | 31 ++- tests/Actions/Mail/AttachmentUpload.php | 73 +++++- tests/Actions/Settings/Upload.php | 55 +++- tests/bootstrap.php | 1 + 37 files changed, 780 insertions(+), 366 deletions(-) create mode 100644 SQL/mysql/2022100100.sql create mode 100644 SQL/postgres/2022100100.sql create mode 100644 SQL/sqlite/2022100100.sql create mode 100644 program/lib/Roundcube/rcube_uploads.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 1614db92e..f8bf17c87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased - Removed support for MS SQL Server and Oracle (#7854) +- Store uploads metadata in a separate sql database table instead of a session (#8415) - Advanced mail search syntax with more possibilities (without UI) (#8502) - Support for HAproxy protocol header in IMAP connections (#8625) - Enigma: Support Kolab's Web Of Anti-Trust feature (#8626) diff --git a/SQL/mysql.initial.sql b/SQL/mysql.initial.sql index f829cb304..bde2b728a 100644 --- a/SQL/mysql.initial.sql +++ b/SQL/mysql.initial.sql @@ -250,6 +250,18 @@ CREATE TABLE `filestore` ( UNIQUE `uniqueness` (`user_id`, `context`, `filename`) ) ROW_FORMAT=DYNAMIC ENGINE=INNODB CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci; +-- Table structure for table `uploads` + +CREATE TABLE `uploads` ( + `upload_id` varchar(64) NOT NULL, + `session_id` varchar(128) NOT NULL, + `group` varchar(128) NOT NULL, + `metadata` mediumtext NOT NULL, + `created` datetime NOT NULL DEFAULT '1000-01-01 00:00:00', + PRIMARY KEY (`upload_id`), + INDEX `uploads_session_group_index` (`session_id`, `group`, `created`) +) ROW_FORMAT=DYNAMIC ENGINE=INNODB CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci; + -- Table structure for table `system` CREATE TABLE `system` ( @@ -260,4 +272,4 @@ CREATE TABLE `system` ( SET FOREIGN_KEY_CHECKS=1; -INSERT INTO `system` (`name`, `value`) VALUES ('roundcube-version', '2022081200'); +INSERT INTO `system` (`name`, `value`) VALUES ('roundcube-version', '2022100100'); diff --git a/SQL/mysql/2022100100.sql b/SQL/mysql/2022100100.sql new file mode 100644 index 000000000..c8a29a83b --- /dev/null +++ b/SQL/mysql/2022100100.sql @@ -0,0 +1,9 @@ +CREATE TABLE `uploads` ( + `upload_id` varchar(64) NOT NULL, + `session_id` varchar(128) NOT NULL, + `group` varchar(128) NOT NULL, + `metadata` mediumtext NOT NULL, + `created` datetime NOT NULL DEFAULT '1000-01-01 00:00:00', + PRIMARY KEY (`upload_id`), + INDEX `uploads_session_group_index` (`session_id`, `group`, `created`) +) ROW_FORMAT=DYNAMIC ENGINE=INNODB CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci; diff --git a/SQL/postgres.initial.sql b/SQL/postgres.initial.sql index 209f0066c..5a3bafd1d 100644 --- a/SQL/postgres.initial.sql +++ b/SQL/postgres.initial.sql @@ -365,6 +365,21 @@ CREATE TABLE "filestore" ( CONSTRAINT filestore_user_id_filename UNIQUE (user_id, context, filename) ); +-- +-- Table "uploads" +-- Name: uploads; Type: TABLE; Schema: public; Owner: postgres +-- + +CREATE TABLE "uploads" ( + upload_id varchar(64) PRIMARY KEY, + session_id varchar(128) NOT NULL, + "group" varchar(128) NOT NULL, + metadata text NOT NULL, + created timestamp with time zone DEFAULT now() NOT NULL +); + +CREATE INDEX uploads_session_id_idx ON uploads (session_id, "group", created); + -- -- Table "system" -- Name: system; Type: TABLE; Schema: public; Owner: postgres @@ -375,4 +390,4 @@ CREATE TABLE "system" ( value text ); -INSERT INTO "system" (name, value) VALUES ('roundcube-version', '2022081200'); +INSERT INTO "system" (name, value) VALUES ('roundcube-version', '2022100100'); diff --git a/SQL/postgres/2022100100.sql b/SQL/postgres/2022100100.sql new file mode 100644 index 000000000..44c4d311b --- /dev/null +++ b/SQL/postgres/2022100100.sql @@ -0,0 +1,9 @@ +CREATE TABLE "uploads" ( + upload_id varchar(64) PRIMARY KEY, + session_id varchar(128) NOT NULL, + "group" varchar(128) NOT NULL, + metadata text NOT NULL, + created timestamp with time zone DEFAULT now() NOT NULL +); + +CREATE INDEX uploads_session_id_idx ON uploads (session_id, "group", created); diff --git a/SQL/sqlite.initial.sql b/SQL/sqlite.initial.sql index dd8f2f500..2cc5baa4d 100644 --- a/SQL/sqlite.initial.sql +++ b/SQL/sqlite.initial.sql @@ -251,6 +251,20 @@ CREATE TABLE filestore ( CREATE UNIQUE INDEX ix_filestore_user_id ON filestore(user_id, context, filename); +-- +-- Table structure for table uploads +-- + +CREATE TABLE uploads ( + upload_id varchar(64) NOT NULL PRIMARY KEY, + session_id varchar(128) NOT NULL, + "group" varchar(128) NOT NULL, + metadata text NOT NULL, + created datetime NOT NULL default '0000-00-00 00:00:00' +); + +CREATE INDEX ix_uploads_session_id ON uploads (session_id, "group", created); + -- -- Table structure for table system -- @@ -260,4 +274,4 @@ CREATE TABLE system ( value text NOT NULL ); -INSERT INTO system (name, value) VALUES ('roundcube-version', '2022081200'); +INSERT INTO system (name, value) VALUES ('roundcube-version', '2022100100'); diff --git a/SQL/sqlite/2022100100.sql b/SQL/sqlite/2022100100.sql new file mode 100644 index 000000000..97d83d2b2 --- /dev/null +++ b/SQL/sqlite/2022100100.sql @@ -0,0 +1,9 @@ +CREATE TABLE uploads ( + upload_id varchar(64) NOT NULL PRIMARY KEY, + session_id varchar(128) NOT NULL, + "group" varchar(128) NOT NULL, + metadata text NOT NULL, + created datetime NOT NULL default '0000-00-00 00:00:00' +); + +CREATE INDEX ix_uploads_session_id ON uploads(session_id, "group", created); diff --git a/plugins/filesystem_attachments/filesystem_attachments.php b/plugins/filesystem_attachments/filesystem_attachments.php index 172726ce0..a382982e8 100644 --- a/plugins/filesystem_attachments/filesystem_attachments.php +++ b/plugins/filesystem_attachments/filesystem_attachments.php @@ -77,14 +77,11 @@ class filesystem_attachments extends rcube_plugin // use common temp dir for file uploads $tmpfname = rcube_utils::temp_filename('attmnt'); - if (move_uploaded_file($args['path'], $tmpfname) && file_exists($tmpfname)) { + if (!empty($args['path']) && move_uploaded_file($args['path'], $tmpfname) && file_exists($tmpfname)) { $args['id'] = $this->file_id(); $args['path'] = $tmpfname; $args['status'] = true; @chmod($tmpfname, 0600); // set correct permissions (#1488996) - - // Note the file for later cleanup - $_SESSION['plugins']['filesystem_attachments'][$group][$args['id']] = $tmpfname; } return $args; @@ -114,9 +111,6 @@ class filesystem_attachments extends rcube_plugin $args['id'] = $this->file_id(); $args['status'] = true; - // Note the file for later cleanup - $_SESSION['plugins']['filesystem_attachments'][$group][$args['id']] = $args['path']; - return $args; } @@ -143,7 +137,7 @@ class filesystem_attachments extends rcube_plugin /** * This attachment plugin doesn't require any steps to put the file - * on disk for use. This stub function is kept here to make this + * on disk for use. This stub function is kept here to make this * class handy as a parent class for other plugins which may need it. */ function get($args) @@ -156,26 +150,16 @@ class filesystem_attachments extends rcube_plugin } /** - * Delete all temp files associated with this user + * Delete all temp files associated with this user session */ function cleanup($args) { - // $_SESSION['compose']['attachments'] is not a complete record of - // temporary files because loading a draft or starting a forward copies - // the file to disk, but does not make an entry in that array - if (!empty($_SESSION['plugins']['filesystem_attachments'])) { - foreach ($_SESSION['plugins']['filesystem_attachments'] as $group => $files) { - if (!empty($args['group']) && $args['group'] != $group) { - continue; - } - - foreach ((array) $files as $filename) { - if (file_exists($filename)) { - unlink($filename); - } - } - - unset($_SESSION['plugins']['filesystem_attachments'][$group]); + $rcube = rcube::get_instance(); + $group = $args['group'] ?? null; + + foreach ($rcube->list_uploaded_files($group) as $file) { + if ($file['path'] && $this->verify_path($file['path']) && file_exists($file['path'])) { + unlink($file['path']); } } @@ -184,12 +168,12 @@ class filesystem_attachments extends rcube_plugin protected static function file_id() { - $userid = rcube::get_instance()->user->ID; + $rcube = rcube::get_instance(); list($usec, $sec) = explode(' ', microtime()); - $id = preg_replace('/[^0-9]/', '', $userid . $sec . $usec); + $id = preg_replace('/[^0-9]/', '', $rcube->user->ID . $sec . $usec); // make sure the ID is really unique (#1489546) - while (self::find_file_by_id($id)) { + while ($rcube->get_uploaded_file($id)) { // increment last four characters $x = substr($id, -4) + 1; $id = substr($id, 0, -4) . sprintf('%04d', ($x > 9999 ? $x - 9999 : $x)); @@ -198,17 +182,6 @@ class filesystem_attachments extends rcube_plugin return $id; } - private static function find_file_by_id($id) - { - if (!empty($_SESSION['plugins']['filesystem_attachments'])) { - foreach ((array) $_SESSION['plugins']['filesystem_attachments'] as $files) { - if (isset($files[$id])) { - return true; - } - } - } - } - /** * For security we'll always verify the file path stored in session, * as session entries can be faked in various ways e.g. #6026. diff --git a/plugins/help/help.php b/plugins/help/help.php index e4b71d399..0162b5b31 100644 --- a/plugins/help/help.php +++ b/plugins/help/help.php @@ -178,7 +178,8 @@ class help extends rcube_plugin // resolve language placeholder $rcmail = rcmail::get_instance(); $langmap = $rcmail->config->get('help_language_map', ['*' => 'en_US']); - $lang = !empty($langmap[$_SESSION['language']]) ? $langmap[$_SESSION['language']] : $langmap['*']; + $lang = $_SESSION['language'] ?? 'en_US'; + $lang = !empty($langmap[$lang]) ? $langmap[$lang] : $langmap['*']; return str_replace('%l', $lang, $path); } diff --git a/plugins/vcard_attachments/vcard_attachments.php b/plugins/vcard_attachments/vcard_attachments.php index c4cc10f2a..d4bb268df 100644 --- a/plugins/vcard_attachments/vcard_attachments.php +++ b/plugins/vcard_attachments/vcard_attachments.php @@ -162,9 +162,7 @@ class vcard_attachments extends rcube_plugin rcube_utils::get_input_string('_attach_vcard', rcube_utils::INPUT_GET) == '1' && ($uri = rcube_utils::get_input_string('_uri', rcube_utils::INPUT_GET)) ) { - if ($attachment = $this->attach_vcard(['compose_id' => $p['id'], 'uri' => $uri])) { - $p['attachments'][] = $attachment; - }; + $this->attach_vcard(['compose_id' => $p['id'], 'uri' => $uri]); } return $p; diff --git a/program/actions/contacts/index.php b/program/actions/contacts/index.php index 63c4ba20b..b192a1fe7 100644 --- a/program/actions/contacts/index.php +++ b/program/actions/contacts/index.php @@ -1336,7 +1336,7 @@ class rcmail_action_contacts_index extends rcmail_action if (self::$contact['photo'] == '-del-') { $record['photo'] = ''; } - else if (!empty($_SESSION['contacts']['files'][self::$contact['photo']])) { + else if (is_numeric(self::$contact['photo']) && $rcmail->get_uploaded_file(self::$contact['photo'])) { $record['photo'] = $file_id = self::$contact['photo']; } } diff --git a/program/actions/contacts/photo.php b/program/actions/contacts/photo.php index bada2312b..1f1269be6 100644 --- a/program/actions/contacts/photo.php +++ b/program/actions/contacts/photo.php @@ -38,8 +38,7 @@ class rcmail_action_contacts_photo extends rcmail_action_contacts_index $file_id = rcube_utils::get_input_string('_photo', rcube_utils::INPUT_GPC); // read the referenced file - if ($file_id && !empty($_SESSION['contacts']['files'][$file_id])) { - $tempfile = $_SESSION['contacts']['files'][$file_id]; + if ($file_id && ($tempfile = $rcmail->get_uploaded_file($file_id))) { $tempfile = $rcmail->plugins->exec_hook('attachment_display', $tempfile); if (!empty($tempfile['status'])) { diff --git a/program/actions/contacts/save.php b/program/actions/contacts/save.php index 193edbf9a..822afc49f 100644 --- a/program/actions/contacts/save.php +++ b/program/actions/contacts/save.php @@ -63,20 +63,15 @@ class rcmail_action_contacts_save extends rcmail_action_contacts_index if ($a_record['photo'] == '-del-') { $a_record['photo'] = ''; } - else if (!empty($_SESSION['contacts']['files'][$a_record['photo']])) { - $tempfile = $_SESSION['contacts']['files'][$a_record['photo']]; + else if (is_numeric($a_record['photo']) && ($tempfile = $rcmail->get_uploaded_file($a_record['photo']))) { $tempfile = $rcmail->plugins->exec_hook('attachment_get', $tempfile); - if ($tempfile['status']) { - $a_record['photo'] = $tempfile['data'] ?: @file_get_contents($tempfile['path']); + if (empty($tempfile['abort'])) { + $a_record['photo'] = $tempfile['data'] ?? @file_get_contents($tempfile['path']); } } else { unset($a_record['photo']); } - - // cleanup session data - $rcmail->plugins->exec_hook('attachments_cleanup', ['group' => 'contact']); - $rcmail->session->remove('contacts'); } // update an existing contact diff --git a/program/actions/contacts/upload_photo.php b/program/actions/contacts/upload_photo.php index b3fa98c08..a6f3a8316 100644 --- a/program/actions/contacts/upload_photo.php +++ b/program/actions/contacts/upload_photo.php @@ -45,11 +45,12 @@ class rcmail_action_contacts_upload_photo extends rcmail_action_contacts_index // check file type and resize image $image = new rcube_image($_FILES['_photo']['tmp_name']); $imageprop = $image->props(); + $inserted = false; if ( in_array(strtolower($imageprop['type']), self::$IMAGE_TYPES) - && $imageprop['width'] - && $imageprop['height'] + && !empty($imageprop['width']) + && !empty($imageprop['height']) ) { $maxsize = intval($rcmail->config->get('contact_photo_size', 160)); $tmpfname = rcube_utils::temp_filename('imgconvert'); @@ -62,22 +63,22 @@ class rcmail_action_contacts_upload_photo extends rcmail_action_contacts_index } // save uploaded file in storage backend - $attachment = $rcmail->plugins->exec_hook($save_hook, [ - 'path' => $filepath, - 'size' => $_FILES['_photo']['size'], - 'name' => $_FILES['_photo']['name'], - 'mimetype' => 'image/' . $imageprop['type'], - 'group' => 'contact', - ]); + $attachment = [ + 'path' => $filepath, + 'size' => $_FILES['_photo']['size'], + 'name' => $_FILES['_photo']['name'], + 'mimetype' => 'image/' . $imageprop['type'], + 'group' => 'contact', + ]; + + $inserted = $rcmail->insert_uploaded_file($attachment, $save_hook); } else { $attachment = ['error' => $rcmail->gettext('invalidimageformat')]; } - if (!empty($attachment['status']) && empty($attachment['abort'])) { - $file_id = $attachment['id']; - $_SESSION['contacts']['files'][$file_id] = $attachment; - $rcmail->output->command('replace_contact_photo', $file_id); + if ($inserted) { + $rcmail->output->command('replace_contact_photo', $attachment['id']); } else { // upload failed diff --git a/program/actions/mail/attachment_delete.php b/program/actions/mail/attachment_delete.php index b129b4b8c..f90c0bbf7 100644 --- a/program/actions/mail/attachment_delete.php +++ b/program/actions/mail/attachment_delete.php @@ -31,16 +31,10 @@ class rcmail_action_mail_attachment_delete extends rcmail_action_mail_attachment { self::init(); - $rcmail = rcmail::get_instance(); - $attachment = self::get_attachment(); + $rcmail = rcmail::get_instance(); - if (is_array($attachment)) { - $attachment = $rcmail->plugins->exec_hook('attachment_delete', $attachment); - - if (!empty($attachment['status'])) { - $rcmail->session->remove(self::$SESSION_KEY . '.attachments.' . self::$file_id); - $rcmail->output->command('remove_from_attachment_list', 'rcmfile' . self::$file_id); - } + if ($rcmail->delete_uploaded_file(self::$file_id)) { + $rcmail->output->command('remove_from_attachment_list', 'rcmfile' . self::$file_id); } $rcmail->output->send(); diff --git a/program/actions/mail/attachment_display.php b/program/actions/mail/attachment_display.php index 64bca4ba7..002fb166d 100644 --- a/program/actions/mail/attachment_display.php +++ b/program/actions/mail/attachment_display.php @@ -29,7 +29,11 @@ class rcmail_action_mail_attachment_display extends rcmail_action_mail_attachmen public function run($args = []) { self::init(); - self::display_uploaded_file(self::get_attachment()); + + $rcmail = rcmail::get_instance(); + $file = $rcmail->get_uploaded_file(self::$file_id); + + self::display_uploaded_file($file); exit; } } diff --git a/program/actions/mail/attachment_rename.php b/program/actions/mail/attachment_rename.php index 53dba0cba..f112b1b6b 100644 --- a/program/actions/mail/attachment_rename.php +++ b/program/actions/mail/attachment_rename.php @@ -36,16 +36,7 @@ class rcmail_action_mail_attachment_rename extends rcmail_action_mail_attachment $filename = rcube_utils::get_input_string('_name', rcube_utils::INPUT_POST); $filename = trim($filename); - if ( - strlen($filename) - && ($attachment = self::get_attachment()) - && is_array($attachment) - ) { - $attachment['name'] = $filename; - - $rcmail->session->remove(self::$SESSION_KEY . '.attachments. ' . self::$file_id); - $rcmail->session->append(self::$SESSION_KEY . '.attachments', $attachment['id'], $attachment); - + if (strlen($filename) && ($rcmail->update_uploaded_file(self::$file_id, ['name' => $filename]))) { $rcmail->output->command('rename_attachment_handler', 'rcmfile' . self::$file_id, $filename); } diff --git a/program/actions/mail/attachment_upload.php b/program/actions/mail/attachment_upload.php index 3e844acbb..0c3727f01 100644 --- a/program/actions/mail/attachment_upload.php +++ b/program/actions/mail/attachment_upload.php @@ -94,13 +94,14 @@ class rcmail_action_mail_attachment_upload extends rcmail_action_mail_index } // handle file(s) upload - if (is_array($_FILES['_attachments']['tmp_name'])) { - $multiple = count($_FILES['_attachments']['tmp_name']) > 1; - $errors = []; + if (isset($_FILES['_attachments']['tmp_name']) && is_array($_FILES['_attachments']['tmp_name'])) { + $errors = []; foreach ($_FILES['_attachments']['tmp_name'] as $i => $filepath) { // Process uploaded attachment if there is no error $err = $_FILES['_attachments']['error'][$i]; + $inserted = false; + $attachment = null; if (!$err) { $filename = $_FILES['_attachments']['name'][$i]; @@ -117,20 +118,18 @@ class rcmail_action_mail_attachment_upload extends rcmail_action_mail_index continue; } - $attachment = $rcmail->plugins->exec_hook('attachment_upload', [ - 'path' => $filepath, - 'name' => $filename, - 'size' => $filesize, - 'mimetype' => $filetype, - 'group' => self::$COMPOSE_ID, - ]); - } + $attachment = [ + 'path' => $filepath, + 'name' => $filename, + 'size' => $filesize, + 'mimetype' => $filetype, + 'group' => self::$COMPOSE_ID, + ]; - if (!$err && !empty($attachment['status']) && empty($attachment['abort'])) { - // store new attachment in session - unset($attachment['status'], $attachment['abort']); - $rcmail->session->append(self::$SESSION_KEY . '.attachments', $attachment['id'], $attachment); + $inserted = $rcmail->insert_uploaded_file($attachment); + } + if (!$err && $inserted) { self::attachment_success($attachment, $uploadid); } else { // upload failed @@ -182,11 +181,6 @@ class rcmail_action_mail_attachment_upload extends rcmail_action_mail_index self::$file_id = preg_replace('/^rcmfile/', '', self::$file_id) ?: 'unknown'; } - public static function get_attachment() - { - return self::$COMPOSE['attachments'][self::$file_id]; - } - public static function attachment_success($attachment, $uploadid) { $rcmail = rcmail::get_instance(); @@ -270,12 +264,10 @@ class rcmail_action_mail_attachment_upload extends rcmail_action_mail_index } // add size of already attached files - if (!empty(self::$COMPOSE['attachments'])) { - foreach ((array) self::$COMPOSE['attachments'] as $att) { - // All attachments are base64-encoded except message/rfc822 (see sendmail.inc) - $multip = $att['mimetype'] == 'message/rfc822' ? 1 : 1.33; - $size += $att['size'] * $multip; - } + foreach ($rcmail->list_uploaded_files(self::$COMPOSE_ID) as $att) { + // All attachments are base64-encoded except message/rfc822 + $multip = $att['mimetype'] == 'message/rfc822' ? 1 : 1.33; + $size += $att['size'] * $multip; } // add size of the new attachment diff --git a/program/actions/mail/compose.php b/program/actions/mail/compose.php index 1ca73f75c..385dea390 100644 --- a/program/actions/mail/compose.php +++ b/program/actions/mail/compose.php @@ -435,12 +435,7 @@ class rcmail_action_mail_compose extends rcmail_action_mail_index (!empty($attachment['data']) && !empty($attachment['name'])) || (!empty($attachment['path']) && file_exists($attachment['path'])) ) { - $attachment = $rcmail->plugins->exec_hook('attachment_save', $attachment); - } - - if (!empty($attachment['status']) && empty($attachment['abort'])) { - unset($attachment['data'], $attachment['status'], $attachment['abort']); - $COMPOSE['attachments'][$attachment['id']] = $attachment; + $rcmail->insert_uploaded_file($attachment, 'attachment_save'); } } } @@ -683,7 +678,6 @@ class rcmail_action_mail_compose extends rcmail_action_mail_index $content = self::get_resource_content('blocked.gif'); if ($content && ($attachment = self::save_image('blocked.gif', 'image/gif', $content))) { - self::$COMPOSE['attachments'][$attachment['id']] = $attachment; $url = sprintf('%s&_id=%s&_action=display-attachment&_file=rcmfile%s', $rcmail->comm_path, self::$COMPOSE['id'], $attachment['id']); $body = preg_replace($regexp, ' src="' . $url . '"', $body); @@ -1047,17 +1041,14 @@ class rcmail_action_mail_compose extends rcmail_action_mail_index return; } - $messages = []; - $loaded_attachments = []; - - if (!empty(self::$COMPOSE['attachments'])) { - foreach ((array) self::$COMPOSE['attachments'] as $attachment) { - $loaded_attachments[$attachment['name'] . $attachment['mimetype']] = $attachment; - } - } - $rcmail = rcmail::get_instance(); $has_html = $message->has_html_part(); + $messages = []; + $loaded = []; + + foreach ($rcmail->list_uploaded_files(self::$COMPOSE_ID) as $attachment) { + $loaded[$attachment['name'] . $attachment['mimetype']] = $attachment; + } foreach ((array) $message->mime_parts() as $pid => $part) { if ($part->mimetype == 'message/rfc822') { @@ -1118,8 +1109,8 @@ class rcmail_action_mail_compose extends rcmail_action_mail_index $key = self::attachment_name($part) . $part->mimetype; - if (!empty($loaded_attachments[$key])) { - $attachment = $loaded_attachments[$key]; + if (!empty($loaded[$key])) { + $attachment = $loaded[$key]; } else { $attachment = self::save_attachment($message, $pid, self::$COMPOSE['id']); @@ -1183,19 +1174,16 @@ class rcmail_action_mail_compose extends rcmail_action_mail_index $rcmail = rcmail::get_instance(); $storage = $rcmail->get_storage(); - $names = []; - $refs = []; $size_errors = 0; $size_limit = parse_bytes($rcmail->config->get('max_message_size')); $total_size = 10 * 1024; // size of message body, to start with + $names = []; + $refs = []; + $loaded = []; - $loaded_attachments = []; - - if (!empty(self::$COMPOSE['attachments'])) { - foreach ((array) self::$COMPOSE['attachments'] as $attachment) { - $loaded_attachments[$attachment['name'] . $attachment['mimetype']] = $attachment; - $total_size += $attachment['size']; - } + foreach ($rcmail->list_uploaded_files(self::$COMPOSE_ID) as $attachment) { + $loaded[$attachment['name'] . $attachment['mimetype']] = $attachment; + $total_size += $attachment['size']; } if (self::$COMPOSE['forward_uid'] == '*') { @@ -1237,7 +1225,7 @@ class rcmail_action_mail_compose extends rcmail_action_mail_index $names[$name] = 1; $name .= '.eml'; - if (!empty($loaded_attachments[$name . 'message/rfc822'])) { + if (!empty($loaded[$name . 'message/rfc822'])) { continue; } @@ -1280,6 +1268,7 @@ class rcmail_action_mail_compose extends rcmail_action_mail_index */ public static function save_image($path, $mimetype = '', $data = null) { + $rcmail = rcmail::get_instance(); $is_file = false; // handle attachments in memory @@ -1307,10 +1296,7 @@ class rcmail_action_mail_compose extends rcmail_action_mail_index 'size' => strlen($data), ]; - $attachment = rcmail::get_instance()->plugins->exec_hook('attachment_save', $attachment); - - if ($attachment['status']) { - unset($attachment['data'], $attachment['status'], $attachment['content_id'], $attachment['abort']); + if ($rcmail->insert_uploaded_file($attachment, 'attachment_save')) { return $attachment; } @@ -1380,9 +1366,12 @@ class rcmail_action_mail_compose extends rcmail_action_mail_index if (!empty($attrib['icon_pos']) && $attrib['icon_pos'] == 'left') { self::$COMPOSE['icon_pos'] = 'left'; } + $icon_pos = self::$COMPOSE['icon_pos'] ?? null; - if (!empty(self::$COMPOSE['attachments'])) { + $attachments = $rcmail->list_uploaded_files(self::$COMPOSE_ID); + + if (!empty($attachments)) { if (!empty($attrib['deleteicon'])) { $button = html::img([ 'src' => $rcmail->output->asset_url($attrib['deleteicon'], true), @@ -1393,7 +1382,7 @@ class rcmail_action_mail_compose extends rcmail_action_mail_index $button = rcube::Q($rcmail->gettext('delete')); } - foreach (self::$COMPOSE['attachments'] as $id => $a_prop) { + foreach ($attachments as $id => $a_prop) { if (empty($a_prop)) { continue; } @@ -1440,10 +1429,10 @@ class rcmail_action_mail_compose extends rcmail_action_mail_index $icon_pos == 'left' ? $delete_link.$content_link : $content_link.$delete_link ); - $jslist['rcmfile'.$id] = [ + $jslist['rcmfile' . $id] = [ 'name' => $a_prop['name'], + 'mimetype' => $a_prop['mimetype'], 'complete' => true, - 'mimetype' => $a_prop['mimetype'] ]; } } @@ -1716,25 +1705,7 @@ class rcmail_action_mail_compose extends rcmail_action_mail_index 'charset' => !empty($part) ? $part->charset : ($params['charset'] ?? null), ]; - $attachment = $rcmail->plugins->exec_hook('attachment_save', $attachment); - - if ($attachment['status']) { - unset($attachment['data'], $attachment['status'], $attachment['content_id'], $attachment['abort']); - - // rcube_session::append() replaces current session data with the old values - // (in rcube_session::reload()). This is a problem in 'compose' action, because before - // the first append() use we set some important data in the session. - // It also overwrites attachments list. Fixing reload() is not so simple if possible - // as we don't really know what has been added and what removed in meantime. - // So, for now we'll do not use append() on 'compose' action (#1490608). - - if ($rcmail->action == 'compose') { - self::$COMPOSE['attachments'][$attachment['id']] = $attachment; - } - else { - $rcmail->session->append('compose_data_' . $compose_id . '.attachments', $attachment['id'], $attachment); - } - + if ($rcmail->insert_uploaded_file($attachment, 'attachment_save')) { return $attachment; } else if (!empty($path)) { diff --git a/program/actions/mail/send.php b/program/actions/mail/send.php index a28d7f9ba..e265c3bb9 100644 --- a/program/actions/mail/send.php +++ b/program/actions/mail/send.php @@ -63,10 +63,6 @@ class rcmail_action_mail_send extends rcmail_action 'keepformatting' => !empty($_POST['_keepformatting']), ]); - if (!isset($COMPOSE['attachments'])) { - $COMPOSE['attachments'] = []; - } - // Collect input for message headers $headers = $SENDMAIL->headers_input(); @@ -84,13 +80,7 @@ class rcmail_action_mail_send extends rcmail_action $message_body = ''; // clear unencrypted attachments - if (!empty($COMPOSE['attachments'])) { - foreach ((array) $COMPOSE['attachments'] as $attach) { - $rcmail->plugins->exec_hook('attachment_delete', $attach); - } - } - - $COMPOSE['attachments'] = []; + $rcmail->delete_uploaded_files(self::$COMPOSE_ID); } if ($isHtml) { @@ -194,29 +184,17 @@ class rcmail_action_mail_send extends rcmail_action $message_body .= "\r\n\r\n"; } - // sort attachments to make sure the order is the same as in the UI (#1488423) - if ($files = rcube_utils::get_input_string('_attachments', rcube_utils::INPUT_POST)) { - $files = explode(',', $files); - $files = array_flip($files); - foreach ($files as $idx => $val) { - if (!empty($COMPOSE['attachments'][$idx])) { - $files[$idx] = $COMPOSE['attachments'][$idx]; - unset($COMPOSE['attachments'][$idx]); - } - } - - $COMPOSE['attachments'] = array_merge(array_filter($files), (array) $COMPOSE['attachments']); - } + $attachments = $rcmail->list_uploaded_files($COMPOSE_ID); // Since we can handle big messages with disk usage, we need more time to work @set_time_limit(360); // create PEAR::Mail_mime instance, set headers, body and params - $MAIL_MIME = $SENDMAIL->create_message($headers, $message_body, $isHtml, $COMPOSE['attachments']); + $MAIL_MIME = $SENDMAIL->create_message($headers, $message_body, $isHtml, $attachments); // add stored attachments, if any - if (is_array($COMPOSE['attachments'])) { - self::add_attachments($SENDMAIL, $MAIL_MIME, $COMPOSE['attachments'], $isHtml); + if (!empty($attachments)) { + self::add_attachments($SENDMAIL, $MAIL_MIME, $attachments, $isHtml); } // compose PGP/Mime message @@ -332,7 +310,7 @@ class rcmail_action_mail_send extends rcmail_action $save_error = true; } else { - $rcmail->plugins->exec_hook('attachments_cleanup', ['group' => $COMPOSE_ID]); + $rcmail->delete_uploaded_files($COMPOSE_ID); $rcmail->session->remove('compose_data_' . $COMPOSE_ID); $_SESSION['last_compose_session'] = $COMPOSE_ID; @@ -353,13 +331,16 @@ class rcmail_action_mail_send extends rcmail_action public static function add_attachments($SENDMAIL, $message, $attachments, $isHtml) { - $rcmail = rcmail::get_instance(); + $rcmail = rcmail::get_instance(); + $folding = (int) $rcmail->config->get('mime_param_folding'); foreach ($attachments as $id => $attachment) { // This hook retrieves the attachment contents from the file storage backend $attachment = $rcmail->plugins->exec_hook('attachment_get', $attachment); $is_inline = false; $dispurl = null; + $is_file = !empty($attachment['path']); + $file = !empty($attachment['path']) ? $attachment['path'] : ($attachment['data'] ?? ''); if ($isHtml) { $dispurl = '/[\'"]\S+display-attachment\S+file=rcmfile' . preg_quote($attachment['id']) . '[\'"]/'; @@ -395,21 +376,13 @@ class rcmail_action_mail_send extends rcmail_action $message->setHTMLBody($message_body); } - if (!empty($attachment['data'])) { - $message->addHTMLImage($attachment['data'], $ctype, $attachment['name'], false, $cid); - } - else { - $message->addHTMLImage($attachment['path'], $ctype, $attachment['name'], true, $cid); - } + $message->addHTMLImage($file, $ctype, $attachment['name'], $is_file, $cid); } else { - $file = !empty($attachment['data']) ? $attachment['data'] : $attachment['path']; - $folding = (int) $rcmail->config->get('mime_param_folding'); - $message->addAttachment($file, $ctype, $attachment['name'], - empty($attachment['data']), + $is_file, $ctype == 'message/rfc822' ? '8bit' : 'base64', 'attachment', $attachment['charset'] ?? null, diff --git a/program/actions/settings/index.php b/program/actions/settings/index.php index fdc3471da..0579b52a0 100644 --- a/program/actions/settings/index.php +++ b/program/actions/settings/index.php @@ -1673,7 +1673,7 @@ class rcmail_action_settings_index extends rcmail_action if ($labels === null) { $labels = []; - $lang = $_SESSION['language']; + $lang = $_SESSION['language'] ?? 'en_US'; if ($lang && $lang != 'en_US') { if (file_exists(RCUBE_LOCALIZATION_DIR . "$lang/timezones.inc")) { include RCUBE_LOCALIZATION_DIR . "$lang/timezones.inc"; @@ -1741,8 +1741,7 @@ class rcmail_action_settings_index extends rcmail_action $file_id = $matches[2]; $data_uri = ' '; - if ($file_id && !empty($_SESSION[$mode]['files'][$file_id])) { - $file = $_SESSION[$mode]['files'][$file_id]; + if ($file_id && ($file = $rcmail->get_uploaded_file($file_id))) { $file = $rcmail->plugins->exec_hook('attachment_get', $file); $data_uri .= 'src="data:' . $file['mimetype'] . ';base64,'; diff --git a/program/actions/settings/upload.php b/program/actions/settings/upload.php index d1cbbdc14..415939b1c 100644 --- a/program/actions/settings/upload.php +++ b/program/actions/settings/upload.php @@ -28,9 +28,10 @@ class rcmail_action_settings_upload extends rcmail_action */ public function run($args = []) { - $rcmail = rcmail::get_instance(); - $from = rcube_utils::get_input_string('_from', rcube_utils::INPUT_GET); - $type = preg_replace('/(add|edit)-/', '', $from); + $rcmail = rcmail::get_instance(); + $uploadid = rcube_utils::get_input_string('_uploadid', rcube_utils::INPUT_GET); + $from = rcube_utils::get_input_string('_from', rcube_utils::INPUT_GET); + $type = preg_replace('/(add|edit)-/', '', $from); // Plugins in Settings may use this file for some uploads (#5694) // Make sure it does not contain a dot, which is a special character @@ -44,15 +45,13 @@ class rcmail_action_settings_upload extends rcmail_action $rcmail->output->reset(); $max_size = $rcmail->config->get($type . '_image_size', 64) * 1024; - $uploadid = rcube_utils::get_input_string('_uploadid', rcube_utils::INPUT_GET); if (!empty($_FILES['_file']['tmp_name']) && is_array($_FILES['_file']['tmp_name'])) { - $multiple = count($_FILES['_file']['tmp_name']) > 1; - foreach ($_FILES['_file']['tmp_name'] as $i => $filepath) { $err = $_FILES['_file']['error'][$i]; $imageprop = null; $attachment = null; + $inserted = false; // Process uploaded attachment if there is no error if (!$err) { @@ -72,22 +71,19 @@ class rcmail_action_settings_upload extends rcmail_action // save uploaded image in storage backend if (!empty($imageprop)) { - $attachment = $rcmail->plugins->exec_hook('attachment_upload', [ + $attachment = [ 'path' => $filepath, 'size' => $_FILES['_file']['size'][$i], 'name' => $_FILES['_file']['name'][$i], 'mimetype' => 'image/' . $imageprop['type'], 'group' => $type, - ]); - } - - if (!$err && !empty($attachment['status']) && empty($attachment['abort'])) { - $id = $attachment['id']; + ]; - // store new file in session - unset($attachment['status'], $attachment['abort']); - $rcmail->session->append($type . '.files', $id, $attachment); + $inserted = $rcmail->insert_uploaded_file($attachment); + } + if (!$err && $inserted) { + $id = $attachment['id']; $content = rcube::Q($attachment['name']); $rcmail->output->command('add2attachment_list', "rcmfile$id", [ diff --git a/program/actions/settings/upload_display.php b/program/actions/settings/upload_display.php index d3c02c38d..1cf973db2 100644 --- a/program/actions/settings/upload_display.php +++ b/program/actions/settings/upload_display.php @@ -28,21 +28,16 @@ class rcmail_action_settings_upload_display extends rcmail_action */ public function run($args = []) { - $from = rcube_utils::get_input_string('_from', rcube_utils::INPUT_GET); - $type = preg_replace('/(add|edit)-/', '', $from); - - // Plugins in Settings may use this file for some uploads (#5694) - // Make sure it does not contain a dot, which is a special character - // when using rcube_session::append() below - $type = str_replace('.', '-', $type); - - $id = 'undefined'; - - if (preg_match('/^rcmfile(\w+)$/', $_GET['_file'], $regs)) { + if (!empty($_GET['_file']) && preg_match('/^rcmfile(\w+)$/', $_GET['_file'], $regs)) { $id = $regs[1]; } + else { + exit; + } + + $file = rcmail::get_instance()->get_uploaded_file($id); - self::display_uploaded_file($_SESSION[$type]['files'][$id]); + self::display_uploaded_file($file); exit; } diff --git a/program/include/rcmail.php b/program/include/rcmail.php index f2ec6358b..bb13cdc73 100644 --- a/program/include/rcmail.php +++ b/program/include/rcmail.php @@ -28,6 +28,8 @@ */ class rcmail extends rcube { + use rcube_uploads; + /** * Main tasks. * @@ -1869,18 +1871,6 @@ class rcmail extends rcube return rcmail_action::upload_form($attrib, $name, $action, $input_attr, $max_size); } - /** - * Outputs uploaded file content (with image thumbnails support - * - * @param array $file Upload file data - * - * @deprecated since 1.5-beta, use rcmail_action::display_uploaded_file() - */ - public function display_uploaded_file($file) - { - rcmail_action::display_uploaded_file($file); - } - /** * Initializes client-side autocompletion. * diff --git a/program/include/rcmail_action.php b/program/include/rcmail_action.php index 5d211fb10..7d29972e8 100644 --- a/program/include/rcmail_action.php +++ b/program/include/rcmail_action.php @@ -377,12 +377,13 @@ abstract class rcmail_action return; } - $lang_codes = [$_SESSION['language']]; + $language = $_SESSION['language'] ?? 'en_US'; + $lang_codes = [$language]; $assets_dir = $rcmail->config->get('assets_dir') ?: INSTALL_PATH; $skin_path = $rcmail->output->get_skin_path(); - if ($pos = strpos($_SESSION['language'], '_')) { - $lang_codes[] = substr($_SESSION['language'], 0, $pos); + if ($pos = strpos($language, '_')) { + $lang_codes[] = substr($language, 0, $pos); } foreach ($lang_codes as $code) { @@ -636,67 +637,8 @@ abstract class rcmail_action */ public static function display_uploaded_file($file) { - if (empty($file)) { - return; - } - - $rcmail = rcmail::get_instance(); - - $file = $rcmail->plugins->exec_hook('attachment_display', $file); - - if (!empty($file['status'])) { - if (empty($file['size'])) { - $file['size'] = !empty($file['data']) ? strlen($file['data']) : @filesize($file['path']); - } - - // generate image thumbnail for file browser in HTML editor - if (!empty($_GET['_thumbnail'])) { - $thumbnail_size = 80; - $mimetype = $file['mimetype']; - $file_ident = $file['id'] . ':' . $file['mimetype'] . ':' . $file['size']; - $thumb_name = 'thumb' . md5($file_ident . ':' . $rcmail->user->ID . ':' . $thumbnail_size); - $cache_file = rcube_utils::temp_filename($thumb_name, false, false); - - // render thumbnail image if not done yet - if (!is_file($cache_file)) { - if (!$file['path']) { - $orig_name = $filename = $cache_file . '.tmp'; - file_put_contents($orig_name, $file['data']); - } - else { - $filename = $file['path']; - } - - $image = new rcube_image($filename); - if ($imgtype = $image->resize($thumbnail_size, $cache_file, true)) { - $mimetype = 'image/' . $imgtype; - - if (!empty($orig_name)) { - unlink($orig_name); - } - } - } - - if (is_file($cache_file)) { - // cache for 1h - $rcmail->output->future_expire_header(3600); - header('Content-Type: ' . $mimetype); - header('Content-Length: ' . filesize($cache_file)); - - readfile($cache_file); - exit; - } - } - - header('Content-Type: ' . $file['mimetype']); - header('Content-Length: ' . $file['size']); - - if (isset($file['data']) && is_string($file['data'])) { - echo $file['data']; - } - else if (!empty($file['path'])) { - readfile($file['path']); - } + if (!empty($file)) { + rcmail::get_instance()->display_uploaded_file($file, !empty($_GET['_thumbnail'])); } } diff --git a/program/include/rcmail_attachment_handler.php b/program/include/rcmail_attachment_handler.php index 2ee02e07f..91105d422 100644 --- a/program/include/rcmail_attachment_handler.php +++ b/program/include/rcmail_attachment_handler.php @@ -95,9 +95,9 @@ class rcmail_attachment_handler else if ($file_id && $compose_id) { $file_id = preg_replace('/^rcmfile/', '', $file_id); - if (($compose = $_SESSION['compose_data_' . $compose_id]) - && ($this->upload = $compose['attachments'][$file_id]) - ) { + $this->upload = $rcube->get_uploaded_file($file_id); + + if ($this->upload) { $this->filename = $this->upload['name']; $this->mimetype = $this->upload['mimetype']; $this->size = $this->upload['size']; diff --git a/program/lib/Roundcube/rcube.php b/program/lib/Roundcube/rcube.php index 31fba2eeb..d7d082772 100644 --- a/program/lib/Roundcube/rcube.php +++ b/program/lib/Roundcube/rcube.php @@ -506,8 +506,8 @@ class rcube } /** - * Garbage collector function for temp files. - * Removes temporary files older than temp_dir_ttl. + * Garbage collector function for temporary files. + * Removes temporary files and upload information older than temp_dir_ttl. */ public function gc_temp() { @@ -520,6 +520,11 @@ class rcube $temp_dir_ttl = 6*3600; // 6 hours sensible lower bound. } + // Uploaded files metadata + $db = $this->get_dbh(); + $db->query("DELETE FROM " . $db->table_name('uploads', true) + . " WHERE `created` < " . $db->now($temp_dir_ttl * -1)); + $expire = time() - $temp_dir_ttl; if ($tmp && ($dir = opendir($tmp))) { @@ -727,8 +732,8 @@ class rcube */ public function read_localization($dir, $lang = null) { - if ($lang == null) { - $lang = $_SESSION['language']; + if (empty($lang)) { + $lang = $_SESSION['language'] ?? 'en_US'; } $langs = array_unique(['en_US', $lang]); $locdir = slashify($dir); @@ -1563,6 +1568,16 @@ class rcube self::write_log($dest, sprintf("%s: %0.4f sec", $label, $diff)); } + /** + * Getter for session identifier. + * + * @return string Session ID + */ + public function get_session_id() + { + return defined('ROUNDCUBE_TEST_SESSION') ? ROUNDCUBE_TEST_SESSION : session_id(); + } + /** * Setter for system user object * diff --git a/program/lib/Roundcube/rcube_plugin.php b/program/lib/Roundcube/rcube_plugin.php index f528b5e79..9cd5c2faa 100644 --- a/program/lib/Roundcube/rcube_plugin.php +++ b/program/lib/Roundcube/rcube_plugin.php @@ -222,7 +222,7 @@ abstract class rcube_plugin $add[$domain.'.'.$key] = $value; } - $rcube->load_language($_SESSION['language'], $add); + $rcube->load_language($_SESSION['language'] ?? null, $add); // add labels to client if ($add2client && method_exists($rcube->output, 'add_label')) { diff --git a/program/lib/Roundcube/rcube_uploads.php b/program/lib/Roundcube/rcube_uploads.php new file mode 100644 index 000000000..4f9133280 --- /dev/null +++ b/program/lib/Roundcube/rcube_uploads.php @@ -0,0 +1,285 @@ + | + +-----------------------------------------------------------------------+ +*/ + +/** + * A trait providing access to metadata of files uploaded in a session. + * + * @package Framework + * @subpackage Core + */ +trait rcube_uploads +{ + /** + * Get an uploaded file information. + * + * @param int $id Upload ID + * + * @return array|null Hash array with file upload metadata, NULL if not found + */ + public function get_uploaded_file($id) + { + if (!($session_id = $this->get_session_id())) { + return null; + } + + $sql_result = $this->db->query( + "SELECT * FROM " . $this->db->table_name('uploads', true) + . " WHERE `session_id` = ? AND `upload_id` = ?", + $session_id, $id + ); + + if ($sql_arr = $this->db->fetch_assoc($sql_result)) { + $sql_arr['id'] = $sql_arr['upload_id']; + $sql_arr += json_decode($sql_arr['metadata'], true); + unset($sql_arr['upload_id'], $sql_arr['session_id'], $sql_arr['metadata']); + + return $sql_arr; + } + } + + /** + * Return a list of all uploaded files (in the current session). + * + * @param string $group The upload context group + * + * @return array List of uploaded files + */ + public function list_uploaded_files($group) + { + if (!($session_id = $this->get_session_id())) { + return []; + } + + $sql_result = $this->db->query( + "SELECT * FROM " . $this->db->table_name('uploads', true) + . " WHERE `session_id` = ? AND `group` = ?" + . " ORDER BY `created`", + $session_id, $group + ); + + $result = []; + + while ($sql_arr = $this->db->fetch_assoc($sql_result)) { + $sql_arr['id'] = $sql_arr['upload_id']; + $sql_arr += json_decode($sql_arr['metadata'], true); + unset($sql_arr['upload_id'], $sql_arr['session_id'], $sql_arr['metadata']); + $result[] = $sql_arr; + } + + return $result; + } + + /** + * Update a specific upload record. + * + * @param int $id Upload ID + * @param array $data Hash array with col->value pairs to update + * + * @return bool True if saved successfully, false if nothing changed + */ + public function update_uploaded_file($id, $data) + { + if (!($file = $this->get_uploaded_file($id))) { + return false; + } + + $metadata = $this->prepare_upload_metadata(array_merge($file, $data)); + + $sql = "UPDATE " . $this->db->table_name('uploads', true) + . " SET `metadata` = ?" + . " WHERE `upload_id` = ? AND `session_id` = ?"; + + $update = $this->db->query($sql, $metadata, $id, $this->get_session_id()); + + return $this->db->affected_rows($update) > 0; + } + + /** + * Create a new uploaded file record in the session. + * + * @param array $data Hash array with col->value pairs to save + * It must include: id, group, name, data (or path), size. + * It can include: mimetype, charset, content_id or any other properties. + * It will be modified by the attachment's handling plugin(s). + * @param string $hook Optional plugin API hook name (attachment_upload or attachment_save) + * + * @return bool True on success or False on error + */ + public function insert_uploaded_file(&$data, $hook = null) + { + if (!($session_id = $this->get_session_id())) { + return false; + } + + $data = $this->plugins->exec_hook($hook ?: 'attachment_upload', $data); + + if (empty($data['status']) || !empty($data['abort'])) { + return false; + } + + $metadata = $this->prepare_upload_metadata($data); + + $sql = "INSERT INTO " . $this->db->table_name('uploads', true) + . " (`created`, `session_id`, `upload_id`, `group`, `metadata`)" + . " VALUES (" . $this->db->now() . ", ?, ?, ?, ?)"; + + $insert = $this->db->query($sql, $session_id, $data['id'], $data['group'] ?? null, $metadata); + + return $this->db->affected_rows($insert) > 0; + } + + /** + * Delete an upload record + * + * @param int $id Upload ID + * + * @return bool True if deleted successfully, False otherwise + */ + public function delete_uploaded_file($id) + { + if (!($session_id = $this->get_session_id())) { + return false; + } + + if ($attachment = $this->get_uploaded_file($id)) { + $attachment = $this->plugins->exec_hook('attachment_delete', $attachment); + } + + if (empty($attachment['status'])) { + return false; + } + + $this->db->query( + "DELETE FROM " . $this->db->table_name('uploads', true) + . " WHERE `session_id` = ? AND `upload_id` = ?", + $session_id, + $id + ); + + return $this->db->affected_rows() > 0; + } + + /** + * Delete all upload records (in the current session) by group name. + * + * @param string $group Group name + * + * @return bool True if deleted successfully, False otherwise + */ + public function delete_uploaded_files($group) + { + if (!($session_id = $this->get_session_id())) { + return false; + } + + $this->plugins->exec_hook('attachments_cleanup', ['group' => $group]); + + $this->db->query( + "DELETE FROM " . $this->db->table_name('uploads', true) + . " WHERE `session_id` = ? AND `group` = ?", + $session_id, + $group + ); + + return $this->db->affected_rows() > 0; + } + + /** + * Outputs uploaded file content (with image thumbnails support) + * + * @param array $file Uploaded file data + * @param bool $thumbnail Generate and return a thumbnail image + */ + public function display_uploaded_file($file, $thumbnail = false) + { + $file = $this->plugins->exec_hook('attachment_display', $file); + + if (!empty($file['status'])) { + if (empty($file['size'])) { + $file['size'] = !empty($file['data']) ? strlen($file['data']) : @filesize($file['path']); + } + + // generate image thumbnail for file browser in HTML editor + if ($thumbnail) { + $thumbnail_size = 80; + $mimetype = $file['mimetype']; + $file_ident = $file['id'] . ':' . $file['mimetype'] . ':' . $file['size']; + $thumb_name = 'thumb' . md5($file_ident . ':' . $this->user->ID . ':' . $thumbnail_size); + $cache_file = rcube_utils::temp_filename($thumb_name, false, false); + + // render thumbnail image if not done yet + if (!is_file($cache_file)) { + if (empty($file['path'])) { + $orig_name = $filename = $cache_file . '.tmp'; + file_put_contents($orig_name, $file['data']); + } + else { + $filename = $file['path']; + } + + $image = new rcube_image($filename); + if ($imgtype = $image->resize($thumbnail_size, $cache_file, true)) { + $mimetype = 'image/' . $imgtype; + + if (!empty($orig_name)) { + unlink($orig_name); + } + } + } + + if (is_file($cache_file)) { + // cache for 1h + $this->output->future_expire_header(3600); + header('Content-Type: ' . $mimetype); + header('Content-Length: ' . filesize($cache_file)); + + readfile($cache_file); + exit; + } + } + + header('Content-Type: ' . $file['mimetype']); + header('Content-Length: ' . $file['size']); + + if (isset($file['data']) && is_string($file['data'])) { + echo $file['data']; + } + else if (!empty($file['path'])) { + readfile($file['path']); + } + } + } + + /** + * Prepare upload metadata to store in DB + */ + protected function prepare_upload_metadata($data) + { + // Remove unwanted properties + $data = array_diff_key($data, array_fill_keys(['id', 'group', 'status', 'abort', 'error', 'data', 'created'], 1)); + + // Remove null values + $data = array_filter($data, function ($v) { return !is_null($v); }); + + // Convert to string + $data = json_encode($data, JSON_INVALID_UTF8_IGNORE); + + return $data; + } +} diff --git a/program/lib/Roundcube/rcube_user.php b/program/lib/Roundcube/rcube_user.php index cb421d38f..365505ec4 100644 --- a/program/lib/Roundcube/rcube_user.php +++ b/program/lib/Roundcube/rcube_user.php @@ -11,8 +11,8 @@ | See the README file for a full license statement. | | | | PURPOSE: | - | This class represents a system user linked and provides access | - | to the related database records. | + | This class represents a user and provides access to the related | + | database records. | +-----------------------------------------------------------------------+ | Author: Thomas Bruederli | | Author: Aleksander Machniak | @@ -20,7 +20,7 @@ */ /** - * Class representing a system user + * Class representing a user * * @package Framework * @subpackage Core @@ -212,7 +212,7 @@ class rcube_user $save_prefs = serialize($save_prefs); if (!$no_session) { - $this->language = $_SESSION['language']; + $this->language = $_SESSION['language'] ?? 'en_US'; } $this->db->query( diff --git a/tests/ActionTestCase.php b/tests/ActionTestCase.php index 9474bc120..5122d6bea 100644 --- a/tests/ActionTestCase.php +++ b/tests/ActionTestCase.php @@ -168,6 +168,81 @@ class ActionTestCase extends PHPUnit\Framework\TestCase return $file; } + /** + * Prepare fake file upload handling + */ + protected function fakeUpload($name = '_file', $is_array = true, $error = 0) + { + $content = base64_decode(rcmail_output::BLANK_GIF); + $file = [ + 'name' => 'test.gif', + 'type' => 'image/gif', + 'tmp_name' => $this->createTempFile($content), + 'error' => $error, + 'size' => strlen($content), + 'id' => 'i' . microtime(true), + ]; + + // Attachments handling plugins use move_uploaded_file() which does not work + // here. We'll add a fake hook handler for our purposes. + $rcmail = rcmail::get_instance(); + $rcmail->plugins->register_hook('attachment_upload', function($att) use ($file) { + $att['status'] = true; + $att['id'] = $file['id']; + return $att; + }); + + $_FILES = []; + + if ($is_array) { + $_FILES[$name] = [ + 'name' => [$file['name']], + 'type' => [$file['type']], + 'tmp_name' => [$file['tmp_name']], + 'error' => [$file['error']], + 'size' => [$file['size']], + 'id' => [$file['id']], + ]; + } + else { + $_FILES[$name] = $file; + } + + return $file; + } + + /** + * Create file upload record + */ + protected function fileUpload($group) + { + $content = base64_decode(rcmail_output::BLANK_GIF); + $file = [ + 'name' => 'test.gif', + 'type' => 'image/gif', + 'size' => strlen($content), + 'group' => $group, + 'id' => 'i' . microtime(true), + ]; + + // Attachments handling plugins use move_uploaded_file() which does not work + // here. We'll add a fake hook handler for our purposes. + $rcmail = rcmail::get_instance(); + $rcmail->plugins->register_hook('attachment_upload', function($att) use ($file) { + $att['status'] = true; + $att['id'] = $file['id']; + return $att; + }); + + $rcmail->insert_uploaded_file($file); + + $upload = rcube::get_instance()->get_uploaded_file($file['id']); + + $this->assertTrue(is_array($upload)); + + return $upload; + } + /** * Load an execute specified SQL script */ diff --git a/tests/Actions/Contacts/UploadPhoto.php b/tests/Actions/Contacts/UploadPhoto.php index ffd144e84..733eac401 100644 --- a/tests/Actions/Contacts/UploadPhoto.php +++ b/tests/Actions/Contacts/UploadPhoto.php @@ -30,35 +30,22 @@ class Actions_Contacts_Upload_Photo extends ActionTestCase $this->assertTrue(strpos($result['exec'], 'this.photo_upload_end();') !== false); // Upload a file - $content = base64_decode(rcmail_output::BLANK_GIF); - $file = $this->createTempFile($content); - $_SESSION['contacts'] = null; - $_FILES['_photo'] = [ - 'name' => 'test.gif', - 'type' => 'image/gif', - 'tmp_name' => $file, - 'error' => 0, - 'size' => strlen($content), - ]; - - // Attachments handling plugins use move_uploaded_file() which does not work - // here. We'll add a fake hook handler for our purposes. - $rcmail = rcmail::get_instance(); - $rcmail->plugins->register_hook('attachment_upload', function($att) { - $att['status'] = true; - $att['id'] = 'fake'; - return $att; - }); + $file = $this->fakeUpload('_photo', false); $this->runAndAssert($action, OutputJsonMock::E_EXIT); $result = $output->getOutput(); + $this->assertSame(['Content-Type: application/json; charset=UTF-8'], $output->headers); $this->assertSame('upload-photo', $result['action']); - $this->assertTrue(strpos($result['exec'], 'this.replace_contact_photo("fake");') !== false); + $this->assertTrue(strpos($result['exec'], 'this.replace_contact_photo("' . $file['id'] . '");') !== false); $this->assertTrue(strpos($result['exec'], 'this.photo_upload_end();') !== false); - $this->assertSame('test.gif', $_SESSION['contacts']['files']['fake']['name']); + $upload = rcube::get_instance()->get_uploaded_file($file['id']); + $this->assertSame($file['name'], $upload['name']); + $this->assertSame($file['type'], $upload['mimetype']); + $this->assertSame($file['size'], $upload['size']); + $this->assertSame('contact', $upload['group']); // TODO: Test invalid image format, upload errors handling } diff --git a/tests/Actions/Mail/AttachmentDelete.php b/tests/Actions/Mail/AttachmentDelete.php index 945de8fcc..c75a604c2 100644 --- a/tests/Actions/Mail/AttachmentDelete.php +++ b/tests/Actions/Mail/AttachmentDelete.php @@ -8,12 +8,47 @@ class Actions_Mail_AttachmentDelete extends ActionTestCase { /** - * Class constructor + * Test uploaded attachment delete */ - function test_class() + function test_run() { - $object = new rcmail_action_mail_attachment_delete; + $rcmail = rcube::get_instance(); + $action = new rcmail_action_mail_attachment_delete; + $output = $this->initOutput(rcmail_action::MODE_AJAX, 'mail', 'delete-attachment'); - $this->assertInstanceOf('rcmail_action', $object); + $this->assertInstanceOf('rcmail_action', $action); + $this->assertTrue($action->checks()); + + // First we create the upload record + $file = $this->fileUpload('101'); + + // Test list_uploaded_files(), just because + $list = $rcmail->list_uploaded_files('101'); + + $this->assertSame([$file], $list); + + // This is needed so upload deletion works + $rcmail = rcmail::get_instance(); + unset($rcmail->plugins->handlers['attachment_delete']); + $rcmail->plugins->register_hook('attachment_delete', function($att) { + $att['status'] = true; + $att['break'] = true; + return $att; + }); + + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_SESSION = ['compose_data_101' => ['test' => 'test']]; + + // Invoke the delete action + $_POST = ['_id' => '101', '_file' => 'rcmfile' . $file['id']]; + $this->runAndAssert($action, OutputJsonMock::E_EXIT); + + $result = $output->getOutput(); + + $this->assertSame(['Content-Type: application/json; charset=UTF-8'], $output->headers); + $this->assertSame('delete-attachment', $result['action']); + $this->assertSame('this.remove_from_attachment_list("rcmfile' . $file['id'] . '");', trim($result['exec'])); + + $this->assertNull(rcube::get_instance()->get_uploaded_file($file['id'])); } } diff --git a/tests/Actions/Mail/AttachmentRename.php b/tests/Actions/Mail/AttachmentRename.php index dde8593a8..8474cd451 100644 --- a/tests/Actions/Mail/AttachmentRename.php +++ b/tests/Actions/Mail/AttachmentRename.php @@ -8,12 +8,35 @@ class Actions_Mail_AttachmentRename extends ActionTestCase { /** - * Class constructor + * Test uploaded attachment rename */ - function test_class() + function test_run() { - $object = new rcmail_action_mail_attachment_rename; + $rcmail = rcube::get_instance(); + $action = new rcmail_action_mail_attachment_rename; + $output = $this->initOutput(rcmail_action::MODE_AJAX, 'mail', 'rename-attachment'); - $this->assertInstanceOf('rcmail_action', $object); + $this->assertInstanceOf('rcmail_action', $action); + $this->assertTrue($action->checks()); + + // First we create the upload record + $file = $this->fileUpload('100'); + + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_SESSION = ['compose_data_100' => ['test' => 'test']]; + + // Invoke the rename action + $_POST = ['_id' => '100', '_file' => 'rcmfile' . $file['id'], '_name' => 'mod.gif']; + $this->runAndAssert($action, OutputJsonMock::E_EXIT); + + $result = $output->getOutput(); + + $this->assertSame(['Content-Type: application/json; charset=UTF-8'], $output->headers); + $this->assertSame('rename-attachment', $result['action']); + $this->assertSame('this.rename_attachment_handler("rcmfile' . $file['id'] . '","mod.gif");', trim($result['exec'])); + + $upload = rcube::get_instance()->get_uploaded_file($file['id']); + $this->assertSame($_POST['_name'], $upload['name']); + $this->assertSame($_POST['_id'], $upload['group']); } } diff --git a/tests/Actions/Mail/AttachmentUpload.php b/tests/Actions/Mail/AttachmentUpload.php index 00b65ba6b..f45f44acf 100644 --- a/tests/Actions/Mail/AttachmentUpload.php +++ b/tests/Actions/Mail/AttachmentUpload.php @@ -5,15 +5,78 @@ * * @package Tests */ -class Actions_Mail_Attachmentupload extends ActionTestCase +class Actions_Mail_AttachmentUpload extends ActionTestCase { /** - * Class constructor + * Test file upload */ - function test_class() + function test_run() { - $object = new rcmail_action_mail_attachment_upload; + $action = new rcmail_action_mail_attachment_upload; + $output = $this->initOutput(rcmail_action::MODE_AJAX, 'mail', 'upload'); - $this->assertInstanceOf('rcmail_action', $object); + $this->assertInstanceOf('rcmail_action', $action); + $this->assertTrue($action->checks()); + + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_GET = [ + '_id' => '123', + '_uploadid' => 'upload123', + ]; + + $_SESSION = ['compose_data_123' => ['test' => 'test']]; + + // No files uploaded case + $this->runAndAssert($action, OutputJsonMock::E_EXIT); + + $result = $output->getOutput(); + + $this->assertSame(['Content-Type: application/json; charset=UTF-8'], $output->headers); + $this->assertSame('upload', $result['action']); + $this->assertTrue(strpos($result['exec'], 'this.remove_from_attachment_list("upload123");') !== false); + $this->assertTrue(strpos($result['exec'], 'this.auto_save_start(false);') !== false); + + // Upload a file + $_SESSION = ['compose_data_123' => ['test' => 'test']]; + + $file = $this->fakeUpload('_attachments'); + + $this->runAndAssert($action, OutputJsonMock::E_EXIT); + + $result = $output->getOutput(); + + $this->assertSame(['Content-Type: application/json; charset=UTF-8'], $output->headers); + $this->assertSame('upload', $result['action']); + $this->assertTrue(strpos($result['exec'], 'this.add2attachment_list("rcmfile' . $file['id'] . '"') !== false); + $this->assertTrue(strpos($result['exec'], 'this.auto_save_start(false);') !== false); + + $upload = rcube::get_instance()->get_uploaded_file($file['id']); + $this->assertSame($file['name'], $upload['name']); + $this->assertSame($file['type'], $upload['mimetype']); + $this->assertSame($file['size'], $upload['size']); + $this->assertSame($_GET['_id'], $upload['group']); + + // Upload error case + $_SESSION = ['compose_data_123' => ['test' => 'test']]; + $file = $this->fakeUpload('_attachments', true, UPLOAD_ERR_INI_SIZE); + + $this->runAndAssert($action, OutputJsonMock::E_EXIT); + + $result = $output->getOutput(); + + $this->assertSame(['Content-Type: application/json; charset=UTF-8'], $output->headers); + $this->assertSame('upload', $result['action']); + $this->assertTrue(strpos($result['exec'], 'this.display_message("The uploaded file exceeds the maximum size') !== false); + $this->assertTrue(strpos($result['exec'], 'this.auto_save_start(false);') !== false); + + // TODO: Test max_message_size handling + } + + /** + * Test file upload via URI + */ + function test_uri() + { + $this->markTestIncomplete(); } } diff --git a/tests/Actions/Settings/Upload.php b/tests/Actions/Settings/Upload.php index a89ddded5..3fe4bd390 100644 --- a/tests/Actions/Settings/Upload.php +++ b/tests/Actions/Settings/Upload.php @@ -8,12 +8,59 @@ class Actions_Settings_Upload extends ActionTestCase { /** - * Class constructor + * Test file uploads */ - function test_class() + function test_run() { - $object = new rcmail_action_settings_upload; + $action = new rcmail_action_settings_upload; + $output = $this->initOutput(rcmail_action::MODE_AJAX, 'settings', 'upload'); - $this->assertInstanceOf('rcmail_action', $object); + $this->assertInstanceOf('rcmail_action', $action); + $this->assertTrue($action->checks()); + + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_GET = [ + '_from' => 'add-identity', + '_unlock' => 'loading123', + '_uploadid' => 'upload123', + ]; + + // No files uploaded case + $this->runAndAssert($action, OutputJsonMock::E_EXIT); + + $result = $output->getOutput(); + + $this->assertSame(['Content-Type: application/json; charset=UTF-8'], $output->headers); + $this->assertSame('upload', $result['action']); + $this->assertTrue(strpos($result['exec'], 'this.remove_from_attachment_list("upload123");') !== false); + $this->assertSame($_GET['_unlock'], $result['unlock']); + + // Upload a file + $file = $this->fakeUpload(); + + $this->runAndAssert($action, OutputJsonMock::E_EXIT); + + $result = $output->getOutput(); + + $this->assertSame(['Content-Type: application/json; charset=UTF-8'], $output->headers); + $this->assertSame('upload', $result['action']); + $this->assertTrue(strpos($result['exec'], 'this.add2attachment_list("rcmfile' . $file['id'] . '"') !== false); + + $upload = rcube::get_instance()->get_uploaded_file($file['id']); + $this->assertSame($file['name'], $upload['name']); + $this->assertSame($file['type'], $upload['mimetype']); + $this->assertSame($file['size'], $upload['size']); + $this->assertSame('identity', $upload['group']); + + // Upload error case + $file = $this->fakeUpload('_file', true, UPLOAD_ERR_INI_SIZE); + + $this->runAndAssert($action, OutputJsonMock::E_EXIT); + + $result = $output->getOutput(); + + $this->assertSame(['Content-Type: application/json; charset=UTF-8'], $output->headers); + $this->assertSame('upload', $result['action']); + $this->assertTrue(strpos($result['exec'], 'this.display_message("The uploaded file exceeds the maximum size') !== false); } } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index e0204d589..da3e13b0d 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -25,6 +25,7 @@ if (php_sapi_name() != 'cli') { if (!defined('INSTALL_PATH')) define('INSTALL_PATH', realpath(__DIR__ . '/..') . '/' ); define('ROUNDCUBE_TEST_MODE', true); +define('ROUNDCUBE_TEST_SESSION', microtime(true)); define('TESTS_DIR', __DIR__ . '/'); if (@is_dir(TESTS_DIR . 'config')) { -- cgit v1.2.3