-3

My php code for registration does not insert values to the database. I tried different ways but it is still not working. Here is the code:

Database connection:

<?php $link=mysqli_connect("localhost", "root", "");
  mysqli_select_db($link, "dataadventurers");
 ?>

My registration form PHP code:

<?php
include "connection.php"; ?>
            <?php
              if(isset($_POST['submit1'])){
                  $firstname = $_POST['first_name'];
                  $lastname = $_POST['last_name'];
                  $middle = $_POST['middle_initial'];
                  $idnum = $_POST['id_number'];
                  $email = $_POST['email_add'];
                  $pass = $_POST['password'];
                  $bday = $_POST['birthdate'];
                  $course = $_POST['course'];
                  $year = $_POST['year'];


                  mysqli_query($link, "insert into member_registration values('', '$firstname', '$lastname'
                  , '$middle', '$idnum', '$email', '$pass', '$bday', '$course', '$year')");


           ?>
GBouffard
  • 1,125
  • 4
  • 11
  • 24
  • 1
    Your code is vulnerable to SQL injection. You should use prepared statements. – Dharman Dec 01 '19 at 10:46
  • 2
    Please never make this live in it's current state. Look at using prepared statements as well as password hashing. You must NEVER put direct user input into a table with some sort of sanitising – Juakali92 Dec 01 '19 at 10:49
  • 2
    Aside from the above mentioned security problems, SQL injection is also a very common source of bugs. To put it simply... You're not controlling the SQL code that you execute. It could be anything. When it fails, *at the very least* you should examine what that SQL code was and if it's what you expect, *and* check `mysqli_error($link)` to get the error from the database. – David Dec 01 '19 at 10:57

1 Answers1

0

Welcome to StackOverflow.

First of all, your code is vulnerable to SQL Injection. This is a major flaw but thankfully, one that's easily fixed. It is important that you do not leave this open to SQL Injection, even if this is something just for you to use. It'll keep your data safe in the event that someone else manages to access it and also gets you in to good habits.

Secondly, your code isn't working because you haven't specified what columns you want to insert into.

Using your example as a basis, here's a working version.

DO NOT USE THIS, IT IS VULNERABLE CODE

<?php 
    $link=mysqli_connect("localhost", "root", "");
    mysqli_select_db($link, "dataadventurers");
 ?>


<?php
    include "connection.php"; 
?>
<?php
    if(isset($_POST['submit1'])){
        $firstname = $_POST['first_name'];
        $lastname = $_POST['last_name'];
        $middle = $_POST['middle_initial'];
        $idnum = $_POST['id_number'];
        $email = $_POST['email_add'];
        $pass = $_POST['password'];
        $bday = $_POST['birthdate'];
        $course = $_POST['course'];
        $year = $_POST['year'];

        //If someone passes 2019'); drop table member_registration; -- for example as the year parameter, MySQL interprets the query string as two separate queries. One to insert a record and the second to drop the table and will execute both

        mysqli_query($link, "insert into member_registration (firstname, lastname, middle, idnum, email, pass, bday, course, year) values( '$firstname', '$lastname', '$middle', '$idnum', '$email', '$pass', '$bday', '$course', '$year')");;

    }
?>

A MORE SECURE VARIANT

I have a couple of SQL convenience functions based on PDO I use on a regular basis.

They pick up their credentials from an ini file stored outside of the publicly accessible folder structure.

The GetData procedure returns the results in the form of an associative array UpdateData returns the amount of rows affected.

Ini file example

host=localhost
dbname=dataadventurers
username=user
password=pass

Convenience Functions

/*Put credential ini file path here*/
    $credentialFile = "..."; 

   function GetData($sql, $params = null, $paramtypes = null){
        //Get database connection details
        $credentialsArray = parse_ini_file($credentialFile);
        //Create PDO Instance 
        $db = new PDO('mysql:host='.$credentialsArray['host'].';dbname='.$credentialsArray['dbname'].';charset=utf8mb4', $credentialsArray['username'], $credentialsArray['password'], array(PDO::ATTR_EMULATE_PREPARES => false, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION));
        if(is_null($params)){ //If no parameters supplied, execute the query as is
            $stmt = $db->query($sql);
            $results = $stmt->fetchAll(PDO::FETCH_ASSOC);
        }
        else{
            if(count($params) <> count($paramtypes)){ //Check that the parameter count and type count are the same
                throw new InvalidArgumentException;
            }
            $stmt = $db->prepare($sql); //Prepare the statement
            for($i=0; $i<count($params); $i++){ //Bind the parameters
                $stmt->bindValue($i+1, $params[$i], $paramtypes[$i]);
            }
            $stmt->execute(); //Execute query
            $results = $stmt->fetchAll(PDO::FETCH_ASSOC); //Return the results as an associative array
        }
        return $results;        
    }

    function UpdateData($sql, $params){
        //Get database connection details
        $credentialsArray = parse_ini_file($credentialFile);
        //Create PDO Instance 
        $db = new PDO('mysql:host='.$credentialsArray['host'].';dbname='.$credentialsArray['dbname'].';charset=utf8mb4', $credentialsArray['username'], $credentialsArray['password'], array(PDO::ATTR_EMULATE_PREPARES => false, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION));
        try{
            $stmt = $db->prepare($sql); //Prepare the statement
            is_null($params){ //If there aren't any parameters to bind...
                $stmt->execute(); //...execute statement as is
            }
            else{
                $stmt->execute($params); //otherwise execute with the supplied parameters
            }
            $results = $stmt->rowCount(); //Return the rowcount
            return $results;
        }
        catch(PDOException $ex){ //Catch any PDO Exceptions
            return $ex->getMessage(); //Return the exception message
        }
    }

Usage

The usage is simple. When selecting data, pass a SQL string, an array containing any parameters and an array containing the parameter types. These arrays must be of the same length.

When updating/inserting/deleting data, pass a SQL string and an array containing the parameters. There is no parameter type requirement for UpdateData.

//GetData with no parameters
$results = GetData('select * from member_registration', [], []);

//GetData with one parameter of type String.
$results2 = GetData('select * from member_registration where firstname = ?', ['David'], [PDO::PARAM_STR]);

//Your insert example
$parameters = [
    $firstname, 
    $lastname, 
    $middle, 
    $idnum, 
    $email, 
    $pass, 
    $bday, 
    $course, 
    $year
];

$rowsAffected = UpdateData('insert into member_registration (firstname, lastname, middle, idnum, email, pass, bday, course, year) values(?, ?, ?, ?, ?, ?, ?, ?, ?)', $parameters);

Final Thoughts

You'll need to substitute the column names for the fields you have in your database. If any are auto-generated, such as an auto-incrementing ID field, omit that field so it works correctly.

One of your parameters is called $pass. If you're storing passwords in a database, ALWAYS store them in an encrypted form, preferably using bCrypt. This StackOverflow answer explains why/how.

Will Jones
  • 1,861
  • 13
  • 24
  • I do understand your desire to help but it makes your answer way too broad and inconsistent. In the end, it consists of a wild guess, a piece of insecure code and a set of questionable functions. Consider writing an answer that is focused on the certain problem (in case you can tell what the certain problem is) – Your Common Sense Dec 01 '19 at 15:59
  • Your usage example, doesn't explain how one would create an instance of this class you created or how to call the private methods. Why use such complicated function over a few simple lines of PDO code? These functions could be a good idea if they actually provided some tangible benefit and weren't so messy. In general the recommendations are good, but after reading your answer I still don't know what the original problem was. – Dharman Dec 01 '19 at 17:12
  • Sorry, I've removed the Private Static modifiers from the function definitions. That'll teach me to proof read my answers. I don't see how my answer is too broad or inconsistent. I'll concede that the first part has an element of guesswork attached, however IMHO it's a reasonable assumption. If you read about SQL Injection, there are examples there that are insecure; the reason is it teaches you what to avoid. Please explain why you think the functions are questionable? There may be simpler methods, however I've found these useful for a variety of scenarios, hence why I've shared them here – Will Jones Dec 02 '19 at 16:11
  • If you want us to see the messages use @WillJones to ping us. Otherwise we might never see your reply. – Dharman Dec 05 '19 at 20:05
  • There are few things which are questionable about it. 1. You seem to be opening a new connection every time you call this function. 2. You throw exceptions; this is already done by PDO. 3. You use `rowCount()` 4. In one place you use `bindValue` in another you use bind-in-execute. – Dharman Dec 05 '19 at 20:09
  • That's a fair shout, thanks for the tips! – Will Jones Dec 06 '19 at 12:12