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

undefined index in ThreadReply.php (and undefined variable recaptcha) #30

Open
smxi opened this issue Jan 15, 2022 · 1 comment
Open

Comments

@smxi
Copy link

smxi commented Jan 15, 2022

Thanks for doing Quip.

While testing with strict full php error reporting, I found this issue in quip/controllers/web/ThreadReply.php

Notice: Undefined index: name in /modx/components/quip/controllers/web/ThreadReply.php on line 188
Notice: Undefined index: email in /modx/components/quip/controllers/web/ThreadReply.php on line 189
Notice: Undefined index: website in /modx/components/quip/controllers/web/ThreadReply.php on line 190
        $fields['name']    = strip_tags($fields['name']);
        $fields['email']   = strip_tags($fields['email']);
        $fields['website'] = strip_tags($fields['website']);

should be:

if (isset($fields['name'])){        
       	$fields['name']    = strip_tags($fields['name']);
}
if (isset($fields['email'])){  
        $fields['email']   = strip_tags($fields['email']);
}
if (isset($fields['website'])){  
        $fields['website'] = strip_tags($fields['website']);
}

Obviously no need to strip tags off an unset index value.

And also one more in:
Notice: Undefined variable: hook in /modx/cache/includes/elements/modsnippet/50.include.cache.php on line 48

Snippet 50 is: Renders ReCaptcha V2 form: recaptchav2_render

Again, a test without checking if is set:

if ($hook) { 
    $hook->setValue('recaptchav2_html', $recaptcha_html); // This won't re-render on page reload there's validation errors
    return true;
} else { // This works at least
    return $recaptcha_html;
}

Should be:

if (isset($hook)) { 
    $hook->setValue('recaptchav2_html', $recaptcha_html); // This won't re-render on page reload there's validation errors
    return true;
} else { // This works at least
    return $recaptcha_html;
}

I wasn't getting these error/warnings on my local system even with all error reporting turned on, but I was getting it on a production server, not sure why the difference.

@smxi
Copy link
Author

smxi commented Feb 12, 2022

I should add, there are more of these undefined index popping up, all based on trying to test undefined indexes, I may work iup a more complete list and patch if work will pay me for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant