Incorrect use of PHP method compact

[maweder]

This is a follow-up to #18926.

$mail_error_data = compact( 'to', 'subject', 'message', $headers, $attachments );

instead of ...

$mail_error_data = compact( $to, $subject, $message, $headers, $attachments );

[dd32]

The existing format is how PHP's compact() method works - ​http://php.net/compact

[johnbillion]

This is a valid bug. The current code uses variable references instead of variable names.

[Ankit K Gupta]

added patch

[Ankit K Gupta]

@maweder Welcome to the trac! Thanks for reporting the bug, I have added patch as per your suggestion.

[Ankit K Gupta]

Updated patch file

[SergeyBiryukov]

Introduced in [34221].

[SergeyBiryukov]

In 36688:

Mail: Correct compact() usage in wp_mail().

Props Ankit K Gupta, maweder.
Fixes #35781 for trunk.

[maweder]

Replying to SergeyBiryukov:

In 36688:

Mail: Correct compact() usage in wp_mail().

Props Ankit K Gupta, maweder.
Fixes #35781 for trunk.

Hi Sergey, the change in 36688 is again incorrect.

From ​http://php.net/manual/en/function.compact.php: Each parameter can be either a string containing the name of the variable, or an array of variable names. The array can contain other arrays of variable names inside it; compact() handles it recursively.

$headers and $attachments, these are arrays and not names of variables. The correct code:

$mail_error_data = compact( 'to', 'subject', 'message', $headers, $attachments );

Thanks & Greets

[shahpranaf]

@maweder Firstly, Welcome to the WP trac.

Yes you are correct. 'headers' and 'attachments' are arrays and not the strings.
I've done changes at both places i.e inside wp_mail() function also.

Thanks

[ocean90]

"needs-unit-test" so it doesn't happen again.

[dd32]

Not to sound like a broken record, but the current code in trunk is correct.

$to = '[email protected]';
$from = '[email protected]';
$headers = array( 'Header', 'Two Header' );
$Header = 'Should not appear';

var_dump( compact( 'to', 'from', 'headers' ) );

will result in :

array (size=3)
  'to' => string '[email protected]' (length=14)
  'from' => string '[email protected]' (length=16)
  'headers' => 
    array (size=2)
      0 => string 'Header' (length=6)
      1 => string 'Two Header' (length=10)

Whereas var_dump( compact( 'to', 'from', $headers ) ); would result in:

array (size=3)
  'to' => string '[email protected]' (length=14)
  'from' => string '[email protected]' (length=16)
  'Header' => string 'Should not appear' (length=17)

The array handling in compact() is to pass an array of variable names, when the variable you want in the output is itself a array it should be specified as a string.

[johnbillion]

Confirmed, [36688] fixed the issue and the values passed to the wp_mail_failed action are as expected. A unit test would help us out here though.

[maweder]

My mistake. You're right.

Replying to dd32:

Not to sound like a broken record, but the current code in trunk is correct.

$to = '[email protected]';
$from = '[email protected]';
$headers = array( 'Header', 'Two Header' );
$Header = 'Should not appear';

var_dump( compact( 'to', 'from', 'headers' ) );

will result in :

array (size=3)
  'to' => string '[email protected]' (length=14)
  'from' => string '[email protected]' (length=16)
  'headers' => 
    array (size=2)
      0 => string 'Header' (length=6)
      1 => string 'Two Header' (length=10)

Whereas var_dump( compact( 'to', 'from', $headers ) ); would result in:

array (size=3)
  'to' => string '[email protected]' (length=14)
  'from' => string '[email protected]' (length=16)
  'Header' => string 'Should not appear' (length=17)

The array handling in compact() is to pass an array of variable names, when the variable you want in the output is itself a array it should be specified as a string.

[SergeyBiryukov]

In 37081:

Mail: Correct compact() usage in wp_mail().

Merges [36688] to the 4.4 branch.

Props Ankit K Gupta, maweder.
Fixes #35781.