1

I have an update information form which does two things:

  • updates information already in the database table.
  • adds information that were not previously stored in the database table.

below is my PHP code:

    <?php
    require_once("../includes/database.php");
?>

<?php
// restricts access to logged in users only
session_start();
if(isset($_SESSION['student_id'])) {
    // do nothing
}
else {
    header('Location: login.php');
}
?>

<?php
    //mysql_
    $connection = mysqli_connect($dbhost, $dbuser, $dbpass, $dbname);
    if(mysqli_connect_errno())
    {
        die("Database connection failed: " .
                    mysqli_connect_error() .
        " (" . mysqli_connect_errno() . ")");
    }
?>

<?php
    // retrieves current user information
    $student_id = $_SESSION['student_id'];
    $query1 = "SELECT * FROM students WHERE student_id = {$student_id}";
    $result1 = mysqli_query($connection, $query1);
    $row = mysqli_fetch_assoc($result1);
    $_SESSION["fname"] = $row["f_name"];
    $_SESSION["lname"] = $row["l_name"];
    $_SESSION["email"] = $row["email"];
    $_SESSION["key"] = $row["password"];

?>
<?php
    // updates database with updated user info
    if(isset($_POST['update'])) {
        $update_f_name = $_POST['fname'];
        $update_l_name = $_POST['lname'];
        $update_email = $_POST['email'];
        $update_pword = $_POST['key'];
        $insert_username = $_POST['username'];
        $insert_city = $_POST['city'];
        $insert_state = $_POST['state'];
        $insert_zip = $_POST['zip'];
        $insert_bio = $_POST['bio'];
        //updates information already in the db
        $query ="UPDATE students 
                SET f_name = '{$update_f_name}', 
                l_name = '{$update_l_name}', 
                email = '{$update_email}', 
                password = '{$update_pword}' 
                WHERE student_id='{$student_id}'";
        //inserts additional information into the db
        $query2 = "INSERT INTO students 
                   (username, city, state, zip, bio) 
                   VALUES('{$insert_username}', '{$insert_city}', '{$insert_state}', '{$insert_zip}', '{$insert_bio}')
                   WHERE student_id = '{$student_id}'";
        $result2 = mysqli_query($connection, $query2);
        $result = mysqli_query($connection, $query);

        if(!$result and !result2){
            die("Database query failed.". mysqli_error($connection));
        }
        header('Location: dashboard.php');
    }

There are no errors displayed and the redirect (to the same page successfully happens). But when i check my database in phpmyadmin, the columns that are supposed to be inserted to (the columns in the $query2 string) still have the value NULL.


Below is my database schema:

    CREATE TABLE students
(
    student_id INT NOT NULL AUTO_INCREMENT,
    username VARCHAR(30),
    email VARCHAR(80),
    password VARCHAR(30),
    f_name VARCHAR(30),
    l_name VARCHAR(30),
    bio VARCHAR(350),
    dp VARCHAR(15),
    is_suspended CHAR(1) DEFAULT '0' NOT NULL,
    suspension_reason VARCHAR(150),
    role_id INT NOT NULL,
    created_on DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
    updated_on TIMESTAMP,
    is_active CHAR(1) DEFAULT '1' NOT NULL,
    city VARCHAR(15) NOT NULL,
    state VARCHAR(15) NOT NULL,
    zip VARCHAR(6) NOT NULL,
    b_day DATE,
    CONSTRAINT students_id_pk PRIMARY KEY(student_id),
    CONSTRAINT students_role_id_fk FOREIGN KEY(role_id) REFERENCES user_roles(role_id) ON DELETE CASCADE,
    CONSTRAINT students_username_uq UNIQUE(username),
    CONSTRAINT students_email_uq UNIQUE(email)
);

EDIT: I understand that my code is prone to SQL injections. I will implement that after the update works perfectly.

snow
  • 455
  • 2
  • 13
  • 28

3 Answers3

0

The session_start() will not be working because you seperate the first block of php from the second by a blank line which outputs space to the output buffer. session_start() must be run before you output anything to the output buffer, so change like this

If you had looked at your PHP error log you would have seen an error

<?php
// restricts access to logged in users only
session_start();

require_once("../includes/database.php");

if(isset($_SESSION['student_id'])) {
    // do nothing
} else {
    header('Location: login.php');
}
?>

