Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.9.7 won't validate email addresses in Register #143

Closed
pyrographics opened this issue Jan 18, 2019 · 8 comments
Closed

1.9.7 won't validate email addresses in Register #143

pyrographics opened this issue Jan 18, 2019 · 8 comments
Labels

Comments

@pyrographics
Copy link

When we upgraded to 1.9.7 the site would no longer validate email addresses in the Register snippet but when I downgraded to 1.9.5 the emails would validate again. We are running PHP 7.0 on Cpanel with MODX 2.6.5. I tested our same form on a fresh install on MODX cloud which worked fine but the same form wouldn't work on this site. I uninstalled and deleted all versions and packages of Login and re-downloaded and reinstalled 1.9.7 but it still wouldn't function. Not sure what changed but something isn't working with the new version.

@mrhaw
Copy link

mrhaw commented Jun 15, 2020

I have the same issue! Glad to see you have it to. Will report back if I find a fix!

@mrhaw
Copy link

mrhaw commented Jun 15, 2020

I suspect it has to do with: "Escape MODX tags after htmlspecialchars because of the ampersand sign"

Login 1.9.6

@Jako Jako added the bug label Jun 15, 2020
@mrhaw
Copy link

mrhaw commented Jun 15, 2020

The validation itself is working!!
user-reg3

@pyrographics
Copy link
Author

I again had this problem on a site. I have narrowed it down to loginvalidator.class.php lines 395 through 444. Comparing that to the 1.9.5, Line 412 has a different pattern variable (one slash removed it appears) and the condition variables added preg_quote when the pattern is called to add slashes. Evidently this change is what is breaking the register snippet. I am not very good with regular expressions so someone who is will need to figure it out. For the time being I just rolled back this particular function to the 1.9.5 version.

@pyrographics
Copy link
Author

Broken function: `public function email($key,$value) {
/* allow empty emails, :required should be used to prevent blank field */
if (empty($value)) return true;

    /* validate length and @ */
    $pattern = "^[^@]{1,64}\@[^\@]{1,255}$";
    $condition = $this->config['use_multibyte'] ? @mb_ereg($pattern, $value) : @preg_match('#' . preg_quote($pattern, '#') . '#', $value);
    if (!$condition) {
        return $this->_getErrorMessage($key,'vTextEmailInvalid','register.email_invalid',array(
            'field' => $key,
            'value' => $value,
        ));
    }

    $email_array = explode("@", $value);
    $local_array = explode(".", $email_array[0]);
    for ($i = 0; $i < sizeof($local_array); $i++) {
        $pattern = "^(([A-Za-z0-9!#$%&'*+/=?^_`{|}~-][A-Za-z0-9!#$%&'*+/=?^_`{|}~\.-]{0,63})|(\"[^(\\|\")]{0,62}\"))$";
        $condition = $this->config['use_multibyte'] ? @mb_ereg($pattern, $local_array[$i]) : @preg_match('#' . preg_quote($pattern, '#') . '#', $local_array[$i]);
        if (!$condition) {
            return $this->_getErrorMessage($key,'vTextEmailInvalid','register.email_invalid',array(
                'field' => $key,
                'value' => $value,
            ));
        }
    }
    /* validate domain */
    $pattern = "^\[?[0-9\.]+\]?$";
    $condition = $this->config['use_multibyte'] ? @mb_ereg($pattern, $email_array[1]) : @preg_match('#' . preg_quote($pattern, '#') . '#', $email_array[1]);
    if (!$condition) {
        $domain_array = explode(".", $email_array[1]);
        if (sizeof($domain_array) < 2) {
            return $this->_getErrorMessage($key,'vTextEmailInvalidDomain','register.email_invalid_domain',array(
                'field' => $key,
                'value' => $value,
            ));
        }
        for ($i = 0; $i < sizeof($domain_array); $i++) {
            $pattern = "^(([A-Za-z0-9][A-Za-z0-9-]{0,61}[A-Za-z0-9])|([A-Za-z0-9]+))$";
            $condition = $this->config['use_multibyte'] ? @mb_ereg($pattern, $domain_array[$i]) : @preg_match('#' . preg_quote($pattern, '#') . '#', $domain_array[$i]);
            if (!$condition) {
                return $this->_getErrorMessage($key,'vTextEmailInvalidDomain','register.email_invalid_domain',array(
                    'field' => $key,
                    'value' => $value,
                ));
            }
        }
    }
    return true;
}`

Working function:
`public function email($key,$value) {
/* allow empty emails, :required should be used to prevent blank field */
if (empty($value)) return true;

    /* validate length and @ */
    $pattern = "^[^@]{1,64}\@[^\@]{1,255}$";
    $condition = $this->config['use_multibyte'] ? @mb_ereg($pattern, $value) : @preg_match('/' . $pattern . '/', $value);
    if (!$condition) {
        return $this->_getErrorMessage($key,'vTextEmailInvalid','register.email_invalid',array(
            'field' => $key,
            'value' => $value,
        ));
    }

    $email_array = explode("@", $value);
    $local_array = explode(".", $email_array[0]);
    for ($i = 0; $i < sizeof($local_array); $i++) {
        $pattern = "^(([A-Za-z0-9!#$%&'*+\/=?^_`{|}~-][A-Za-z0-9!#$%&'*+\/=?^_`{|}~\.-]{0,63})|(\"[^(\\|\")]{0,62}\"))$";
        $condition = $this->config['use_multibyte'] ? @mb_ereg($pattern, $local_array[$i]) : @preg_match('/' . $pattern . '/', $local_array[$i]);
        if (!$condition) {
            return $this->_getErrorMessage($key,'vTextEmailInvalid','register.email_invalid',array(
                'field' => $key,
                'value' => $value,
            ));
        }
    }
    /* validate domain */
    $pattern = "^\[?[0-9\.]+\]?$";
    $condition = $this->config['use_multibyte'] ? @mb_ereg($pattern, $email_array[1]) : @preg_match('/' . $pattern . '/', $email_array[1]);
    if (!$condition) {
        $domain_array = explode(".", $email_array[1]);
        if (sizeof($domain_array) < 2) {
            return $this->_getErrorMessage($key,'vTextEmailInvalidDomain','register.email_invalid_domain',array(
                'field' => $key,
                'value' => $value,
            ));
        }
        for ($i = 0; $i < sizeof($domain_array); $i++) {
            $pattern = "^(([A-Za-z0-9][A-Za-z0-9-]{0,61}[A-Za-z0-9])|([A-Za-z0-9]+))$";
            $condition = $this->config['use_multibyte'] ? @mb_ereg($pattern, $domain_array[$i]) : @preg_match('/' . $pattern . '/', $domain_array[$i]);
            if (!$condition) {
                return $this->_getErrorMessage($key,'vTextEmailInvalidDomain','register.email_invalid_domain',array(
                    'field' => $key,
                    'value' => $value,
                ));
            }
        }
    }
    return true;
}`

@sepiariver
Copy link

What would we lose by doing something like:

    if ((filter_var($value, FILTER_VALIDATE_EMAIL) === false) || (strlen($value) > 99)) {
        return $this->_getErrorMessage($key,'vTextEmailInvalid','register.email_invalid',array(
            'field' => $key,
            'value' => $value,
        ));
    }

The regex code was added 10 years ago. Unsure what value it adds at this point?

What do you think @Jako ?

@Jako
Copy link
Collaborator

Jako commented Jul 17, 2020

I am fine with filter_var.

@Jako
Copy link
Collaborator

Jako commented Sep 18, 2020

Fixed in 1.9.9.

@Jako Jako closed this as completed Sep 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants