0

I have the following php code that seeks to

a) ask a user to enter a school pin b) Look up the database table 'teachers' and then overwrite the new value (entered into the textbox) over the old value of the school pin in the database.

Currently the issue is that it does ADD a new row to the table with the new entered pin, but I want it to overwite the existing value for that particular teacher (basically overwrite the existing value).

The php handler code is below:

if (isset($_POST["addpin1"]) && !empty($_POST['schoolpin1']) && $_SESSION['teacher']) {
    $school_pin         = mysqli_real_escape_string($con,$_POST['schoolpin1']);
    $email              = $_SESSION['email'];
    $username           = $_SESSION['username'];
    $pass               = $_SESSION['pass'];
    $check_duplicate = mysqli_query($con,"SELECT school_pin FROM teachers WHERE school_pin='$school_pin' ");
    if(@mysqli_num_rows($check_duplicate) == 0){
        $query = mysqli_query($con,"INSERT INTO teachers (school_pin,email,username,pass) 
            VALUES('$school_pin','$email','$username','$pass')");
        if($query){
            header("Location: teachers.php");   
        }
    }
}

The HTML form code:

 <form action="" method="post">
    <input type="text" name="schoolpin1" id="schoolpin1">
    <input type="submit" class="btn btn-success" name="addpin1" value="CHANGE PIN">

 </form>

Database Structure (tbl: teachers) Fields shown below

id  
email  
school_pin  
username  
pass

The page on which this is occuring has a teacher logged in, so their username is displayed on the screen.

UPDATE: As per one of the answers regarding starting session. This has been done

<?php
require_once("scripts/connect_db.php");

session_start();
if(!isset($_SESSION['teacher'])) header('Location: teacher_login.php');;
$result="";
if (isset($_POST["logout"])) {


    session_destroy();
   header('Location:index.php');

}
$err = "";
Compoot
  • 2,227
  • 6
  • 31
  • 63
  • I only see an `INSERT` statement. You'll add new rows no matter what. If you want to overwrite an existing row, you need an `UPDATE` statement. – WillardSolutions Feb 02 '18 at 22:04
  • @WillardSolutions are you able to post your suggested solution in the context of my code please? – Compoot Feb 02 '18 at 22:05
  • use UPDATE then or INSERT ON DUPLICATE – Funk Forty Niner Feb 02 '18 at 22:06
  • I had tried UPDATE and it resulted in new errors such as: Notice: Undefined index: email in C:\xampp\.php on line 19. Basically, it then stopped recognising the declared sessions. – Compoot Feb 02 '18 at 22:07
  • Can either of you @FunkFortyNiner please post an answer with code (inside of my existing code and context) please – Compoot Feb 02 '18 at 22:08
  • seeing that error of yours here, you may not have started the session and/or there is no value for the session array – Funk Forty Niner Feb 02 '18 at 22:09
  • You can simply use `REPLACE` instead of `INSERT` assuming that you have a primary key field: https://dev.mysql.com/doc/refman/5.7/en/replace.html – Exit Feb 02 '18 at 22:09
  • Do I need the sessions at all, or can I simply look up a field and replace it directly? Again, code in answers would be appreciated! – Compoot Feb 02 '18 at 22:11
  • @Exit - I tried exactly that: $query = mysqli_query($con,"REPLACE INTO teachers (school_pin,email,username,pass) but it did the same thing: Just added a new row into the table 'teachers' instead of replacing the row with the matching school pin – Compoot Feb 02 '18 at 22:12
  • I also tried changing one of the lines to: if(@mysqli_num_rows($check_duplicate) == 1){ but that resulted in the undefined index errors for email, username and pass again. – Compoot Feb 02 '18 at 22:14
  • For your table structure, you would need to set `school_pin` to `UNIQUE` so that it knows to replace when a duplicate is found. – Exit Feb 02 '18 at 22:16
  • I'm assuming that there would never be more than one row with the same `school_pin`. – Exit Feb 02 '18 at 22:17
  • 1
    Also, this script is vulnerable to MySQL injection, you should be using prepared statements and making sure each field being used is properly validated and cleansed. – Exit Feb 02 '18 at 22:19

3 Answers3

0

If you are using session variables at all, you need to start the session first. Just add session_start() at the beginning of your script.

Dustin Hammack
  • 154
  • 1
  • 1
  • 15
  • no. I already have that (which is a given as the code doesn't always produce the session variable errors). I will update the code and question to include the fact that this is there and working fine – Compoot Feb 02 '18 at 22:16
  • Yeah I figured you would have had it, but I read your comment above about using session variables at all.. Then I reread it after writing my comment... I think I misunderstood it at first. – Dustin Hammack Feb 02 '18 at 22:18
0

It's not clear quite what you are asking - what are your primary keys?

You say "Look up the database table 'teachers' and then overwrite the new value (entered into the textbox) over the old value of the school pin in the database" so I guess you want to update the value of school pin for the current teacher? The main logic problems are:

  • when you are checking for duplicates, you are looking for the new pin and doing an insert if you don't find it

  • you are never doing an update!

The first point: you have 

$check_duplicate = mysqli_query($con,"SELECT school_pin FROM teachers WHERE school_pin='$school_pin' ");

I think you want 

SELECT school_pin FROM teachers WHERE username=` not `SELECT school_pin FROM teachers WHERE school_pin=

The second, you need an else block on your if that does an update. Alternatively (if username is a primary key for example) you could use the insert ... on duplciate key update a nd let mysql handle the check for if the row exists. See https://dev.mysql.com/doc/refman/5.7/en/insert-on-duplicate.html

And please, look up how to do parametrised queries and use them instead of strings like you have here. Start with How can I prevent SQL injection in PHP?

Adam
  • 6,539
  • 3
  • 39
  • 65
  • my assumption is that school_pin is not being looked up in the database, but it is generated from the SESSION declarations...and that is what it is checking. So: check the current school_pin (by using the session, I assume) and then once found, overwrite it with the new pin. Re. sql injections, doesn't this have something to do with protection: $school_pin = mysqli_real_escape_string($con,$_POST['schoolpin1']); – Compoot Feb 02 '18 at 22:25
  • You are using the same variable, `$school_pin`, both in the select for `$check_duplicate` and as the new value when inserting. As I and others have said, you need an else to do the update, but what are you wanting to update: are you wanting to find the teacher and update the school pin associated with that teacher, or find the school pin and update the teacher associated with the pin? You need to be clear what are the keys (unique values that you search for and look up using) and what are the data you are updating. It's not clear at the moment. – Adam Feb 03 '18 at 16:57
  • Yes, `mysqli_real_escape_string` has something to do with protection. However, you are still using the session variables without checking them. Rule number one of web development is never to trust any user-provided data. The best practice is to use parametrised queries, not `mysqli_real_escape_string` and the like directly. Check the answers on the question I linked. – Adam Feb 03 '18 at 16:59
  • Thank you. I wish to find the teacher or find the schoolpin that is already displayed on the page (for that teacher that is logged in), and then change that school pin to whatever is entered in the form. The issue is that it is currently a) not finding the right record/field and b) writing to a new row instead of updating the one field in that row. Example: TeacherA logs in and has school pin of: 1234. (both username of teacher and school pin are on page). TeacherA enters 3432 and wants to change pin 1234 for TeacherA record to 3432. @Adam – Compoot Feb 03 '18 at 22:57
  • @MissComputing: _"find the teacher or find the schoolpin that is already displayed on the page ... and then change that school pin to whatever is entered in the form"_ I think you need to decide which! As I said, you are currently checking for the _new_ pin but I think you need to think about the data model a bit. What are your keys? What fields should be unique? – Adam Feb 04 '18 at 21:35
0

This is the kind of structure you need. If a current records exists, update it with the new info. Else insert. Adjust accordingly to what you want to update. Also as a further point. Please don't put user input directly into the database. Use prepared statements by binding parameters to the query. 

 if(@mysqli_num_rows($check_duplicate) == 0){
    $query = mysqli_query($con,"INSERT INTO teachers (school_pin,email,username,pass) 
                VALUES('$school_pin','$email','$username','$pass')");
    if($query){
        header("Location: teachers.php");
    }
}else{

    $query = mysqli_query($con,"
          UPDATE teachers 
           SET email = '$email'  ,username = 
             '$username',pass = '$pass'
          WHERE school_pin='$school_pin'
     ");
    if($query){
        header("Location: teachers.php");
    }

}
Juakali92
  • 1,155
  • 8
  • 20