There is absolutely no need to keep starting and stopping the PHP interpreter with all these <?php ... ?> <?php ... ?>

You only need one <?php at the start of PHP code and one ?> at the end.

Also the header() will not work for the same reason!

Your code is also full of SQL Injection opertunaties read this to see hwo you can stop this.

This line is also an issue

if(!$result and !result2){

It should be

if(!$result or !result2){

but of course that will leave your database partially updated i.e. one or the other query will have happened but the other one will not.

You also need to look at transactional processing

This also is a problem

$query2 = "INSERT INTO students 
               (username, city, state, zip, bio) 
               VALUES('{$insert_username}', '{$insert_city}', '{$insert_state}', '{$insert_zip}', '{$insert_bio}')
               WHERE student_id = '{$student_id}'";

As an INSERT does not have or support a WHERE clause

Community
  • 1
  • 1
RiggsFolly
  • 93,638
  • 21
  • 103
  • 149
0

If you're taking $_POST data from a client, you want to avoid SQL Injection (see PHP and SQL Injection).

However, I have a feeling that it's inserting null values because the $_POST variables don't exist. I would suggest performing an isset() check on your values.

Aha, found your mistake. As you have if(!$result and !result2){ then your code will only die if both $result AND $result2 fail. Change this to an OR (||) and you should be good to go!

if(!$result || !result2){

And yes, Riggs said, there's no need to keep opening and closing the PHP tags (<?php). If your script is completely PHP, you can safely begin your script with <?php and not bother with the closing tag.

Huw Jones
  • 150
  • 1
  • 9
0

Looking at your second query, MySQL INSERT Syntax does not support the WHERE clause so your query as it stands will fail.

Also I can not see the logic behind an update and insert statement to the same table. Your 'student_id' is PRIMARY KEY so assuming the key exists and your first Update statement is successful the INSERT would naturally fail as the Primary Key will already exist.

You may need to change this to a single statement using; INSERT ... ON DUPLICATE KEY UPDATE ...

e.g.

INSERT INTO students 
                   (student_id, username, city, state, zip, bio) 
                   VALUES('{$student_id}', '{$insert_username}', '{$insert_city}', '{$insert_state}', '{$insert_zip}', '{$insert_bio}')
ON DUPLICATE KEY UPDATE
                f_name = '{$update_f_name}', 
                l_name = '{$update_l_name}', 
                email = '{$update_email}', 
                password = '{$update_pword}'";

See http://dev.mysql.com/doc/refman/5.7/en/insert-on-duplicate.html

Your script will also only exit if both update statements fail.

Also on a general point, if this is a complete script you should really think about the security of your data input, and the mechanism of checking if a user is logged in. There is also non need to stop the PHP interpreter as this will create browser output before script execution.

Matt
  • 106
  • 5
  • A quick note about SQL Injection might be appropriate here also – RiggsFolly Apr 19 '16 at 16:29
  • @Matt Thanks for your suggestion. Although now, i get the error **Database query failed.You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'WHERE student_id = '10' ON DUPLICATE KEY UPDATE f_name = 'Gane' at line 4** which i can't understand one bit. – snow Apr 19 '16 at 16:40
  • @snow. The primary key needs to be in the insert statement and the 'Where' clause needs to be removed. - answer updated to reflect this. http://dev.mysql.com/doc/refman/5.7/en/insert-on-duplicate.html – Matt Apr 19 '16 at 16:46
  • @Matt Updated the code as per your updated answer. Removed redundant php tags. The error was removed. Although the values are still not being inserted into the table. Do you want me to mention any additional code? – snow Apr 19 '16 at 17:05
  • @snow. If these values '{$student_id}', '{$insert_username}', '{$insert_city}', '{$insert_state}', '{$insert_zip}', '{$insert_bio}' are not being inserted, then the update clause of the statement will be running. Have you checked if the primary key (student_id) already exists? If it does, do these fields already contain NULL values? If so you would expect them to remain NULL as they are not being updated in the UPDATE clause. If you want them to be updated, just add them to the update part of the clause. – Matt Apr 19 '16 at 17:17
  • @Matt you inadvertently gave me an amazing idea. What i did was Insert NULL values in the fields where i was supposed to insert new values in my update information page. I made neccessary changes to the query string in my `sign up` page. That way i just ran one update statement and updated the NULL fields. Thank you. – snow Apr 19 '16 at 17:39