WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 8 weeks ago

#28407 new enhancement

You are unable to override the attachment name in wp_mail() when adding attachments

Reported by: syntaxart Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.9.1
Component: Mail Keywords: has-patch has-unit-tests 2nd-opinion needs-testing dev-feedback
Focuses: Cc:
PR Number:

Description

Hi,

When you create an email and add an attachment your may have obtained the attachment filepath using the file upload control. This means you will have a .tmp file extension on your email attachment. When the user receives the email, the attachment is not useful. In order to get the attachment to work correctly you need to first add the tmp filepath and then override the filename. PHPMailer allows you to do this with the AddAttachment function. The wordpress wp_mail function does not allow for this to happen.

The problem with wp_mail() in wp-includes/pluggable.php is at line 483

if ( !empty( $attachments ) ) {
		foreach ( $attachments as $attachment ) {
			try {
				$phpmailer->AddAttachment($attachment);
			} catch ( phpmailerException $e ) {
				continue;
			}
		}
	}

I overcame this problem by copying the function and then replacing the above block of code with

if (!empty($attachments)) {
                foreach ($attachments as $name => $attachment) {
                    try {
                        $phpmailer->AddAttachment($attachment, $name);
                    } catch (phpmailerException $e) {
                        continue;
                    }
                }
            }

The wp_mail function is an important core function and in my opinion is best not to override.

Is it possible to enhance the wp_mail function for this. I have read forums on the internet which has pointed to the issue above. It would be good to offer some of the features which phpmailer is offering.

Thank you very much.

Ryan

Attachments (3)

28407.diff (912 bytes) - added by rittesh.patel 5 years ago.
Check for whether it is associative array or sequential array for attachments
28407.2.diff (2.2 KB) - added by swissspidy 23 months ago.
Refreshed patch with unit tests
28407-alternative.diff (3.3 KB) - added by swissspidy 23 months ago.
Alternative patch that allows changing every argument of addAttachment()

Download all attachments as: .zip

Change History (21)

#1 @rittesh.patel
5 years ago

You need to check that whether it is associative array or sequential array.

@rittesh.patel
5 years ago

Check for whether it is associative array or sequential array for attachments

#2 @syntaxart
5 years ago

Thank you Rittesh. Will this change be added to the next version of WordPress.

#3 @syntaxart
5 years ago

I have placed your solution into a function and have used the add_action hook 'phpmailer_init'. This works perfectly fine for this. Thank you for your help.

#4 follow-up: @SergeyBiryukov
5 years ago

#28782 was marked as a duplicate.

#5 in reply to: ↑ 4 @p_j_albuquerque
5 years ago

Replying to SergeyBiryukov:

#28782 was marked as a duplicate.

It is not possible to upload multiple file attachment.
It would be possible to change.

if ( !empty( $attachments ) ) {

foreach ( $attachments as $attachment ) {

try {

if(is_array($attachment)){

list($name, $file) = each($attachment);
$phpmailer->AddAttachment($file, $name);

}else{

$phpmailer->AddAttachment($attachment);

}

} catch ( phpmailerException $e ) {

continue;

}

}

}

Exemple:

$attachments[] = array(

$_FILES[arquivo][name] => $_FILES[arquivo][tmp_name],

);

Version 3, edited 5 years ago by p_j_albuquerque (previous) (next) (diff)

#6 @stephenharris
4 years ago

  • Keywords has-patch needs-unit-tests needs-codex added

#7 @ragulka
3 years ago

The patch by @rittesh.patel looks solid - the best part is that it does not change the $attachments structure, so it's 100% backwards compatible. Would love to see it in the next release :)

#8 @swissspidy
23 months ago

  • Keywords needs-codex removed
  • Milestone changed from Awaiting Review to Future Release

@swissspidy
23 months ago

Refreshed patch with unit tests

@swissspidy
23 months ago

Alternative patch that allows changing every argument of addAttachment()

#9 @swissspidy
23 months ago

  • Keywords has-unit-tests added; needs-unit-tests removed
  • Milestone changed from Future Release to 5.0

I just uploaded 28407.2.diff as a refreshed patch of the original idea, along with unit tests.

While I was at it, I also created 28407-alternative.diff, which would allow developers to override every argument for an attachment. An example from the unit test:

wp_mail( 'user@example.org', 'Subject', 'Hello World', '', array(
        array(
                'path' => DIR_TESTDATA . '/images/canola.jpg',
                'name' => 'myawesomeimage.jpg',
                'encoding' => '8bit',
                'type' => 'image/jpeg',
                'disposition' => 'attachment',
        ),
        array(
                'path' => DIR_TESTDATA . '/images/waffles.jpg',
                'name' => 'foobar.jpg',
                'encoding' => 'base64',
                'type' => 'image/png',
                'disposition' => 'inline',
        ),
) );

I'm not 100% this is a good idea, since it makes it very easy to mess something up. But it would certainly be more future-proof than using an associative array.

#10 @johnbillion
13 months ago

  • Milestone changed from 5.0 to 5.1

#11 @pento
10 months ago

  • Milestone changed from 5.1 to 5.2

#12 @desrosj
8 months ago

  • Keywords 2nd-opinion needs-testing added

The latest patch needs review/testing, and could benefit from a second opinion.

#13 @desrosj
8 months ago

  • Milestone changed from 5.2 to 5.3

#14 @Grandy
6 months ago

The latest patch looks good and should be implemented ASAP. The idea of using an array with even more options seems amazing and should be considered as future improvement.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


2 months ago

#16 @davidbaumwald
2 months ago

  • Keywords dev-feedback added

#17 @garrett-eclipse
2 months ago

Hi @swissspidy this was discussed in bugscrub today on Slack here;
https://wordpress.slack.com/archives/C02RQBWTW/p1568264953282100

With concerns of the alternate providing too much control and potential for error would the .2.diff make sense for 5.3 and look into what extended options are potentially malicious and introduce them in a later enhancement?

#18 @davidbaumwald
8 weeks ago

  • Milestone changed from 5.3 to Future Release

This ticket still needs a decision, and with 5.3 Beta 1 only a few days away, this is being moved to Future Release.

Note: See TracTickets for help on using tickets.