-1

I have created a forum where people can register/login to post topics and replies.

Now I added a Delete link next to each topic that if pressed will go to deletetopic.php and if the user has created this topic it will be deleted, if not, it will say You Didn't create this topic.

this is the deletetopic.php

<?php
session_start();
include("config.php");

if(!isset($_SESSION['uid'])){
            echo "<p><b>ERROR: Please log in to delete a topic.";
        }

if(isset($_SESSION['username']))
{
    $uid = $_SESSION['uid']; 
    $id=$_GET['id'];

    $query1=mysql_query("delete FROM topics WHERE id='$id' and uid='$uid'");
    if($query1){
        header('location:index.php');
    }
    else{
        echo "<p><b>ERROR: You didnt make this topic.";
    }
}

It doesnt work, it just gives me the else {error}

here are my tables:

CREATE TABLE `users` (
  `id` INT(11) NOT NULL AUTO_INCREMENT,
  `firstname` VARCHAR(255) NOT NULL,
  `lastname` VARCHAR(255) NOT NULL,
  `email` VARCHAR(255) NOT NULL,
  `username` VARCHAR(255) NOT NULL,
  `password` VARCHAR(100) NOT NULL,

  PRIMARY KEY (`id`)

CREATE TABLE `topics` (
  `id` INT(11) NOT NULL AUTO_INCREMENT,
  `categoryID` TINYINT(4) NOT NULL,
  `topicTitle` VARCHAR(150) NOT NULL,
  `topicCreator` INT(11) NOT NULL,
  `topicLastUser` INT(11) NOT NULL,
  `topicDate` DATETIME NOT NULL,
  `topicReplyDate` DATETIME NOT NULL,
  `topicViews` INT(11) NOT NULL DEFAULT '0',
  PRIMARY KEY (`id`)

EDIT:

uid comes from here I think: login.php

if (isset($_POST['username'])){
    $username = $_POST['username'];
    $password = $_POST['password'];
    $sql = "SELECT * FROM users WHERE username='".$username."' AND password='".$password."' LIMIT 1";
    $result = mysql_query($sql) or die(mysql_error());
    if (mysql_num_rows($result) == 1){
        $row = mysql_fetch_assoc($result);
        $_SESSION['uid'] = $row['id'];
        $_SESSION['username'] = $row['username'];
        header("Location: index.php");
        exit();
    }else{
        echo "<p>Invalid information. Please return to the previous page.";
        exit();
    }
}

Update

if(isset($_SESSION['username']))
{
    $uid = $_SESSION['uid']; 
    $id=$_GET['id'];
    
    $check = mysql_query("SELECT * FROM topics WHERE id = '$id' AND topicCreator = '$uid'");
    if($check){
        $query1=mysql_query("delete FROM topics WHERE id='$id' AND topicCreator='$uid'");
        header('location:index.php');
    }
    else{
        echo "<p><b>ERROR: You didnt make this topic.";
    }
}

Still doesnt work, just goes back to index

Johnny
  • 63
  • 1
  • 7
  • 1
    If you can, you should [stop using `mysql_*` functions](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php). They are no longer maintained and are [officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) [statements](http://php.net/manual/en/pdo.prepared-statements.php) instead, and consider using PDO, [it's really not hard](http://jayblanchard.net/demystifying_php_pdo.html). – Jay Blanchard Jun 18 '15 at 20:54
  • [Your script is at risk for SQL Injection.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – Jay Blanchard Jun 18 '15 at 20:54
  • 3
    There is no `uid` column in `topics` table??? – Rahul Jun 18 '15 at 20:54
  • Add *actual* error checking, such as `or die(mysql_error())` to your queries. Or you can find the issues in your current error logs. – Jay Blanchard Jun 18 '15 at 20:55
  • 1
    Your logic is broken. `mysql_query()` returns false on FAILURE, e.g. a syntax error, db down, no network. A query which returns no results is **NOT** a failure. it's a valid result that happens to have a rowcount of 0. – Marc B Jun 18 '15 at 20:55
  • 1
    maybe the user will be so upset that they can't delete the topic, that they decide to drop the table – Drew Jun 18 '15 at 20:56
  • Rahul I think uid comes from login.php. Edited in the question – Johnny Jun 18 '15 at 20:58
  • I would suggest adding a `view: yes or No` toggle instead of permanently deleting a database entry. – Kirk Powell Jun 18 '15 at 21:00
  • Not only is `mysql_*()` deprecated, it __has been removed__ in PHP 7, now available as an Alpha test product. Do not use it for new code. You have been warned. –  Jun 18 '15 at 21:01
  • I will upgrade once I make my code work – Johnny Jun 18 '15 at 21:03
  • "I will break my code, once I fix it" -- notice the issue here? http://wiki.hashphp.org/PDO_Tutorial_for_MySQL_Developers That will get you started with PDO, The entire question will change once you finish the tutorial :) – steve Jun 18 '15 at 21:25
  • What's great about this code is you can delete anything you want, even the entire database, so you've got a long way to go before this is even close to secure. Please, use parameterized statements. They're not hard. PDO will actually be easier to work with than this mess. – tadman Jun 18 '15 at 21:29
  • side note, this is 2015! Why are people still finding their way to tutorials using 15 year old functions?? :D – steve Jun 18 '15 at 21:31

1 Answers1

2

There is no uid column in table topics, it is probably topicCreator:

$query1=mysql_query("delete FROM topics WHERE id='$id' and topicCreator='$uid'");

You should consider the comments left here about changing from mysql to mysqli or PDO. And use of prepared statements to prevent SQL injections.

There is another problem. You need to check if the user is the topicCreator BEFORE deleting the topic.

$check = mysql_query("SELECT * FROM topics WHERE id = '$id' AND topicCreator = '$uid'");

if($check){
// Allow deletion
}
else{
// Don't allow deletion
}
Kirk Powell
  • 908
  • 9
  • 14