CAPTCHA未正确验证

Question

I'm working on a form, and everything seems to be working fine except for the custom captcha. Whether I enter a wrong or the actually correct answer, the form does not submit to the database, and reloads the form with my "Please try again" error message... I'm wanting the validation for this field to follow the same format as the others, but don't know if that is possible (it's the $captchaValid in the validation section). Here is the php code that I have in my form:

<?php   
//captcha
session_start();
$digit1 = mt_rand(1, 10);
$digit2 = mt_rand(1, 10);

$math ="$digit1 + $digit2";
$_SESSION['answer'] = $digit1 + $digit2;

//error reporting
error_reporting(E_ALL);
ini_set('display_errors','1');

//include validation file here!!
include_once('validationresult.php');

//results
$firstnameValid = new ValidationResult("", "", "", true);
$lastnameValid  = new ValidationResult("", "", "", true);
$emailValid = new ValidationResult("", "", "", true);
$captchaValid = new ValidationResult("", "", "", true);

//validation
if ($_SERVER["REQUEST_METHOD"] == "POST") {
    $firstnameValid = ValidationResult::checkParameter("firstname", '/^[a-zA-Z ]*$/', 'Only Letters and Spaces Allowed');
    $lastnameValid = ValidationResult::checkParameter("lastname", '/^[a-zA-Z ]*$/', 'Only Letters and Spaces Allowed');
    $emailValid = ValidationResult::checkParameter("email", '/(.+)@([^\.].*)\.([a-z]{2,})/', 'Please enter valid e-mail');
    $captchaValid = ValidationResult::checkParameter("captcha", $_SESSION['answer'] == $_POST['answer'], 'Please try again');
//redirection

if ($firstnameValid->isValid() && $lastnameValid->isValid() && $emailValid->isValid() && $captchaValid->isValid() ) {
    require "connectiontest.php";
    header('Location: thankyou.html');
    exit;
    }
}


?>

And following is my separate validation file:

<?php 

class ValidationResult
{
    private $value;
    private $cssClassName;
    private $errorMessage;
    private $isValid = true;


    function __construct($cssClassName, $value, $errorMessage, $isValid)
    {
        $this->cssClassName = $cssClassName;
        $this->value = $value;
        $this->errorMessage = $errorMessage;
        $this->isValid = $isValid;
    }

    public function getCssClassName() { return $this->cssClassName;}
    public function getValue() { return $this->value;}
    public function getErrorMessage() { return $this->errorMessage;}
    public function isValid() { return $this->isValid;}

    static public function checkParameter($queryName, $pattern, $errMsg) {
        $error = "";
        $errClass = "";
        $value = "";
        $isValid = "true";

        if (empty($_POST[$queryName])) {
            $error = $errMsg;
            $errClass = "error";
            $isValid = false;
        }
        else {
            $value = $_POST[$queryName];
            if ( ! preg_match($pattern, $value) ) {
                $error = $errMsg;
                $errClass = "error";
                $isValid = false;
            }
        }
        return new ValidationResult($errClass, $value, $error, $isValid);
    }
}

 ?>

I've tried changning this part:

$captchaValid = ValidationResult::checkParameter("captcha", $_SESSION['answer'] == $_POST['answer'], 'Please try again');

a few different ways to see if I could possibly make it work, but as I said, I don't even know if this is possible (since it's not really a regular expression?). I've tried removing the "==$_POST..." part, enclosing it in single and double quotes, etc, and either get an error or the same result as before. If want I want isn't possible, I appreciate suggestions on alternative solutions...

A few notes: I am very new to php and don't really know much of anything, so please keep that in mind. Also, I don't know javascript yet (not that it matters in this case). HTML, CSS, and PHP is about the breadth of my knowledge. This is for a school assignment.

EDIT Here is the full code, with the html with the form included. There have been a few edits based on the previous suggestion:

<?php   
//captcha
if (!isset($_SESSION['answer'])) {
    $digit1 = mt_rand(1, 10);
    $digit2 = mt_rand(1, 10);

    $math ="$digit1 + $digit2";
    $_SESSION['answer'] = $digit1 + $digit2;
}


//error reporting
error_reporting(E_ALL);
ini_set('display_errors','1');

//include validation file here!!
include_once('validationresult.php');

//results
$firstnameValid = new ValidationResult("", "", "", true);
$lastnameValid  = new ValidationResult("", "", "", true);
$emailValid = new ValidationResult("", "", "", true);
$captchaValid = new ValidationResult("", "", "", true);

