Page 1 of 1
What's wrong in this code?
Posted: Mon Mar 23, 2009 11:17 am
by MicroBoy
Can anyone tell me what's wrong in this code because when I try to delete "news" It says that the "news" deleted, but still the "news" is in the table:
Code: Select all
<html>
<head>
<link rel="stylesheet" type="text/css" href="menu/stili.css" />
</head>
<body>
<?php
include ('menu/menu.php');
$id = $_GET['id'];
if (empty($id))
{
?>
<TABLE class="col_top" width="800" align="left" cellspacing="0" cellpadding="0" border="0">
<TR>
<TD valign="left">
<?php
if (!$lidhja)
{
die('Nuk Mund Të Konektohet: ' . mysql_error());
}
mysql_select_db("$databaza", $lidhja);
$result = mysql_query("SELECT * FROM lajmet ORDER BY ora DESC");
echo "<table border='1'>
<tr>
<th>Emri</th>
<th>Lajmi</th>
<th>Data&Ora</th>
<th>Fshi</th>
</tr>";
$id = $_GET['id'];
while($row = mysql_fetch_array($result))
{
echo "<tr>";
echo "<td>" . $row['emri'] . "</td>";
echo "<td>" . $row['lajmi'] . "</td>";
echo "<td>" . $row['ora'] . "</td>";
echo "<td><a href=" . $url . "/lexo.php?id=" . $row['id'] . "><img src='menu/el.png' /></a></td>";
echo "</tr>";
}
echo "</table>";
}
if (isset($id))
{
mysql_query("DELETE FROM lajmet WHERE id='$id'");
echo "</br><center><strong>The News Deleted.<strong></br></center>";
}
mysql_close($lidhja);
include ("menu/posht.php");
?>
</TR>
</TABLE>
</body>
</html>
Re: What's wrong in this code?
Posted: Mon Mar 23, 2009 11:56 am
by Apollo
MicroBoy wrote:Code: Select all
mysql_query("DELETE FROM lajmet WHERE id='$id'");
echo "</br><center><strong>The News Deleted.<strong></br></center>";
You're always printing "The News Deleted", regardless of whether it was actually deleted or not. You're not using the result from mysql_query at all!
Try this instead:
Code: Select all
if (mysql_query("DELETE FROM lajmet WHERE id='$id'"))
{
echo "News deleted";
}
else
{
echo "Epic fail :-(";
}
Oh, and by the way: deleting rows based on an ID directly specified by the user seems tricky business to me. Besides the fact that you don't escape anything, so your code is extremely vulnerable to SQL injections.
Re: What's wrong in this code?
Posted: Mon Mar 23, 2009 12:06 pm
by MicroBoy
Now I'm using this, but it reply "Epic fail

