3
\$\begingroup\$

I have been asked to make a registration form that will add a users details to a database, create a registration code, then email the registrant and client the details including the registration code.

I am quite new to PHP but have put together something that works for me, however, I don't have the experience to know if it will work for everyone. Also, it uses the PHPMailer to send the email.

Any advice regarding security, as well as anything that would be considered bad practice would be appreciated.

HTML:

<div class="col-sm-12">
  <form id="SubmissionForm" action="insert.php" method="POST" role="form">
    <div class="form-group">
      <label for="FirstName">First Name *</label>
      <input type="text" class="form-control" id="FirstName" placeholder="" name="first_name" value="" data-error="Please enter a valid first name." required maxlength="255">
      <div class="help-block with-errors"></div>
    </div>

    <div class="form-group">
      <label for="LastName">Last Name *</label>
      <input type="text" class="form-control" id="LastName" placeholder="" name="last_name" value=""  data-error="Please enter a valid last name." required maxlength="255">
      <div class="help-block with-errors"></div>              
    </div>

    <div class="form-group">
      <label for="Email">Email address *</label>
      <input type="email" class="form-control" id="Email" placeholder="" name="email" value="" data-error="Please enter a valid email address." required maxlength="255">
      <div class="help-block with-errors"></div>              
    </div>

    <div class="form-group">
      <label for="Age">Age *</label>
      <input type="text" class="form-control" id="Age" placeholder="" name="age" value="" data-error="Please enter a valid age." required maxlength="255">
      <div class="help-block with-errors"></div>
    </div>

    <div class="form-group">
      <label for="Phone">Phone *</label>
      <input type="text" class="form-control" id="Phone" placeholder="" name="phone" value="" data-error="Please enter a valid phone number." required maxlength="255">
      <div class="help-block with-errors"></div>
    </div>

    <div class="form-group">
      <label for="StreetAddress">Street Address *</label>
      <input type="text" class="form-control" id="StreetAddress" placeholder="" name="street_address" value="" data-error="Please enter a valid street address." required maxlength="255">
      <div class="help-block with-errors"></div>
    </div>

    <div class="form-group">
      <label for="Suburb">Suburb</label>
      <input type="text" class="form-control" id="Suburb" placeholder="" name="suburb" value="" maxlength="255">
      <div class="help-block with-errors"></div>
    </div>

    <div class="form-group">
      <label for="City">City *</label>
      <input type="text" class="form-control" id="City" placeholder="" name="city" value="" data-error="Please enter a valid city." required maxlength="255">
      <div class="help-block with-errors"></div>
    </div>

    <div class="form-group">
      <label for="Postcode">Postcode *</label>
      <input type="text" class="form-control" id="Postcode" placeholder="" name="postcode" value="" data-error="Please enter a valid postcode." required maxlength="255">
      <div class="help-block with-errors"></div>
    </div>

    <div class="form-group">
      <label for="Country">Country *</label>
      <input type="text" class="form-control" id="Country" placeholder="" name="country" value="" data-error="Please enter a valid country." required maxlength="255">
      <div class="help-block with-errors"></div>
    </div>

    <div class="form-group">
      <label for="FindUs">Where did you hear about us?</label>
      <input type="text" class="form-control" id="FindUs" placeholder="" name="findus" value="" maxlength="255">
    </div>

    <div class="form-group">
      <div class="gender-label">Gender *</div>
      <div id="Gender">
        <div class="radio" style="margin-top:0px;">
          <label>
            <input type="radio" name="gender" id="Female" value="F" required>
            Female
          </label>
        </div>
        <div class="radio">
          <label>
            <input type="radio" name="gender" id="Male" value="M" required>
            Male
          </label>
        </div>
      </div>
      <div class="help-block with-errors"></div>
    </div>

    <hr>

    <div class="form-group">
      <button type="submit" class="btn btn-primary">SUBMIT</button>
    </div>
  </form>
</div>

PHP:

<?php
//include 'connect.php';
$servername = "servername";
$username = "username";
$password = "password";
$dbname = "dbname";

