From cd219a45dd64d7d6916656e5e1b3a4ead68974c2 Mon Sep 17 00:00:00 2001
From: Aleksander Machniak <alec@alec.pl>
Date: Fri, 18 Dec 2015 05:42:05 -0500
Subject: [PATCH] Fix regression where some message attachments could be missing on edit/forward (#1490608)

---
 CHANGELOG                               |    1 +
 program/lib/Roundcube/rcube_session.php |    6 ++++--
 program/steps/mail/func.inc             |   23 ++++++++++++++++++++++-
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index ee79237..3eabdb4 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 CHANGELOG Roundcube Webmail
 ===========================
 
+- Fix regression where some message attachments could be missing on edit/forward (#1490608)
 - Fix regression in displaying contents of message/rfc822 parts (#1490606)
 - Fix handling of message/rfc822 attachments on replies and forwards (#1490607)
 - Fix PDF support detection in Firefox > 19 (#1490610)
diff --git a/program/lib/Roundcube/rcube_session.php b/program/lib/Roundcube/rcube_session.php
index 23b1d33..d087182 100644
--- a/program/lib/Roundcube/rcube_session.php
+++ b/program/lib/Roundcube/rcube_session.php
@@ -293,6 +293,8 @@
     /**
      * Append the given value to the certain node in the session data array
      *
+     * Warning: Do not use if you already modified $_SESSION in the same request (#1490608)
+     *
      * @param string Path denoting the session variable where to append the value
      * @param string Key name under which to append the new value (use null for appending to an indexed list)
      * @param mixed  Value to append to the session data array
@@ -319,8 +321,9 @@
         $this->appends[] = $path;
 
         // when overwriting a previously unset variable
-        if ($this->unsets[$path])
+        if ($this->unsets[$path]) {
             unset($this->unsets[$path]);
+        }
     }
 
     /**
@@ -398,7 +401,6 @@
                 }
             }
         }
-
     }
 
     /**
diff --git a/program/steps/mail/func.inc b/program/steps/mail/func.inc
index 0f1a87a..5436368 100644
--- a/program/steps/mail/func.inc
+++ b/program/steps/mail/func.inc
@@ -2203,6 +2203,8 @@
 
 function rcmail_save_attachment($message, $pid, $compose_id, $params = array())
 {
+    global $COMPOSE;
+
     $rcmail  = rcmail::get_instance();
     $storage = $rcmail->get_storage();
 
@@ -2265,7 +2267,26 @@
 
     if ($attachment['status']) {
         unset($attachment['data'], $attachment['status'], $attachment['content_id'], $attachment['abort']);
-        $rcmail->session->append('compose_data_' . $compose_id . '.attachments', $attachment['id'], $attachment);
+
+        $session_key = 'compose_data_' . $compose_id;
+
+        // 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') {
+            $_SESSION[$session_key]['attachments'][$attachment['id']] = $attachment;
+        }
+        else {
+            $rcmail->session->append($session_key . '.attachments', $attachment['id'], $attachment);
+        }
+
+        // Fix reference to compose session data after rcube_session::append()
+        $COMPOSE =& $_SESSION[$session_key];
+
         return $attachment;
     }
     else if ($path) {

--
Gitblit v1.9.1