You should have some sort of sanitizer for those user inputs though. This is the only real issue I see with your code. Take a look at filter_input filter_input if you have PHP 5.2 or greater, otherwise you'll have to take a look around at what others are doing. It is never a good idea to take raw user input and run it in your program. That can lead to all sorts of nasty things.
You should have some sort of sanitizer for those user inputs though. This is the only real issue I see with your code. Take a look at filter_input if you have PHP 5.2 or greater, otherwise you'll have to take a look around at what others are doing. It is never a good idea to take raw user input and run it in your program. That can lead to all sorts of nasty things.
You should have some sort of sanitizer for those user inputs though. This is the only real issue I see with your code. Take a look at filter_input if you have PHP 5.2 or greater, otherwise you'll have to take a look around at what others are doing. It is never a good idea to take raw user input and run it in your program. That can lead to all sorts of nasty things.
- 6.2k
- 1
- 16
- 27
You should have some sort of sanitizer for those user inputs though. This is the only real issue I see with your code. Take a look at filter_input if you have PHP 5.32 or greater, otherwise you'll have to take a look around at what others are doing. It is never a good idea to take raw user input and run it in your program. That can lead to all sorts of nasty things.
You should have some sort of sanitizer for those user inputs though. This is the only real issue I see with your code. Take a look at filter_input if you have PHP 5.3, otherwise you'll have to take a look around at what others are doing. It is never a good idea to take raw user input and run it in your program. That can lead to all sorts of nasty things.
You should have some sort of sanitizer for those user inputs though. This is the only real issue I see with your code. Take a look at filter_input if you have PHP 5.2 or greater, otherwise you'll have to take a look around at what others are doing. It is never a good idea to take raw user input and run it in your program. That can lead to all sorts of nasty things.
Don't know if you wanted a full review or not so I supplied one since I was going over it anyways. Not really much wrong, mostly aesthetics. I could find no errors in your code, but maybe one of these fixes will ninja your problem away. You never know, the all-powerful PHP god's might be feeling better today :)
POST Data
Not sure if better, but if I'm looking for $_POST variables, I usually just do the following. Up to you though, nothing wrong with what you've got.
if($_POST) { etc... }
You should have some sort of sanitizer for those user inputs though. This is the only real issue I see with your code. Take a look at filter_input if you have PHP 5.3, otherwise you'll have to take a look around at what others are doing. It is never a good idea to take raw user input and run it in your program. That can lead to all sorts of nasty things.
Ignoring Keys
If you have a list of keys, or other items that you want to exclude, make them into an array and then check against it. Its cleaner than having a bunch of if statements and allows for extension later.
$exceptions = array(
'hiddenField',
'button2',
'cloneAdmin',
);
if ($value !== "" && ! in_array($key, $exceptions)) { etc... }
Also, please use curly braces everywhere and not just on outter or large if statements. Yes PHP can work without them, but debugging your code will become very difficult later. Many might argue this point, but it truly is easier to read.
Since you are already checking if the value is blank, you might as well move that to the beginning and place all other checks inside it. What's the use of running all that code if the value is blank or a key you don't want to process? Better yet, just add the following to the top of the foreach loop and it will skip those records entirely.
if($value === '' || in_array($key, $exceptions)) { continue; }
Multiple If Statements
Don't use if statements for checking variables that you know a range of values for. Switch statements are faster and easier to read.
//Don't do this
if($key === 'DESCRIPTION_OF_ITEMS') { etc... }
if($key === 'EMAIL') { etc... }
//Do this
switch($key) {
case 'DESCRIPTION_OF_ITEMS':
//etc...
break;
case ''EMAIL':
//etc...
break;
}
Long Variables
Heredoc will make your long variables easier to read.
$content .= <<HTML
<h3>Thank you for submitting your registration form... we'll get back to you shortly.</h3>
<p>You can expect to receive an email with the following registration information:</p>
$emailMessage
HTML;
Long Strings as Arguments
If you are going to use long strings in functions you should replace them with variables, even if you aren't going to use those variables again. It makes it cleaner and easier to read.
$recipients = array();//You'll see why I used an array soon
$recipients[] = "[email protected]";
$recipients[] = "$sendTo";
$recipients = implode(', ', $recipients);//See :)
$subject = "Foobar Registration Form";
$headers = array();//Same as recipients
$headers[] = "MIME-Version: 1.0";
$headers[] = "Content-type: text/html;";//This line didn't originally have "\r\n" so maybe this was your problem?
$headers[] = "charset=iso-8859-1";
$headers[] = "From: $FROM_EMAIL";
$headers[] = "Reply-to: $email";
$headers = implode("\r\n", $headers);
mail($recipients, $subject, $emailMessage, $headers);//See much cleaner
Sorry I couldn't be of more use in finding your error. Since you are indeed receiving a success string, I would say that your send to line is just wrong somehow. Try dumping its contents before sending the message to manually check if anything is wrong. Only other thing I can think of is that maybe your server doesn't have the mail() function installed. Though, since you said it succeeded at some point, I don't know what to tell you. Check php_info() anyways to see if its enabled. As for if this is the wrong stackoverflow subsite? Not really. It could have gone on stackoverflow and no one would have complained. Here you just get the added benefit that I reviewed your code too :)