0

I have a Delete.php page that deletes records based on their ID.

When there is an ID, i.e., Delete.php?id=3610, all is well, and it functions as expected.

If I just go to "Delete.php" and that's it - no ID, it generates:

"You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1"

From the little I understand, it is doing this because I am trying to pass a nonexistent variable into my query.

I have been trying to put if (empty($_POST['id'])) { } in different places, which removes the error, but breaks something else.

Here is my code:

  <?php
require_once 'functions.php';
$conn = mysqli_connect("localhost", "user", "pass",'db');
writeHead("Delete Track");
if (isset($_POST['delete'])) {
    $trkid = $_POST['trkid'];
    $query = "DELETE FROM track WHERE TrackID=$trkid";
    mysqli_query($conn, $query) or die(mysqli_error($conn));
        if (mysqli_affected_rows($conn)>0) {
            header("Location: Display.php?action=deleted&id=$trkid&status=deleted");
            exit();
        }
        echo "<p class='error'>Unable to update record</p>";
} else {
    if (!isset($_GET['id'])) {
        echo "<p class='error'>No Track ID provided.<br><a href='Display.php'>Return to display page.</a><p>";
    }
    $trkid=$_GET['id'];
    $query = "SELECT * FROM track WHERE TrackID=$trkid";
    $result = mysqli_query($conn,$query);
    if (!$result) {
        die(mysqli_error($conn));
    }
    if (mysqli_num_rows($result)> 0) {
        $row = mysqli_fetch_assoc($result);
        $Name=$row['Name'];
        $Album=$row['AlbumId'];
        $Composer=$row['Composer'];
        $Milli=$row['Milliseconds'];
        $Bytes=$row['Bytes'];
        $UnitPrice=$row['UnitPrice'];
    } else {
        echo "<p class='error'>Unable to retrieve Track $trkid.<br><a href='Display.php'>Return to display page.</a>";
    }
} 

?>

    <p>Track Information:</p>
    <p><?php echo "<b>ID: $trkid <br>Title: $Name</b>"; ?></p>
        <form method="post" action="Comp3Delete.php">
        <p>
            <input type="hidden" name="trkid" value="<?php echo $trkid; ?>">
            <input type="submit" name="delete" class="btn" value="Confirm Delete">
        </p>
    </form>
    <p>Return to <a href="Comp3Display.php">Track Table Display</a></p>
    <?php writeFoot(); ?>
AdamDallas
  • 67
  • 7
  • 1
    [Little Bobby](http://bobby-tables.com/) says [your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) statements for [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php). Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! – Jay Blanchard Apr 29 '16 at 18:19
  • Try changing `if(!isset($_GET['id'])) {` to `if(isset($_GET['username']) === false) {` – The Codesee Apr 29 '16 at 18:20
  • 1
    OMG, there goes my database!! – Funk Forty Niner Apr 29 '16 at 18:22
  • When writing low-level PHP you have to do all of this yourself. That's why it's usually a lot better to use a [development framework](http://codegeekz.com/best-php-frameworks-for-developers/) which can help handle this for you. Most have a routing layer which decodes parameters and does a bit of validation to help you out. [Laravel](http://laravel.com/) is a good example, it's well documented and has strong, clear conventions for laying out your application. As a plus they also usually have an ORM to help avoid SQL mistakes like you've made here. – tadman Apr 29 '16 at 18:24

2 Answers2

2

Your post code is fine. it's the GET code that's wrong:

if (!isset($_GET['id'])) {
            ^^^^^^^^--check if the parameter exists
}
$trkid=$_GET['id'];
       ^---try to use the parameter ANYWAYS, even if it doesn't exist.
Marc B
  • 356,200
  • 43
  • 426
  • 500
0

$trkid=$_GET['id']; has no condition so it runs even when no id is passed which generates the error. Your code should go like this:

if(isset($_GET['id'])){
$trkid=$_GET['id'];
$query = "SELECT * FROM track WHERE TrackID=$trkid";
$result = mysqli_query($conn,$query);
if (!$result) {
    die(mysqli_error($conn));
}
if (mysqli_num_rows($result)> 0) {
    $row = mysqli_fetch_assoc($result);
    $Name=$row['Name'];
    $Album=$row['AlbumId'];
    $Composer=$row['Composer'];
    $Milli=$row['Milliseconds'];
    $Bytes=$row['Bytes'];
    $UnitPrice=$row['UnitPrice'];
} else {
    echo "<p class='error'>Unable to retrieve Track $trkid.<br><a href='Display.php'>Return to display page.</a>";
}
}
KANAYO AUGUSTIN UG
  • 2,078
  • 3
  • 17
  • 31