phpmailer failing for certain devices

Question

Hi I have quite a large PhpMailer script that uploads 9 files and emails a HTML application form.

It gets sent via a pretty standard AJAX script. I have tested it on various devices and platforms and it all works fine. In fact I cant break it whatever I try to do however, my client's tenants seem to have found a way to break it.

They say they have used it and as far as they were concerned it sent successfully however there is no record of the email being sent or received or any of the files being uploaded to the server.

Here is the full script minus some form fields and also details of a connection to a database for spam checking.

if ( isset($_POST['email']) && isset($_POST['name']) && filter_var($_POST['email'], FILTER_VALIDATE_EMAIL) ) {

    // detect & prevent header injections
    $test = "/(content-type|bcc:|cc:|to:)/i";
    foreach ( $_POST as $key => $val ) {
        if ( preg_match( $test, $val ) ) {
            exit;
        }
    }


    $dateKey = date( 'd-m-Y--H-i-s' );




    $my_email = "control@XXXXXXXXXXXX.com";
    ob_start();

    require("smtp/class.phpmailer.php");

    $mail = new PHPMailer();



    $mail->IsSMTP();
    $mail->Host = "mail.XXXXXXXX.com";
    $mail->SMTPAuth = true;
    $mail->Username = $my_email;
    $mail->Password = "XXXXXXXXXXX";



    $mail->From = $mail->Username;

    $mail->FromName = $_POST['name'];
    $mail->Sender = $_POST['email'];



    function clean($string) {
        $string = str_replace(' ', '-', $string);

        return preg_replace('/[^A-Za-z0-9\-]/', '', $string);
    }

    if(isset($_FILES)) {

        $uploadOk = 1;
        $fileString = '';

        $fileMessage = 'FILEERROR(';

        $files = $_FILES;

        $target_dir = $_SERVER['DOCUMENT_ROOT'] . "/XXXXXXXXXX/uploads/";



        foreach ( $_FILES as $key => $file ) {

            $imageFileExt = strtolower( pathinfo( $file["name"], PATHINFO_EXTENSION ) );

            $file['name'] = clean($_POST['name']). "_" . $key . "_" . $dateKey . "." . $imageFileExt;


            $target_file = $target_dir . basename($file["name"]);
            $imageFileType = strtolower(pathinfo($target_file,PATHINFO_EXTENSION));



            $check = getimagesize($file["tmp_name"]);
            if($check === false) {
                $fileMessage .=  $key."=noimage,";
                $uploadOk = 0;
            }

// Allow certain file formats

            else if($imageFileType !== "jpg" && $imageFileType !== "png" && $imageFileType !== "jpeg"
                    && $imageFileType !== "gif" ) {
                $fileMessage .=  $key."=wrongfile,";
                $uploadOk = 0;
            }

// Check if file already exists
            else if (file_exists($target_file)) {
                $fileMessage .=  $key."=fileexists,";
                $uploadOk = 0;
            }


// Check file size
            else if ($file["size"] > 20000000) { //20mb
                $fileMessage .=  $key."=toobig,";
                $uploadOk = 0;
            }

            $fileString .= strtoupper($key).": <a href='http://www.XXXXXXXXXXXX.com/XXXXXXXXX/uploads/".$file['name']."'>".$file['name']."</a><br>";


        }

        $fileMessage .= ')';



    }





    $mail->CharSet = 'utf-8';
    $mail->Encoding = 'quoted-printable';



    $bcc = "xxx@xxxxx.com";
    $mail->AddBCC($bcc);

    $mail->AddReplyTo($_POST['email']);


    $mail->WordWrap = 50;



    $mail->Body     = "<p><strong>APPLICATION<br /></strong></p>

<p>Property ".$_POST['address']."<br />

<div style='background:#f1f1f1;padding:5px 15px; margin-bottom:20px;'><p><strong>APPLICANT DETAILS:<br /></strong></p><p>
Name: ".$_POST['name']."<br />

Email: ".$_POST['email']."<br />

Telephone: ".$_POST['tel']."<br />

Date of birth: ".$_POST['DOB']."<br />

National insurance number: ".$_POST['NI']."<br /></p></div>

<div style='background:#f1f1f1;padding:5px 15px; margin-bottom:20px;'><p><strong>ADDRESS<br /></strong></p><p>

Address: ".$_POST['address']."<br />
Time at address: ".$_POST['addLength']."<br />
Reason to move: ".$_POST['move']."<br />";


    ///more fields added to body here but not necessary to show



    $mail->Body.="<div style='background:#f1f1f1;padding:5px 15px; margin-bottom:20px;'><p><strong>FILE ATTACHMENTS:<br /></strong></p><p>".$fileString."</p></div>";



    $mail->IsHTML(true);

    $mail->Subject = 'Application';



    /* my own validation */
    $formerrors = array();
    $errorstring = "";


    ///connects to database here, details removed but checks against spam keywords and creates an array of $formerrors





    $conn->close();



    if (sizeof($formerrors) > 0){

        $errorstring = "(" ;

        foreach($formerrors as $key=>$value){

            if($y < sizeof($formerrors) ){


                $errorstring .= $value.",";
                $y++;

            } else{

                $errorstring .= $value.")";

            }


        }

        echo $errorstring;


        #### file errors ####

    } else if($uploadOk === 0){

        echo $fileMessage;

    }
    else {


            $mail->AddAddress("XXX@XXXXX.com", 'recipient');

        ///send here
        if ($mail->Send() == true) {

            if ($uploadOk === 1) {

                if(isset($_FILES)) {


                    $uploadfiles = $_FILES;


                    // Compress image
                    function compressImage($source, $destination, $quality) {

                        $info = getimagesize($source);

                        if ($info['mime'] == 'image/jpeg') {
                            $image = imagecreatefromjpeg($source);
                        } elseif ($info['mime'] == 'image/gif') {
                            $image = imagecreatefromgif($source);
                        } elseif ($info['mime'] == 'image/png') {
                            $image = imagecreatefrompng($source);
                        }

                        imagejpeg($image, $destination, $quality);

                    }

                    foreach ( $uploadfiles as $key => $upfile ) {


                        $imageFileType = strtolower( pathinfo( $upfile['name'], PATHINFO_EXTENSION ) );




                        $fileName = clean($_POST['name']). "_" . $key . "_" . $dateKey . "." . $imageFileType;


                        $target_file = $target_dir . basename( $fileName );


                        $img_dir = "img/";

                        compressImage($upfile["tmp_name"], $target_dir . basename( $fileName ), 60);


                    }
                }
            }

            echo 'Message sent successfully';




        }
        else {
            echo "failed";

        }



    }


}

