Match Email Address problem

[GrumpyYoungMan]

Code: [Select]

if( ( empty($email) || ! filter_var($email, FILTER_VALIDATE_EMAIL ) ) == ( empty($email1) || ! filter_var($email1, FILTER_VALIDATE_EMAIL ) ) ) {

$errors[] = "Email Addresses are Invalid";
}
I am trying to make sure that $email and $email are the same, I thought I would be clever and try to do it in the one statement and you guessed it I can't any ideas on how I can? Please?

[GrumpyYoungMan]

I was using the wrong operator!

Code: [Select]

if( ( empty($email) || ! filter_var($email, FILTER_VALIDATE_EMAIL ) ) && ( empty($email1) || ! filter_var($email1, FILTER_VALIDATE_EMAIL ) ) ) {

$errors[] = "Email Addresses are Invalid";
}
But I am sure Jason will have something to say? 

I was wrong it only half works....

[GrumpyYoungMan]

Works:

Code: [Select]

// Are email addresses the same and a valid format?
if ( ( $email != $email1 ) || ( ( empty($email) || ! filter_var($email, FILTER_VALIDATE_EMAIL ) ) || ( empty($email1) || ! filter_var($email1, FILTER_VALIDATE_EMAIL ) ) ) ) {

$errors[] = "Email Addresses are Invalid";
}

[Jason Knight]

Some advice:

1) You are checking if the second field is empty AFTER you try to do an operation on it. Always check your empty BEFORE proceding forward.

2) Whitespace is your friend, it could greatly clarify your code.

3) there's more to checking e-mails than filter_var... which fails to check specific lengths to domain validity.

4) you don't need to validate both if they have to be EQUAL!

First off let's get you a more robust mail check function:

Code: [Select]

function isValidEmail($address) {
if (filter_var($address, FILTER_VALIDATE_EMAIL) === FALSE) return false;
[$local, $domain] = explode('@', $address);
$localLength = strlen($local);
$domainLength = strlen($domain);
return (
($localLength > 0 && $localLength < 65) &&
($domainLength > 3 && $domainLength < 256) &&
(checkdnsrr($domain, 'MX') || checkdnsrr($domain, 'A'))
);
} // isValidEmail

Beware the rdns lookups can add some delay if you're server is misconfigured or just sucks at it, but it's usually worth it to reject made up garbage domains.

Then your error logic can be simplified down to:

Code: [Select]

if (
empty($email) ||
empty($email1) ||
($email != $email1) ||
!isValidEmail($email)
) $errors[] = "Email Addresses are Invalid";

Though I suspect you've got "variables for nothing" in there, since there's nothing there that couldn't (or shouldn't) be done directly on $_POST.

Code: [Select]

if (
empty($_POST['email']) ||
empty($_POST['email_duplicate') ||
($_POST['email'] != $_POST['email_duplicate']) ||
!isValidEmail($_POST['email'])
) $errors[] = "Email Addresses are Invalid";

This isn't JavaScript where array or object lookups are slow as molasses in February, you don't need to be making extra variables for everything; as that usually ends up being variables for nothing.

Note I put the robust check function last in the testing so that the simple/smaller/faster checks are done first -- that way short circuit eval can exit prematurely when appropriate.

[John_Betong]

This PHP function may be of interest because it allows indexing on the email addresses which should reduce time spent in finding matches:


https://www.php.net/manual/en/function.strrev.php

[Jason Knight]

This PHP function may be of interest because it allows indexing on the email addresses which should reduce time spent in finding matches:

Did you link to the function you actually meant to, because changing "this is a test" to "tset a si siht" has absolutely nothing to do with e-mail indexing or matching...

Whatever the blazes you mean by "indexing on the email addresses" which too seems to be total gibberish.

[John_Betong]

I did mention that "this may be of interest"


I was thinking of what to do after the email was verified and thought it may be saved to a database table... if so then use the strrev(,,,) function to make searching easier


That is the only reason I can find for using strrev(...).


Just my two Satang

[GrumpyYoungMan]

Some advice:

1) You are checking if the second field is empty AFTER you try to do an operation on it. Always check your empty BEFORE proceding forward.

2) Whitespace is your friend, it could greatly clarify your code.

3) there's more to checking e-mails than filter_var... which fails to check specific lengths to domain validity.

4) you don't need to validate both if they have to be EQUAL!

First off let's get you a more robust mail check function:

Code: [Select]

