dev/core#2588 - Crash when sending to 3+ email recipients with an attachment in 5.36+

[demeritcowboy]

Overview

https://lab.civicrm.org/dev/core/-/issues/2588

Tentative fix - I haven't fully tested it but wanted to put it up given the timing.

Before

1. Create an email activity (not a mass mailing) to 3 or more people.
2. Add one or more attachments.
3. Send the email.
4. Crashes while sending to the 3rd recipient.

After

No crash

Technical Details

array_merge_recursive ends up creating an array for the id on the 3rd iteration. $attachmentFileIds will look something like this on the 2nd and all following iterations:

array(
 'attachFile_1' => array('id' => 46),
 'attachFile_2' => array('id' => 47),
)

So there's no merge the first time, and on the 2nd time $attachments doesn't have an id member yet so the merge is fine. But then just before it's merged into $attachments on the 3rd time $attachments looks something like:

array(
  'attachFile_1' => array(
    'uri' => 'something',
    'type' => 'something',
    // ...
    'id' => 46,
  ),
  'attachFile_2' => array(
    'uri' => 'something',
    'type' => 'something',
    // ...
    'id' => 47,
  ),
)

so it converts the id member into e.g. array(46, 46) because that's what array_merge_recursive does.

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

Thanks for jumping on this @demeritcowboy - do you know when it regressed?

[demeritcowboy]

It started in 5.36.0 - it's from #18299

[totten]

• General standards
  • (r-explain) Pass
  • (r-user) Pass
  • (r-doc) Pass
  • (r-run) Pass: I've r-run with and without the patch - reproduced the original problem and confirmed this fixes it. Also, with the fixed branch, I did some r-run with various permutations of:
    * Initiating the email from "Search Results" and from "View Contact"
    * Sending to 1, 2, 3, and 5 contacts
    * Attaching 0, 1, and 2 files
    All looked good. ✅
• Developer standards
  • (r-tech) Pass
  • (r-code) Pass
  • (r-maint) Pass: There's a related PR dev/core#2588 - failing test for PR 20222 #20227. The test does reproduce an error in the old code -- and passes with the new code .:)
  • (r-test) Pass

[demeritcowboy]

Thanks for testing!
So it's still an unknown with 4 recipients but that's ok I'm on good speaking terms with the number 4 if something comes up.