This looks like you may not be following the [PHP docs on handling uploads safely](https://www.php.net/manual/en/features.file-upload.php). Have a look at [the PHPMailer "send file upload" example](https://github.com/PHPMailer/PHPMailer/blob/master/examples/send_file_upload.phps). This is an extension of a question you asked earlier – break down your problem and solve one piece at a time; you're trying to do the whole thing at once, but you need to check whether each piece works by itself first. Add more logging! — Synchro, Oct 07 '20 at 22:05
I'm sorry I don't understand which bit is unsafe? I did use move_uploaded_file() originally but found I needed to swap it with compressImage() so that I could store a compressed image. Is that it or is it because I'm sending the email then storing the files when your example script does it the other way around? — cannon303, Oct 07 '20 at 22:25
Thanks for that Martin, the problem is though that when I set it to tmp_name my script flags it up as the wrong file type. — cannon303, Oct 08 '20 at 09:39
@cannon303 right then, that's not a PhpMailer error. You need to save to your error log what the file type is, if this matches what you expect and update your PHP comparison operator to match this outcome. — Martin, Oct 08 '20 at 09:41
In your original question you didn't show your use of `move_uploaded_file`, just that you were accessing `$_FILES` directly, hence my suspicion. You need to validate `$_FILES` before using *anything* it contains – so I don't see how you can choose to compress an image before you have processed its uploaded file safely: handle the upload, then compress the image, not the other way around, and not one instead of the other. When you send the email is a completely independent decision if it doesn't involve the image files. — Synchro, Oct 08 '20 at 09:47

Martin · Answer 1 · 2020-10-08T13:55:10.150

There are some classic mistakes in your code.

The issue is not PhpMailer.

Here is a summary of how to fix your most obvious problems:

Ensure you have the most up to date version of PhpMailer.
You don't seem to have any error detection on your Phpmailer... use PhpMailer Exceptions to find out if there's a sending issue within PhpMailer.
Use PhpMailer's extensive built in erorr logging with ->debugOutoput and ->SMTPDebug to detect emailer errors.
Turn on your PHP error logging to track errors, issues and notices and check your error logs regularly.
You Need to use PHP Error Logging! I copy and run your script and immediately come up with various notices and issues that you should have found right at the start.
If your host is not localhost you may need to check and confirm you are using the correct port, for example using $mail->Port = 587; instead of $mail->Port = 25 (default, I believe).
If your sending domain is another domain on the same server (as it looks like a mailing subdomain), then it's far easier simply to use "localhost" and avoid the whole authentication workload.
You seem to have misunderstood how PHP handled multiple file uploads , you seem to be expecting $_FILES[x]['name'] but what you're actually being given is $_FILES['name'][x]. You need to reorganise your $_FILES code to this end.
There is also a PHPMailer example for handling multiple uploads.
on your pathinfo call you should be testing the path to the tmp_name of the actual file and NOT the given original name. This is a serious security flaw.
It is advisable to clean $_POSTed data. Your $_POST['email'] at least should be run through FILTER_SANITIZE_EMAIL
Turn on PHP error logging
Do not trust your mechanism in compressImage to detect the correct image type. You should be using fileinfo on the source file rather than simply trusting the given type data from the $_FILES array which can be very easily manipulated by the unploadee.
If you complete all of the above and there is NOT a sending issue with PhpMailer, then use your server logs to check that the email sending program (sendmail or similar) actually received the sending request from PHP and what it did with it.
Your server logs will also outline sending issues from the server side (once PHP passes the data to your sending program).
You must remember that Sending is absolutely no guarentee of delivery, you should run 3rd party checks to ensure your server is correctly set up to send email, and includes basic spam tag avoidance techniques such as SPF, DKIM, DMARC, etc.
Check your PHP error logs

There is also [a PHPMailer example for handling multiple uploads](https://github.com/PHPMailer/PHPMailer/blob/master/examples/send_multiple_file_upload.phps). — Synchro, Oct 08 '20 at 09:49
@Synchro excellent. I'm going to have more bullet points here than an M16 `:-)` — Martin, Oct 08 '20 at 09:50
Thanks @Martin for some very detailed feedback. It looks like back to the drawing board and a steep curve lays ahead. I'll start by updating phpMailer although I have never been very confident with composer and git. As far as the error logs are concerned they flag up lots of things that are depreciated with class.phpmailer.php - are those the issues you mention when you said you ran my code? I'll learn what is actually happening with $_FILES. Then try the rest of your bullet points. — cannon303, Oct 08 '20 at 13:12
@cannon303 I don't use composer. Simply download the current master from github and install it manually, works just as well `:-)`. Good luck with it. If this answer fixes your issues please feel free to tick the tick mark and +1 `:)` — Martin, Oct 08 '20 at 13:54
I don't seem to be using require_once '../class.phpmailer.php'; but instead using: use PHPMailer\PHPMailer\PHPMailer; use PHPMailer\PHPMailer\SMTP; use PHPMailer\PHPMailer\Exception; require 'folder/Exception.php'; require 'folder/PHPMailer.php'; require 'folder/SMTP.php'; - is that correct? — cannon303, Oct 08 '20 at 15:52
@cannon303 I can't read your comment clearly but yes, that looks like the kind of shape I'd expect. — Martin, Oct 08 '20 at 15:54
I am sending each file in their own field so $_FILES[x]['name'] was a red herring. — cannon303, Oct 08 '20 at 18:32

phpmailer failing for certain devices

1 Answers1