Replies With Thread Pictures Failed To Send - edmozley/freeitsm GitHub Wiki

Replies with a picture in the thread failed to send

Reported in #158 Β· Broken in 2.10.0 Β· Fixed in #2081 Β· Ships in 2.10.1

This is a fault in the 2.10.0 fix for Reply attachments never reached the customer. πŸ› οΈ The code underneath: Outbound email: attachments and pictures β€” Developer Guide

Thanks to An Duong (@duongtuanan), who upgraded to 2.10.0, found the error in the log, traced it to the exact line, and tested a fix before reporting it.

What you saw

After upgrading to 2.10.0, some replies and forwards would not send and others went out normally. The analyst got an error, nothing reached the customer, and the server's PHP log said:

PHP Fatal error: Uncaught Error: Undefined constant "INLINE_THREAD_BUDGET"
in /var/www/html/api/tickets/send_email.php:437

It happened on every mail provider β€” Microsoft, Gmail and SMTP alike.

Which emails failed, and which went out

The deciding factor was the quoted thread underneath the reply:

The reply or forward… 2.10.0
quotes an earlier email on the ticket that had a picture in it (a logo in a signature counts) ❌ failed
quotes a thread with no pictures βœ… sent
is a brand-new email (no thread) βœ… sent
has a pasted screenshot or an attached file, but no picture in the thread βœ… sent

So it depended on the ticket, not the analyst or the mailbox β€” which is why it looked random. Customers whose email signatures carry a logo would have hit it on almost every ticket.

What was actually wrong

2.10.0 started embedding the thread's pictures inside the email, with a limit of 2 MB per email. That limit was written as a constant, placed next to the function that uses it β€” near the bottom of send_email.php:

// ...the code that sends, around line 150, calls processInlineImages()...

const INLINE_THREAD_BUDGET = 2 * 1024 * 1024;   // around line 389

function processInlineImages($body, $ticketId) {
    // ...
    if ($threadBytes + $size > INLINE_THREAD_BUDGET) {

PHP treats these two differently. A function anywhere in a file exists from the moment the file starts running. A top-level const only exists once PHP has actually run that line. This file does its work at the top and keeps its functions at the bottom, so by the time a reply was being sent, PHP had never reached line 389 β€” the constant did not exist yet.

The constant was only read when a picture from the ticket was found in the thread. No picture, no read, no error. That is the whole of the "some emails go out, some don't".

Why the tests did not catch it

tests/outbound-email-mime.php cannot run send_email.php as a whole β€” it is a web endpoint that sends the moment it is loaded β€” so it loaded just the bottom half, the functions. That bottom half begins above the constant, so in the test the constant was always defined before anything used it. The test passed every picture check, because it was not running the file the way the server does.

The live sends made while building the fix all went out β€” because none of them quoted a picture already stored on the ticket. Any one that had would have failed.

πŸ“Œ A test that loads only part of a file can pass on code that cannot run. The order things happen in is part of the code.

How it was fixed

  • The constant moved to the top of the file, under the require lines and above anything that sends, with a comment saying why it must stay there.
  • A fault embedding one picture can no longer stop the email. The picture code caught Exception, but a missing constant is an Error, which slipped past it. It now catches both (Throwable), and the picture keeps its link β€” the same thing that happens past the 2 MB limit.
  • The test checks the order. A new section reads the endpoint's source and fails if any constant is declared below the code that sends. Run against 2.10.0, it fails and names INLINE_THREAD_BUDGET.

A sweep of every other endpoint found no other file with a constant below its working code.

πŸ“ Files changed

File Change
api/tickets/send_email.php INLINE_THREAD_BUDGET declared at the top; the picture callback catches Throwable
tests/outbound-email-mime.php New section 0: every constant is declared above the code that sends; the constants are now loaded for the other checks the same way

How it was verified

The bug was reproduced first, through the real endpoint (api/tickets/send_email.php, posted to exactly as the reply box does). Nine throwaway tickets were made β€” three per provider β€” and on 2.10.0 every reply or forward whose thread held a picture failed with the reporter's exact error, on Microsoft, Gmail and SMTP. Nothing was sent.

With the fix, nine numbered emails were sent and received:

# Provider Scenario
1 Microsoft Reply, thread has a picture β€” the case that failed
2 Microsoft Reply, no pictures β€” the case that always worked
3 Microsoft Forward with a thread picture, a pasted screenshot and an attached file
4–6 Gmail The same three
7–9 SMTP The same three

php tests/outbound-email-mime.php β€” 29 checks pass; against 2.10.0's send_email.php the new section fails, as it should.

What this means for you

  • On 2.10.0? Upgrade to 2.10.1. Until you can, the reporter's own fix works: move the const INLINE_THREAD_BUDGET = 2 * 1024 * 1024; line in api/tickets/send_email.php up to just below the require_once lines.
  • Replies that failed were not sent, and were not saved on the ticket. The analyst was told it failed, so there is nothing hidden to find β€” but if one was not retried, the customer is still waiting for it.
  • Nothing to configure. Earlier versions are not affected; this was new in 2.10.0.

Related pages