Page 2 of 2

Posted: Wed May 30, 2007 6:06 am
by otd
ok,

so ive changed the link to look like this

Code: Select all

<td colspan="3" bgcolor="#F8F7F1"><a href="../forumgeneral/delete_answer.php?a_id=<? echo $rows['a_id']; ?>&question_id=<? echo $rows['question_id']; ?>" class="style8"><? echo Yes; ?></a></td>
and my page that the link goes to which is the delete_answer.php the code is

Code: Select all

// Connect to server and select databse.
mysql_connect("$host", "$username", "$password")or die("cannot connect"); 
mysql_select_db("$db_name")or die("cannot select DB");

// Get values from form.
$a_id=$_GET['a_id']; 

// Do delete statement. 
mysql_query("delete from forum_answer where a_id='$a_id'");

$tbl_name2="forum_question"; // Switch to table "forum_question" 

// Get values from form.
if ( !isset($_GET['question_id']) ) { 
        echo '<pre>_GET: '; print_r($_GET); echo "</pre>\n"; 
        die('missing parameter'); 
} 

$question_id=(int)$_GET['question_id']; 
$query = "UPDATE forum_question SET reply=reply-1 WHERE id=$question_id";

mysql_close();

// Redirect to select.php.
header("location:main_forum.php"); 

?>
i have tried this and its deleting the answer as before, but not minusing the replies down by 1 still, i think were getting there thou.

You have been a massive help, but any other ideas why this is still not doing this??

Many Thanks

Chris

Posted: Wed May 30, 2007 7:12 am
by volka

Code: Select all

$query = "UPDATE forum_question SET reply=reply-1 WHERE id=$question_id";
only assign a string to the variable $query. You need to send that string to the mysql server via mysql_query().

Same with

Code: Select all

$tbl_name2="forum_question"; // Switch to table "forum_question"
That only assign a string to the variable $tbl_name2, it does not "switch" to any table. In this script it's superfluous.

There's no need to wrap ariables in double quotes, e.g. mysql_select_db($db_name) will do.

Posted: Wed May 30, 2007 8:25 am
by superdezign
And, not to nitpick, but I'm going to nitpick. :-p

Code: Select all

mysql_query("UPDATE forum_question SET reply=reply-1 WHERE id=$question_id");
To me, it should be more like:

Code: Select all

mysql_query("UPDATE `forum_question` SET `reply`=(`reply`- 1) WHERE `id`=$question_id");
You know, security-wise. But that's just me. :-p (This is only if you've already ensured $question_id was type-casted as an integer, which you did. :-D)

Posted: Wed May 30, 2007 8:34 am
by volka
In what way do your changes affect the security of the query?

Posted: Wed May 30, 2007 8:45 am
by otd
Volka, i would like to say a big thank you, you have helped me a tromendous amount, and without people like yourself willing to put the time into these forums and help others, i wouldnt be where i am today, even though im still a newbie beginner in php, once i am an expert like yourself i will do just the same

THANKYOU!!!!

Chris

Posted: Wed May 30, 2007 8:47 am
by otd
That worked perfect!

Thank you again!

Posted: Wed May 30, 2007 8:56 am
by superdezign
volka wrote:In what way do your changes affect the security of the query?
That query in particular... absolutely nothing. :-p
But it makes it sound important, doesn't it? :lol:
(I prefer the format... Makes table names/columns stand out, as it should be.)

But if he ever had a dollar sign in a table name (too far-fetched? :-p), then it could accidentally be calling a bad, bad variable.

Code: Select all

$table = "`$table` SET `whatever` = 3; DELETE `$table`; UPDATE `otherTable`";
mysql_query("UPDATE $table SET something=$somethingElse");
... It could happen. :wink: