Improve attachment file handling: use one new function to create a temp
authorkink <kink@7612ce4b-ef26-0410-bec9-ea0150e637f0>
Sun, 7 Jan 2007 17:30:13 +0000 (17:30 +0000)
committerkink <kink@7612ce4b-ef26-0410-bec9-ea0150e637f0>
Sun, 7 Jan 2007 17:30:13 +0000 (17:30 +0000)
file for storing the attachment. This replaces the same code in five
places. It also improves on the code, it's now much more safe against
overwriting existing attachments by chance.

git-svn-id: https://svn.code.sf.net/p/squirrelmail/code/trunk/squirrelmail@12097 7612ce4b-ef26-0410-bec9-ea0150e637f0

ChangeLog
functions/compose.php
functions/mailbox_display.php
plugins/spamcop/functions.php
plugins/spamcop/spamcop.php
src/compose.php
src/read_body.php
src/right_main.php
src/search.php

index 66bfd16..c605b57 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -171,6 +171,7 @@ Version 1.5.2 - CVS
   - Drop obsolete ORDB RBL from filters plugin (#1629398).
   - Add warning about magic_quotes_* in configtest.
   - Unify accepted versions for imap_server_type and set_defaults (#1629722).
+  - Improve attachment temp file creation.
 
 Version 1.5.1 (branched on 2006-02-12)
 --------------------------------------
index 068fac0..f64e49d 100644 (file)
  */
 
 
+/**
+ * Get a new file to write an attachment to.
+ * This function makes sure it doesn't overwrite other attachments,
+ * preventing collisions and race conditions.
+ *
+ * @return filename
+ * @since 1.5.2
+ */
+function sq_get_attach_tempfile()
+{
+    global $username, $attachment_dir;
+
+    $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
+
+    // using PHP >= 4.3.2 we can be truly atomic here
+    $filemods = check_php_version ( 4,3,2 ) ? 'x' : 'w';
+
+    // give up after 1000 tries
+    $TMP_MAX = 1000;
+    for ($try=0; $try<$TMP_MAX; ++$try) {
+
+        $localfilename = GenerateRandomString(32, '', 7);
+        $full_localfilename = "$hashed_attachment_dir/$localfilename";
+
+        // filename collision. try again
+        if ( file_exists($full_localfilename) ) {
+            continue;
+        }
+
+        // try to open for (binary) writing
+        $fp = @fopen( $full_localfilename, $filemods);
+
+        if ( $fp !== FALSE ) {
+            // success! make sure it's not readable, close and return filename
+            chmod($full_localfilename, 0600);
+            fclose($fp);
+            return $full_localfilename;
+        }
+    }
+
+    // we tried 1000 times but didn't succeed.
+    error_box( _("Could not open temporary file to store attachment. Contact your system administrator to resolve this issue.") );
+    return FALSE;
+}
+
+
index 280c187..625d48a 100644 (file)
@@ -1477,9 +1477,6 @@ function handleMessageListForm($imapConnection,&$aMailbox,$sButton='',$aUid = ar
  * @author Marc Groot Koerkamp
  */
 function attachSelectedMessages($imapConnection,$aMsgHeaders) {
-    global $username, $attachment_dir,
-           $data_dir;
-
 
     sqgetGlobalVar('composesession', $composesession, SQ_SESSION);
     sqgetGlobalVar('compose_messages', $compose_messages, SQ_SESSION);
@@ -1496,8 +1493,6 @@ function attachSelectedMessages($imapConnection,$aMsgHeaders) {
         sqsession_register($composesession,'composesession');
     }
 
-    $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
-
     $composeMessage = new Message();
     $rfc822_header = new Rfc822Header();
     $composeMessage->rfc822_header = $rfc822_header;
@@ -1517,14 +1512,13 @@ function attachSelectedMessages($imapConnection,$aMsgHeaders) {
             $body = implode('', $body_a);
             $body .= "\r\n";
 
-            $localfilename = GenerateRandomString(32, 'FILE', 7);
-            $full_localfilename = "$hashed_attachment_dir/$localfilename";
-
-            $fp = fopen( $full_localfilename, 'wb');
+            $filename = sq_get_attach_tempfile();
+            $fp = fopen($filename, 'wb');
             fwrite ($fp, $body);
             fclose($fp);
+
             $composeMessage->initAttachment('message/rfc822',$subject.'.msg',
-                 $full_localfilename);
+                 $filename);
         }
     }
 
@@ -1532,3 +1526,4 @@ function attachSelectedMessages($imapConnection,$aMsgHeaders) {
     sqsession_register($compose_messages,'compose_messages');
     return $composesession;
 }
+
index e21fbfe..687a5b6 100644 (file)
@@ -180,9 +180,7 @@ function spamcop_enable_disable($option,$disable_action,$enable_action) {
  */
 function spamcop_getMessage_RFC822_Attachment($message, $composeMessage, $passed_id,
                                       $passed_ent_id='', $imapConnection) {
-    global $attachment_dir, $username;
 
-    $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
     if (!$passed_ent_id) {
         $body_a = sqimap_run_command($imapConnection,
                                     'FETCH '.$passed_id.' RFC822',
@@ -198,21 +196,12 @@ function spamcop_getMessage_RFC822_Attachment($message, $composeMessage, $passed
         array_shift($body_a);
         $body = implode('', $body_a) . "\r\n";
 
-        $localfilename = GenerateRandomString(32, 'FILE', 7);
-        $full_localfilename = "$hashed_attachment_dir/$localfilename";
-        $fp = fopen( $full_localfilename, 'w');
+        $filename = sq_get_attach_tempfile();
+        $fp = fopen($filename, 'wb');
         fwrite ($fp, $body);
         fclose($fp);
-
-        /* dirty relative dir fix */
-        if (substr($attachment_dir,0,3) == '../') {
-           $attachment_dir = substr($attachment_dir,3);
-           $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
-        }
-        $full_localfilename = "$hashed_attachment_dir/$localfilename";
-
         $composeMessage->initAttachment('message/rfc822','email.txt',
-                         $full_localfilename);
+                         $filename);
     }
     return $composeMessage;
 }
index b72ee0c..de4ce27 100644 (file)
@@ -23,6 +23,8 @@ include_once(SM_PATH . 'functions/imap_messages.php');
 /* plugin functions */
 include_once(SM_PATH . 'plugins/spamcop/functions.php');
 
+include_once(SM_PATH . 'functions/compose.php');
+
 /* GLOBALS */
 
 sqgetGlobalVar('mailbox', $mailbox, SQ_GET);
index de1c2f0..3c03f9f 100644 (file)
@@ -26,6 +26,7 @@ require_once(SM_PATH . 'functions/imap_general.php');
 require_once(SM_PATH . 'functions/imap_messages.php');
 require_once(SM_PATH . 'functions/date.php');
 require_once(SM_PATH . 'functions/mime.php');
+require_once(SM_PATH . 'functions/compose.php');
 require_once(SM_PATH . 'class/deliver/Deliver.class.php');
 require_once(SM_PATH . 'functions/addressbook.php');
 require_once(SM_PATH . 'functions/forms.php');
@@ -974,8 +975,8 @@ function newMail ($mailbox='', $passed_id='', $passed_ent_id='', $action='', $se
  * @return object
  */
 function getAttachments($message, &$composeMessage, $passed_id, $entities, $imapConnection) {
-    global $attachment_dir, $username, $data_dir, $squirrelmail_language, $languages;
-    $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
+    global $squirrelmail_language, $languages;
+
     if (!count($message->entities) ||
             ($message->type0 == 'message' && $message->type1 == 'rfc822')) {
         if ( !in_array($message->entity_id, $entities) && $message->entity_id) {
@@ -1003,19 +1004,14 @@ function getAttachments($message, &$composeMessage, $passed_id, $entities, $imap
                     function_exists($languages[$squirrelmail_language]['XTRA_CODE'] . '_encode')) {
                 $filename =  call_user_func($languages[$squirrelmail_language]['XTRA_CODE'] . '_encode', $filename);
             }
-            $localfilename = GenerateRandomString(32, '', 7);
-            $full_localfilename = "$hashed_attachment_dir/$localfilename";
-            while (file_exists($full_localfilename)) {
-                $localfilename = GenerateRandomString(32, '', 7);
-                $full_localfilename = "$hashed_attachment_dir/$localfilename";
-            }
-            $message->att_local_name = $full_localfilename;
+            $localfilename = sq_get_attach_tempfile();
+            $message->att_local_name = $localfilename;
 
             $composeMessage->initAttachment($message->type0.'/'.$message->type1,$filename,
-                    $full_localfilename);
+                    $localfilename);
 
             /* Write Attachment to file */
-            $fp = fopen ("$hashed_attachment_dir/$localfilename", 'wb');
+            $fp = fopen ($localfilename, 'wb');
             mime_print_body_lines ($imapConnection, $passed_id, $message->entity_id, $message->header->encoding, $fp);
             fclose ($fp);
         }
@@ -1029,8 +1025,6 @@ function getAttachments($message, &$composeMessage, $passed_id, $entities, $imap
 
 function getMessage_RFC822_Attachment($message, $composeMessage, $passed_id,
         $passed_ent_id='', $imapConnection) {
-    global $attachment_dir, $username, $data_dir;
-    $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
     if (!$passed_ent_id) {
         $body_a = sqimap_run_command($imapConnection,
                 'FETCH '.$passed_id.' RFC822',
@@ -1048,14 +1042,12 @@ function getMessage_RFC822_Attachment($message, $composeMessage, $passed_id,
         array_pop($body_a);
         $body = implode('', $body_a) . "\r\n";
 
-        $localfilename = GenerateRandomString(32, 'FILE', 7);
-        $full_localfilename = "$hashed_attachment_dir/$localfilename";
-
-        $fp = fopen($full_localfilename, 'w');
+        $localfilename = sq_get_attach_tempfile();
+        $fp = fopen($localfilename, 'wb');
         fwrite ($fp, $body);
         fclose($fp);
         $composeMessage->initAttachment('message/rfc822',$subject.'.msg',
-                $full_localfilename);
+                $localfilename);
     }
     return $composeMessage;
 }
@@ -1381,33 +1373,26 @@ function checkInput ($show) {
 
 /* True if FAILURE */
 function saveAttachedFiles($session) {
-    global $_FILES, $attachment_dir, $username,
-        $data_dir, $compose_messages;
+    global $compose_messages;
 
     /* get out of here if no file was attached at all */
     if (! is_uploaded_file($_FILES['attachfile']['tmp_name']) ) {
         return true;
     }
 
-    $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
-    $localfilename = GenerateRandomString(32, '', 7);
-    $full_localfilename = "$hashed_attachment_dir/$localfilename";
-    while (file_exists($full_localfilename)) {
-        $localfilename = GenerateRandomString(32, '', 7);
-        $full_localfilename = "$hashed_attachment_dir/$localfilename";
-    }
+    $localfilename = sq_get_attach_tempfile();
 
     // m_u_f works better with restricted PHP installs (safe_mode, open_basedir),
     // if that doesn't work, try a simple rename.
-    if (!@move_uploaded_file($_FILES['attachfile']['tmp_name'],$full_localfilename)) {
-        if (!@rename($_FILES['attachfile']['tmp_name'], $full_localfilename)) {
+    if (!@move_uploaded_file($_FILES['attachfile']['tmp_name'],$localfilename)) {
+        if (!@rename($_FILES['attachfile']['tmp_name'], $localfilename)) {
             return true;
         }
     }
     $message = $compose_messages[$session];
     $type = strtolower($_FILES['attachfile']['type']);
     $name = $_FILES['attachfile']['name'];
-    $message->initAttachment($type, $name, $full_localfilename);
+    $message->initAttachment($type, $name, $localfilename);
     $compose_messages[$session] = $message;
     sqsession_register($compose_messages , 'compose_messages');
 }
index 10c372b..7078c95 100644 (file)
@@ -27,6 +27,7 @@ require_once(SM_PATH . 'functions/identity.php');
 require_once(SM_PATH . 'functions/mailbox_display.php');
 require_once(SM_PATH . 'functions/forms.php');
 require_once(SM_PATH . 'functions/attachment_common.php');
+require_once(SM_PATH . 'functions/compose.php');
 
 /**
  * Given an IMAP message id number, this will look it up in the cached
index 196342a..85fe06c 100644 (file)
@@ -27,6 +27,7 @@ require_once(SM_PATH . 'functions/imap_messages.php');
 require_once(SM_PATH . 'functions/date.php');
 require_once(SM_PATH . 'functions/mime.php');
 require_once(SM_PATH . 'functions/mailbox_display.php');
+require_once(SM_PATH . 'functions/compose.php');
 
 
 /* lets get the global vars we may need */
index 0b1623b..2ecbf1f 100644 (file)
@@ -31,6 +31,7 @@ require_once(SM_PATH . 'functions/mime.php');
 require_once(SM_PATH . 'functions/mailbox_display.php'); //getButton()
 require_once(SM_PATH . 'functions/forms.php');
 require_once(SM_PATH . 'functions/date.php');
+require_once(SM_PATH . 'functions/compose.php');
 
 /** Prefs array ordinals. Must match $recent_prefkeys and $saved_prefkeys
  */