//validation
if ($_SERVER["REQUEST_METHOD"] == "POST") {
    $firstnameValid = ValidationResult::checkParameter("firstname", '/^[a-zA-Z ]*$/', 'Only Letters and Spaces Allowed');
    $lastnameValid = ValidationResult::checkParameter("lastname", '/^[a-zA-Z ]*$/', 'Only Letters and Spaces Allowed');
    $emailValid = ValidationResult::checkParameter("email", '/(.+)@([^\.].*)\.([a-z]{2,})/', 'Please enter valid e-mail');
    $captchaValid = ValidationResult::checkParameter("captcha", "/" .  $_POST['answer'] . "/" , 'Please try again');

    //redirection

    if ($firstnameValid->isValid() && $lastnameValid->isValid() && $emailValid->isValid() && $captchaValid->isValid() ) {
        require "connectiontest.php";
        header('Location: thankyou.html');
        exit;
    }
}


?>

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Registration</title>
</head>

<body>

    <form method="post" action="<?php echo $_SERVER["PHP_SELF"];?>" >

        <?php echo $firstnameValid->getCssClassName(); ?>
        <label for="firstname">First name:</label> <input type="text" name="firstname"  value="<?php echo $firstnameValid->getValue(); ?>" required> <br>
        <span class="help-inline" id="errorFirstname"> <?php echo $firstnameValid->getErrorMessage(); ?> </span> <br>

        <?php echo $lastnameValid->getCssClassName(); ?>
        <label for="lastname">Last name:</label> <input type="text" name="lastname" value="<?php echo $lastnameValid->getValue(); ?>" required> <br>
        <span class="help-inline" id="errorLastname"> <?php echo $lastnameValid->getErrorMessage(); ?> </span> <br>


        <?php echo $emailValid->getCssClassName(); ?>
        <label for="email">E-mail:</label> <input type="email" name="email" value="<?php echo $emailValid->getValue(); ?>" required> <br>
        <span class="help-inline" id="errorEmail"> <?php echo $emailValid->getErrorMessage(); ?> </span> <br>

        <label for="optin">Opt In??</label><!--add legal-ese later--> <input type="checkbox" name="optin" required> <br>

        <?php echo $captchaValid->getCssClassName(); ?>
        <label for="captcha">captcha</label> <?php echo $math; ?> = <input type="text" name="captcha" value="<?php echo $captchaValid->getValue(); ?>" required><br>
        <span class="help-inline" id="errorCaptcha"> <?php echo $captchaValid->getErrorMessage(); ?> </span> <br>

        <input type="submit" value="Submit">

    </form>

</body>
</html>

Answer 1

I almost forgot about this question, so I know I am totally late, but anyway: few more things.

1. input with name answer does not exist. So the value $_POST['answer'] will always be null - and then the pattern "/" . $_POST['answer'] . "/" will always be // which matches everything. I didn't notice that last time. You probably want to validate against $_SESSION['answer'], not $_POST['answer'].

2. session_start() disappeared - so the answer is once again generated everytime.

3. If the user did not answered correctly, you want to regenerate the captcha - so in the block at line 38 if ($firstnameValid->isValid() && ... you want to add else block. And generate new captcha answer and store it to the session, in that new else

=============== second guess ===============

I do not see the page (file), where you display the HTML form. So I guess it's in another included file.

First guess is at the bottom, but there is more important issue, at the line

 $captchaValid = ValidationResult::checkParameter("captcha", $_SESSION['answer'] == $_POST['answer'], 'Please try again');

the part $_SESSION['answer'] == $_POST['answer'] gets first evaluated, and the result is passed as pattern. So you are passing either true or false. Also. I am not sure, how you get values of $_POST['captcha'] and $_POST['answer']. Do you automaticly fill hidden input 'captcha' with the correct answer?

You pass captcha and false to ValidationResult->checkParameter(). If you want to check whether $_POST['captcha'] is the same as $_POST['answer'], I would pass as a pattern (second parameter) something like

"/" . $_POST['answer'] . "/". 
// for example: /12/

So that it creates pattern. You will be validating $_POST['captcha'] against that pattern.

=============== original guess ===============

I also guess, that when you submit the form, you reload the entire page. So that the following block loads again:

<?php   
//captcha
session_start();
$digit1 = mt_rand(1, 10);
$digit2 = mt_rand(1, 10);

$math ="$digit1 + $digit2";
$_SESSION['answer'] = $digit1 + $digit2;

If that is true, you are validating against wrong number. You first generate new answer, and then check the old answer against it.

The check, whether the answer is already set, would be:

if(!isset($_SESSION['answer'])) //If the answer doesn't already exists
{
    // generate new answer
    $digit1 = mt_rand(1, 10);
    $digit2 = mt_rand(1, 10);

    $math ="$digit1 + $digit2";
    $_SESSION['answer'] = $digit1 + $digit2;
}

I am new to Stackoverflow, so I'am not completely sure, whether this should be an answer or a comment, but anyway I cannot post comments yet because of my reputation.

Hope it helps :-)