" why?
Code: Select all
if (isset($id))
{
mysql_query("DELETE FROM lajmet WHERE id='$id'");
if (mysql_query("DELETE FROM lajmet WHERE id='$id'"))
{
echo "News deleted";
}
else
{
echo "Epic fail :-(";
}
}
Based on what do you suggest to delete the rows?
-------------------------------------------------------------------
By the way:
When I use this code everything it's ok, the news delete, but I don't want to use this because when I delete a news the message and the table are showed in the same time, I want that when I click Delete, to show just the message not the table:
Code: Select all
<html>
<head>
<link rel="stylesheet" type="text/css" href="menu/stili.css" />
</head>
<body>
<?php
include ('menu/menu.php');
?>
<TABLE class="col_top" width="800" align="left" cellspacing="0" cellpadding="0" border="0">
<TR>
<TD valign="left">
<?php
if (!$lidhja)
{
die('Nuk Mund Të Konektohet: ' . mysql_error());
}
mysql_select_db("$databaza", $lidhja);
$result = mysql_query("SELECT * FROM lajmet ORDER BY ora DESC");
echo "<table border='1'>
<tr>
<th>Emri</th>
<th>Lajmi</th>
<th>Data&Ora</th>
<th>Fshi</th>
</tr>";
$id = $_GET['id'];
while($row = mysql_fetch_array($result))
{
echo "<tr>";
echo "<td>" . $row['emri'] . "</td>";
echo "<td>" . $row['lajmi'] . "</td>";
echo "<td>" . $row['ora'] . "</td>";
echo "<td><a href=" . $url . "/lexo.php?id=" . $row['id'] . "><img src='menu/el.png' /></a></td>";
echo "</tr>";
}
echo "</table>";
if (isset($id))
{
mysql_query("DELETE FROM lajmet WHERE id='$id'");
echo "</br><center><strong>News Deleted.<strong></br></center>";
}
mysql_close($lidhja);
include ("menu/posht.php");
?>
</TR>
</TABLE>
</body>
</html>
Re: What's wrong in this code?
Posted: Mon Mar 23, 2009 12:08 pm
by jayshields
Remove the first myql_query() call. You'd be better off refactoring your code to include some debugging info, such as using mysql_query($query) or die(mysql_error()). Also, you should really sort out your security vulnerabilities as Apollo said.
Re: What's wrong in this code?
Posted: Mon Mar 23, 2009 12:26 pm
by MicroBoy
When I used "die(mysql_error())" it said "No database selected" then I use:
Code: Select all
mysql_select_db("$databaza", $lidhja);
if (isset($id))
{
if (mysql_query("DELETE FROM lajmet WHERE id='$id'"))
{
echo "News deleted";
}
else
{
die(mysql_error());
}
}
But I'm still not understanding why it didn't select the database from the code upper, because I used "mysql_select_db("$databaza", $lidhja);"?
jayshields wrote:Also, you should really sort out your security vulnerabilities as Apollo said.
How to do that? Any suggest?
Re: What's wrong in this code?
Posted: Mon Mar 23, 2009 12:37 pm
by jayshields
Again, use "or die(mysql_error())" after the mysql_select_db() call. Don't use quotes around your database name variable. Have you actually established a database connection and saved the handle to $lidhja?
Use mysql_real_escape_string() to safe-guard against SQL injection.
These points I am raising are extremely common. I have brought them up about 3 times in the last couple of days. Simply searching Google or reading the PHP manual would have told you all this and more.
Re: What's wrong in this code?
Posted: Mon Mar 23, 2009 12:50 pm
by MicroBoy
Thnx a lot for help. I'm still curious to know why I have to use "mysql_select_db("$databaza", $lidhja);" twice?
jayshields wrote:Have you actually established a database connection and saved the handle to $lidhja?
Yes I did.
Re: What's wrong in this code?
Posted: Mon Mar 23, 2009 4:34 pm
by jayshields
MicroBoy wrote:I'm still curious to know why I have to use "mysql_select_db("$databaza", $lidhja);" twice?
You don't. Your script doesn't show you doing that either. Have you posted your full script?
Re: What's wrong in this code?
Posted: Mon Mar 23, 2009 5:32 pm
by MicroBoy
Look, when I delete the code in the 49 line, the news doesn't delete. Why the 23 line doesn't work for all the code but I have to use the same code twice?
Code: Select all
<html>
<head>
<link rel="stylesheet" type="text/css" href="menu/stili.css" />
</head>
<body>
<?php
include ('menu/menu.php');
$id = $_GET['id'];
if (empty($id))
{
?>
<TABLE class="col_top" width="800" align="left" cellspacing="0" cellpadding="0" border="0">
<TR>
<TD valign="left">
<?php
if (!$lidhja)
{
die('Nuk Mund Të Konektohet: ' . mysql_error());
}
mysql_select_db("$databaza", $lidhja) OR die(mysql_error());
$result = mysql_query("SELECT * FROM lajmet ORDER BY ora DESC");
echo "<table border='1'>
<tr>
<th>Emri</th>
<th>Lajmi</th>
<th>Data&Ora</th>
<th>Fshi</th>
</tr>";
$id = $_GET['id'];
while($row = mysql_fetch_array($result))
{
echo "<tr>";
echo "<td>" . $row['emri'] . "</td>";
echo "<td>" . $row['lajmi'] . "</td>";
echo "<td>" . $row['ora'] . "</td>";
echo "<td><a href=" . $url . "/lexo1.php?id=" . $row['id'] . "><img src='menu/el.png' /></a></td>";
echo "</tr>";
}
echo "</table>";
}
mysql_select_db("$databaza", $lidhja) OR die(mysql_error());
if (isset($id))
{
if (mysql_query("DELETE FROM lajmet WHERE id='$id'"))
{
echo "News deleted";
}
else
{
die(mysql_error());
}
}
mysql_close($lidhja);
include ("menu/posht.php");
?>
</TR>
</TABLE>
</body>
</html>
p.s. I'm just curious
Re: What's wrong in this code?
Posted: Mon Mar 23, 2009 11:07 pm
by Apollo
MicroBoy wrote:Based on what do you suggest to delete the rows?
1. An escaped id, not just a plain id from the URL. What do you think will happen if some smartass fills in
id=1'; DELETE * FROM lajmet; ?
Use
mysql_real_escape_string. Or in the case of a numeric ID, use
intval.
2. Not an id that can be freely chosen by the user. Someone can call the delete script with id=1, id=2, etc. Why would you allow that? So verify / validate the id.
Re: What's wrong in this code?
Posted: Tue Mar 24, 2009 5:22 am
by sujithtomy
Hello,
why don't you use mysql_affected_rows() to know whether there is any rows get affected due to query
------------
Sujith