function isValidEmail($address) {
if (filter_var($address, FILTER_VALIDATE_EMAIL) === FALSE) return false;
[$local, $domain] = explode('@', $address);
$localLength = strlen($local);
$domainLength = strlen($domain);
return (
($localLength > 0 && $localLength < 65) &&
($domainLength > 3 && $domainLength < 256) &&
(checkdnsrr($domain, 'MX') || checkdnsrr($domain, 'A'))
);
} // isValidEmail

Beware the rdns lookups can add some delay if you're server is misconfigured or just sucks at it, but it's usually worth it to reject made up garbage domains.

Then your error logic can be simplified down to:

Code: [Select]

if (
empty($email) ||
empty($email1) ||
($email != $email1) ||
!isValidEmail($email)
) $errors[] = "Email Addresses are Invalid";

Though I suspect you've got "variables for nothing" in there, since there's nothing there that couldn't (or shouldn't) be done directly on $_POST.

Code: [Select]

if (
empty($_POST['email']) ||
empty($_POST['email_duplicate') ||
($_POST['email'] != $_POST['email_duplicate']) ||
!isValidEmail($_POST['email'])
) $errors[] = "Email Addresses are Invalid";

This isn't JavaScript where array or object lookups are slow as molasses in February, you don't need to be making extra variables for everything; as that usually ends up being variables for nothing.

Note I put the robust check function last in the testing so that the simple/smaller/faster checks are done first -- that way short circuit eval can exit prematurely when appropriate.

I think the forum software is doing something to the code as I copy and paste it, as I am indenting the formatting the code in my editor.

I have very quickly change the code to this:

Code: [Select]

// NEW Logic Email Validation:
// Are the email addresses the same and a valid format?
if( ! empty( $email ) ) {

if( ! ( $email == $email1 ) && ( filter_var($email, FILTER_VALIDATE_EMAIL ) ) ) {

$errors[] = "Email Addresses do not match";

}
else {

( isValidEmail($email) != 1 ) ? $errors[] = "Invalid Email Address" : "";

}

}
else {

$errors[] = "Email Address Fields empty or invalid...";

}
You're right about the "variables for nothing" but as I will be validating and formatting the inputs I thought it may be easier to assign the $_POST to variables rather than changing the actual $_POST data - is this wrong?

Thank you as always for taking the time to help and correct my mistakes... I really do appeciate it!

[Jason Knight]

You're right about the "variables for nothing" but as I will be validating and formatting the inputs I thought it may be easier to assign the $_POST to variables rather than changing the actual $_POST data - is this wrong?

If you're validating them, why change them? If it's invalid you don't clean it up, you spit the form back at the users.

And what formatting would you possibly be doing to a password or e-mail address?!?

As to your new code, simplify your evaluation order so all your checks are uniform.

Code: [Select]

if (empty($email) || empty($email1))
$errors[] = "An e-mail address field is empty";
else if ($email !== $email1)
$errors[] = "E-mail addresses do not match";
else if (!isValidEmail($email))
$errors[] = "Invalid e-mail address";

Check BOTH empty first. Always leave the most complex check for last. Also why did you add the unneccessary filter_var back into your code? And what's with the !=1 crap? It's returning boolean.  Much less why generate an empty error, doesn't that screw up simply checking "if ($errors)" since a loose compare of an empty array is false?

And people wonder why I say strict types are a task complexity mismatch for PHP...

[GrumpyYoungMan]

You're right about the "variables for nothing" but as I will be validating and formatting the inputs I thought it may be easier to assign the $_POST to variables rather than changing the actual $_POST data - is this wrong?

If you're validating them, why change them? If it's invalid you don't clean it up, you spit the form back at the users.

And what formatting would you possibly be doing to a password or e-mail address?!?

As to your new code, simplify your evaluation order so all your checks are uniform.

Code: [Select]

if (empty($email) || empty($email1))
$errors[] = "An e-mail address field is empty";
else if ($email !== $email1)
$errors[] = "E-mail addresses do not match";
else if (!isValidEmail($email))
$errors[] = "Invalid e-mail address";

Check BOTH empty first. Always leave the most complex check for last. Also why did you add the unneccessary filter_var back into your code? And what's with the !=1 crap? It's returning boolean.  Much less why generate an empty error, doesn't that screw up simply checking "if ($errors)" since a loose compare of an empty array is false?

And people wonder why I say strict types are a task complexity mismatch for PHP...

I missed the filter_var in your email function! - Sorry!!

The only cleaning up I was going to, as this is for a contact form was on the message body to insert new lines etc and remove all the special characters... you got a better suggestion?

But I guess I can just do that on function and send that straight to the query (via Prepare )?

I was also going to remove any whitespaces and check for illegal characters on the name and email fields?

Thanks again!

I have visions of you sat at your desk with your head in your hands every time you see I have posted, thinking what now...