Likely logical bug in Piler_Mime_Decode::removeJournal() in mime.php

[David Requena]

This method is called from from ControllerMessageRestore.

Here' a snippet from the code as it stands in piler 1.3.11.

      $has_journal = 0;

      $s = self::remove_LF($message);
      if(strpos($s, $EOL . $EOL)) {
         list($headers, $body) = explode($EOL . $EOL, $s, 2);
         if(strstr($headers, "\nX-MS-Journal-Report:")) {
            return $has_journal;
         }
      }

As far as I can tell, the check in the second if (line 4) should be if( ! strstr($headers, "\nX-MS-Journal-Report:")).

As it stands, if the X-MS-Journal-Report header is found, the journal is not removed and the method returns with 0 ($has_journal = 0).

As it happens this is the very same header preventing restorations to O365 in my other open issue.

[David Requena]

Went ahead and “fixed” this in our piler deployment.

It fixed issue #1209 without any adverse side effect detected at the moment

[Janos SUTO]

I can see. I think the correct code should be

      $has_journal = 0;

      $s = self::remove_LF($message);
      if(strpos($s, $EOL . $EOL)) {
         list($headers, $body) = explode($EOL . $EOL, $s, 2);
         if(strstr($headers, "\nX-MS-Journal-Report:")) {
            $has_journal = 1;
            return $has_journal;
         }
      }

Can you verify it?

[David Requena]

I will but… there is further busyness down this method actually removing the outer journal email (everything up to the first message/rfc822 appearance) before returning with $has_journal=1. Because strstr call in line 12, this is is actually dead code, and it will remain dead code after your proposed change.

I’m afraid returning that early, although with a correct value, would leave that code dead and and the outer email in-place. Hence, one attachment layer too much again.

At any rate, ControllerMessageRestore is not doing anything with this return value not even storing it in variable.

Here’s the method in full:

   public static function removeJournal(&$message, $EOL = "\n") {
      $has_journal = 0;

      $s = self::remove_LF($message);
      if(strpos($s, $EOL . $EOL)) {
         list($headers, $body) = explode($EOL . $EOL, $s, 2);
         if(strstr($headers, "\nX-MS-Journal-Report:")) {
            return $has_journal;
         }
      }

      $p = strstr($message, "\nX-MS-Journal-Report:");
      if($p) {
         $q = stristr($p, "message/rfc822");
         if($q) {
            $has_journal = 1;
            $i = strlen("message/rfc822");
            while(ctype_space($q[$i])) { $i++; }
            if($i > 0) { $message = substr($q, $i); }
         }
      }

      return $has_journal;
   }

I’ll be reporting back.

---

EDIT POST-VERIFICATION

yeah, return value is of no consequence. Simply correcting it does not fix the broader issue.

On the other hand, allowing the removal code to run gets us a perfectly restored email in O365.

[Janos SUTO]

Can you send me a non-sensitive email with full headers including the journaling stuff to my email address? See piler -V for it.

[David Requena]

Just sent requested emails, both successfully restored and failed.

BTW, the user list may be there but it seems not to be working. At least it’s not possible to to subscribe as server reports [e-mail] is not a valid recipient.

[Janos SUTO]

I took a deeper look, and you are right. The function should return if the journal header X-MS-Journal-Report: is not found. I’ve negated check as you suggested, thank you for the fix.

[David Requena]

You’re welcome. This closes both issues I guess.

I could finally subscribe to the list BTW, so another issue closed

[David Requena]

Resolved as per last Janos comment.

[Frank Schmirler]

With this fix applied, a bug in the journal removal code shows up. The final MIME boundary of the journal is not removed.

Best way to see the effect is by archiving a simple non-MIME text mail via journal:

Without the fix:

• Downloading or restoring the mail gives you the mail in journal format
• Display in piler search interface is fine (i.e. journal wrapper is removed correctly)

Now with the fix:

• Downloading or restoring the mail is removing the journal wrapper - except for the terminating MIME boundary
• Display in piler search interface displays the rogue MIME boundary below the mail text

I didn’t notice problems with journaled MIME messages yet. The rogue boundary is there but probably ignored as a MIME epilogue.

‌

[Janos SUTO]

Ehh, then I reopen the issue, and investigate it further.

[Janos SUTO]

I’d like the both of you to test this commit: https://bitbucket.org/jsuto/piler/commits/1970fc4d8c944b384f3e72666b07dfa37eea1eb4

I hope it still fixes this issue, and not producing any regression Frank mentioned.

[Frank Schmirler]

Looks good. Thanks!

[Janos SUTO]

Then the issue has been resolved.