$conn = new mysqli($servername, $username, $password, $dbname);

if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}

if($_SERVER['REQUEST_METHOD'] == 'POST'){
    $host_email = 'email@email.com';
    $host_name = 'name';

    $first_name = preg_replace("/[^a-zA-Z0-9\s]/", "", $_POST["first_name"]);
    $last_name = preg_replace("/[^a-zA-Z0-9\s]/", "", $_POST["last_name"]);
    $email = filter_var($_POST["email"], FILTER_SANITIZE_EMAIL);
    $gender = preg_replace("/[^a-zA-Z0-9\s]/", "", $_POST["gender"]);
    $age = preg_replace("/[^a-zA-Z0-9\s]/", "", $_POST["age"]);
    $phone = preg_replace("/[^a-zA-Z0-9\s]/", "", $_POST["phone"]);
    $findus = preg_replace("/[^a-zA-Z0-9\s]/", "", $_POST["findus"]);
    $street_address = preg_replace("/[^a-zA-Z0-9\s]/", "", $_POST["street_address"]);

    $suburb = '';
    if(isset($_POST["suburb"]) == ''){
        $suburb = 'n/n';
    }else{
        $suburb = preg_replace("/[^a-zA-Z0-9\s]/", "", $_POST["suburb"]);       
    }

    $postcode = preg_replace("/[^a-zA-Z0-9\s]/", "", $_POST["postcode"]);       
    $city = preg_replace("/[^a-zA-Z0-9\s]/", "", $_POST["city"]);           
    $country = preg_replace("/[^a-zA-Z0-9\s]/", "", $_POST["country"]); 

    $agree = '';
    if(isset($_POST["agree"]) == ''){
        $agree = 'No';
    }else{
        $agree = 'Yes';
    }

    $sql = "INSERT INTO `dbname`.`tablename` (`first_name`, `last_name`, `email`, `gender`, `age`, `phone`, `findus`, `street_address`, `suburb`, `city`, `country`, `postcode`, `email_subscribe`, `date`) 
    VALUES ('$first_name', '$last_name', '$email', '$gender', '$age', '$phone', '$findus', '$street_address', '$suburb', '$city', '$country', '$postcode', '$agree', NOW());";

    if($conn->query($sql) === TRUE) {
        $unique_id = $conn->insert_id;  //get the id from the last db insert

        $name = $first_name . ' ' . $last_name;

        $phone = $phone;

        require 'PHPMailerAutoload.php';

        $mail = new PHPMailer;

        //$mail->SMTPDebug = 3;                                         // Enable verbose debug output

        $mail->isSMTP();                                                // Set mailer to use SMTP
        $mail->Host = 'host';                                           // Specify main and backup SMTP servers
        $mail->SMTPAuth = true;                                         // Enable SMTP authentication
        $mail->Username = 'username';                                   // SMTP username
        $mail->Password = 'password';                                   // SMTP password
        $mail->SMTPSecure = 'tls';                                      // Enable TLS encryption, `ssl` also accepted
        $mail->Port = 587;                                              // TCP port to connect to

        $mail->setFrom('email@email.com', 'email');
        $mail->addAddress($host_email, $host_name);                     // Add a recipient - name is optional       
        $mail->addAddress($email, $first_name.' '.$last_name);          // Add a recipient - name is optional
        $mail->addReplyTo('email@email.com', 'email');

        $mail->addAttachment('map.pdf');                                // Add attachments
        $mail->isHTML(true);                                            // Set email format to HTML

        $mail->Subject = 'Registration ID: #'.$unique_id;

        $message = '<p><span>Name:</span><span><strong>'.$name.'</strong></span></p>
        <p><span>Email:</span><span><strong>'.$email.'</strong></span></p>
        <p><span>Age:</span><span><strong>'.$age.'</strong></span></p>
        <p><span>Gender:</span><span><strong>'.$gender.'</strong></span></p>
        <p><span>Phone Number:</span><span><strong>'.$phone.'</strong></span></p>
        <p><span>Street Address:</span><span><strong>'.$street_address.'</strong></span></p>
        <p><span>Suburb:</span><span><strong>'.$suburb.'</strong></span></p>
        <p><span>City:</span><span><strong>'.$city.'</strong></span></p>
        <p><span>Country:</span><span><strong>'.$country.'</strong></span></p>
        <p><span>How did you find out about us?:</span><span><strong>'.$findus.'</strong></span></p>
        <p><span>Agree to email?:</span><span><strong>'.$agree.'</strong></span></p>
        <hr>';

        $email_copy = "<p>Thank you for registering.</p><p><strong>Your Registration Number is #".$unique_id."</strong></p>";

        $mail->Body = $message.$email_copy;

        $mail->AltBody = $message.'<br><p><span>Name:</span><span><strong>'.$name.'</strong></span></p><p><span>Age:</span><span><strong>'.$age.'</strong></span></p><p><span>Email:</span><span><strong>'.$email.'</strong></span></p><p><span>Gender:</span><span><strong>'.$gender.'</strong></span></p>';

        if(!$mail->send()) {
            header("Location:error.php");   //if mail fails
        }
        else{
            header("Location:thankyou.php");
        }
    }else{
        echo("Error description: " . mysqli_error($conn)); //if db insert fails
    }
}
$conn->close();
?>
\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review! Good job on your first question. \$\endgroup\$ – SirPython Feb 4 '16 at 21:27
  • \$\begingroup\$ My preference would be to use loops, both to create the form, and to sanitize the data. However, that's a fairly minor suggestion. \$\endgroup\$ – user1149 Feb 4 '16 at 22:10
