Where is the Validation At?

As oft stated, the “right” way to validate emails is to do a bare minimum sanity check on format, and then send a verification message to the email address the user supplied; it’s the only way to ensure that what they gave you isn’t just syntactically valid, but is actually usable.

But even that simple approach leaves places to go wrong. Take a look at this code, from Lana.

public function getEmailValidationErrors($data): array
{
     $errors = [];
     if (isset($data["email"]) && !empty($data["email"])) {
         if (!str_contains($data["email"], "@")) {
             $error["email"] = "FORM.CONTACT_DETAILS.ERRORS.NO_AT";
         }
         if (!str_contains($data["email"], ".")) {
             $error["email"] = "FORM.CONTACT_DETAILS.ERRORS.NO_DOT";
         }
         if (strrpos($data["email"], "@") > strrpos($data["email"], ".")) {
             $error["email"] = "FORM.CONTACT_DETAILS.ERRORS.NO_TLD";
         }
     }
     if (isset($data["email1"]) && !empty($data["email1"])) {
        if (!str_contains($data["email1"], "@")) {
            $error["email1"] = "FORM.CONTACT_DETAILS.ERRORS.NO_AT";
        }
        if (!str_contains($data["email1"], ".")) {
            $error["email1"] = "FORM.CONTACT_DETAILS.ERRORS.NO_DOT";
        }
        if (strrpos($data["email1"], "@") > strrpos($data["email1"], ".")) {
            $error["email1"] = "FORM.CONTACT_DETAILS.ERRORS.NO_TLD";
        }
    }
    if (isset($data["email2"]) && !empty($data["email2"])) {
        if (!str_contains($data["email2"], "@")) {
            $error["email2"] = "FORM.CONTACT_DETAILS.ERRORS.NO_AT";
        }
        if (!str_contains($data["email2"], ".")) {
            $error["email2"] = "FORM.CONTACT_DETAILS.ERRORS.NO_DOT";
        }
        if (strrpos($data["email2"], "@") > strrpos($data["email2"], ".")) {
            $error["email2"] = "FORM.CONTACT_DETAILS.ERRORS.NO_TLD";
        }
    }
    if (isset($data["email3"]) && !empty($data["email3"])) {
        if (!str_contains($data["email3"], "@")) {
            $error["email3"] = "FORM.CONTACT_DETAILS.ERRORS.NO_AT";
        }
        if (!str_contains($data["email3"], ".")) {
            $error["email3"] = "FORM.CONTACT_DETAILS.ERRORS.NO_DOT";
        }
        if (strrpos($data["email3"], "@") > strrpos($data["email3"], ".")) {
            $error["email3"] = "FORM.CONTACT_DETAILS.ERRORS.NO_TLD";
        }
    }
     return $errors;
}

Let’s start with the obvious problem: repetition. This function doesn’t validate simply one email, but four, by copy/pasting the same logic multiple times. Lana didn’t supply the repeated blocks, just noted that they existed, so let’s not pick on the bad names: “email1”, etc.- that’s just my placeholder. I assume it’s different contact types for a customer, or similar.

Now, the other problems range from trivial to comical. First, the PHP function empty returns true if the variable has a zero/falsy value or is not set, which means it implies an isset, making the first branch redundant. That’s trivial.

The way the checks get logged into the $error array, they can overwrite each other, meaning if you forget the “@” and the “.”, it’ll only complain about the “.”, but if you forget the “.”, it’ll complain about not having a valid TLD (the “NO_DOT” error will never be output). That’s silly.

Finally, the $errors array is the return value, but the $error array is where we store our errors, meaning this function doesn’t return anything in the first place. And that means that it’s a email validation function which doesn’t do anything at all, which honestly- probably for the best.