-2

I have just migrated my page from mysql to msqli. Now, deleting data is confusing. Here is my code in Admin_Delete.php

require 'Connect.php';
$id = intval($_GET['id']);
$query = "DELETE * FROM student_information WHERE student_id='$_GET[id]'";
// not using return value, and add some debug info
mysqli_query($query) or die(mysql_error().PHP_EOL.$query);
// let's see if anything actually happened...
$rowsDeleted = mysqli_affected_rows();
if($rowsDeleted == 0) {
    error_log("Nothing deleted for this query:".PHP_EOL.$query);
}
echo "<script language='javascript' type='text/javascript'>alert('$rowsDeleted row(s) deleted!')</script>";
echo "<script language='javascript' type='text/javascript'>window.open('Admin_Home.php','_self')</script>";
?>

This is my configuration to fix these. Connect.php

<?php
$host = "localhost";
$dbusername = "root";
$dbpassword = "123456";
$dbname = "student";
$link_id = mysqli_connect($host,$dbusername,$dbpassword,$dbname) or die("Error " . mysqli_error($link_id)); 
?>
Steve H.
  • 6,912
  • 2
  • 30
  • 49
user3481449
  • 13
  • 1
  • 5
  • 6
    **Danger**: You are **vulnerable to [SQL injection attacks](http://bobby-tables.com/)** that you need to [defend](http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php) yourself from. – Quentin Sep 11 '14 at 15:04
  • 4
    Don't worry, with that SQL injection vulnerability I'm sure *somebody* will delete your student data. – David Sep 11 '14 at 15:05
  • 2
    Get rid of the `*` in `DELETE * FROM` for starters. – Funk Forty Niner Sep 11 '14 at 15:07
  • You are not elaborating on what your confusion actually is. What are your inputs and most importantly, what is your expected and actual output? Are you getting any error messages? See http://www.sscce.org/ – Nathaniel Ford Sep 11 '14 at 15:09
  • Obvious sql injection issues aside, it would be helpful to see the query string being used to call the php script so we can see what is going into $_GET. (If this is a public site, you should prolly hide the domain) – Parris Varney Sep 11 '14 at 15:09
  • 1
    You're also mixing MySQL APIs using `mysql_error()` they do **not** mix, unlike *gin & tonic* ;) Add error reporting to the top of your file(s) right after your opening ` – Funk Forty Niner Sep 11 '14 at 15:11
  • @Quentin i think the 4th solution given by Spencer7539 solves the danger you stated – user3481449 Sep 14 '14 at 02:58
  • @Fred-ii-Actually it was on my mind that i should also put the errors display maybe i forget it or it was omitted when my questions was edited by Steve. I feel sorry of this forgetfulness of mine for not reporting errors i lose 3 reputations so sad. – user3481449 Sep 14 '14 at 03:01
  • @ParrisVarney Yes this is a public site. I was also considering before to hide the admin domain for safety and based on your comment it adds to my idea that i should do it. Thanks for the confirmed. – user3481449 Sep 14 '14 at 03:03

1 Answers1

1

A couple of problems.

The reference to mysql_error should be mysqli_error($link_id).

The reference to mysqli_query should be mysqli_query($link_id, $query).

The reference to mysqli_affected_rows should be mysqli_affected_rows($link_id)

Also, you've used intval, to get an integer value from $_GET, but you're using reference to $_GET in the SQL text. If you aren't going to use a prepared statement, then you should be using mysqli_real_escape_string function to make potentially "unsafe" values "safe" for inclusion in SQL text.

$sql = " ... WHERE id='" . mysqli_real_escape_string($link_id, $id) . "'";

Syntax for DELETE statement is not correct. Either omit the *

DELETE FROM student_information WHERE ...

Or qualify the * with a table reference, or table alias

DELETE s.* FROM student_information s WHERE ...
spencer7593
  • 106,611
  • 15
  • 112
  • 140
  • *"The reference to `mysqli_query` should be `mysqli_query($link_id, $query)`"* - I missed that one, good catch +1 – Funk Forty Niner Sep 11 '14 at 15:21
  • @spencer7593 Your answer acquires the best answer it solves the issues that i needed the most if not all. Thanks and God bless for sharing. – user3481449 Sep 14 '14 at 03:06