4
\$\begingroup\$
  1. Put the database connection code in a separate file as you have commented in your code:

    //include 'connect.php';
    

    This will be a better way.

  2. Writing if conditions and comparing the hardcoded values always put the values at left-hand side. So your if condition will be changed to:

    if('POST' == $_SERVER['REQUEST_METHOD'])
    

    This will be a better way to write and it will avoid by assignments by mistake while writing big code.

  3. preg_replace("/[^a-zA-Z0-9\s]/", "", $_POST["first_name"])

    Put this statement in a different function as you are using frequently in your code, so this will be changed to:

    function parseVariabel($paramVariable) {
        return preg_replace("/[^a-zA-Z0-9\s]/", "", $paramVariable);
    }
    

    Now put this function in a separate file, such as functions.php, and include it in your code. In the future, you can add more functions to this file that will be used frequently in your code.

    Keep one thing in mind while writing code: if you are using any code more than once or twice, convert that code to a reusable function.

  4. Put your mail code in a separate file and include that file whenever you need it.

  5. Create a separate function for sending mail and pass your values as arguments to that function.

  6. One more function you can add to the functions.php file:

    function redirect(url) {
        header("Location:"+ url);
    }
    

    Always try to separate the code which is frequently used in your application so it will be easily manageable.

I appreciate your efforts that you have taken in your first program. You have made a good job at initial level. As you will do more practice you will get more understanding about how to write more reusable code.

\$\endgroup\$
0
1
\$\begingroup\$

HTML

You could use type="tel" for the "Phone" field.

You could use type="number" for the "Age" field.

Accessibility / Usability

You should explain the meaning of the *. For example:

* marks required fields

And you could use abbr for the * in the label elements, e.g.:

<label for="FirstName">First Name <abbr title="required field">*</abbr></label>

However, because 10 of 12 fields seem to be required, it can make more sense to mark the optional fields instead. And because there are only two fields, it might be more clear to not use * at all, and spell it out instead. For example by adding "(optional)" to the label:

<label for="Suburb">Suburb (optional)</label>

<label for="FindUs">Where did you hear about us? (optional)</label>

You might want to move the "Age" field next to "Gender", so that "Email address" and "Phone" are next to each other.

If you want, you could add fieldset+legend elements to group fields that belong together:

  • Something like Contact details for "Email address" and "Phone"
  • Something like Address for "Street Address", "Suburb", "City", "Postcode", and "Country"
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.