tags:

views:

132

answers:

5

check it,

i have this table

tablename: reports
id (AI)
content (varchar),
contentID (int),
checked (tinyint)

 if (isset($_GET['reportPost'])){

 $query = mysql_query("select * from reports where contentID = $_GET[reportPost]");
 $report = mysql_fetch_assoc($query);

 if (!mysql_num_rows($query) && $report['checked'] == 0) {
 echo 'There is already a pending report on this object.';
 } else {
 header("Refresh: 2; url=showthread.php?id=$id");
 echo '<div class="successMsg">Thanks for your report!</div>';
 mysql_query("insert into reports...");
 }

 }

What i want to achieve with this code is that if theres already a record in reports with contentID = $_GET[reportPost] and is checked = 0 I dont want this to execute. The above code wont do it.

Some help would be much appriciated :)

A: 

May be you should try:

 if (mysql_num_rows($query) && $report[0]['checked'] == 0) {

?

artemb
negatory. fetch_assoc returns the first row in the set.
davethegr8
A: 

Some combination of...

$query = mysql\_query("select * from reports where contentID = $\_GET[reportPost] AND checked<>0");
$report = mysql\_fetch\_assoc($query);

if (!mysql_num_rows($query) || $report['checked'] == 0) {
    echo 'There is already a pending report on this object.';
} else {
   ...
}

Notice the second clause in WHERE and || instead of &&

davethegr8
A: 

I would add your condition of checked into the sql statement:

$query = mysql_query("select * from reports where contentID = '" . $_GET[reportPost] . "' AND checked = 0");

Now you only get records in the result if they are checked, therefore you only need to test for the number of rows in the response to your query.

if (!mysql_num_rows($query)) {

Hope this helps.

txwikinger
This still includes SQL injection flaw, and does not have quotes around the user-submitted string. Not to mention using {}s in strings which is a perlism :-)
Ivan Vučica
True. In order to be secure, you need to use quotes. I was more focusing on the question in had, but you are right and I have corrected your points.
txwikinger
+5  A: 
  1. $_GET["reportPost"] should go outside of the string. I don't think PHP escapes that properly, and in any case it's bad practice even with simple variables. You probably want to do this:

    $query = mysql_query("select * from reports where contentID = '" . mysql_real_escape_string($_GET[reportPost]) . "';");
    Basic SQL injection protection thrown in for free :-)

  2. Are you trying to verify if we DIDN'T fetch data, and then attempting to check WHAT we fetched?

        if (!mysql_num_rows($query) && $report['checked'] == 0) {
    That doesn't look right. As suggested by others, either replace && with ||, or the negation (!) has to go.

Oh, by the way, I recommend you take a look at PDO. More practical, and makes it easier to switch to another database backend

EDIT: I forgot to add quotes around string generated by mysql_real_escape_string(). I guess I'm spoiled by PDO where $db->quote() and parametrized queries do that automagically...

Ivan Vučica
To 1.: Indeed, PHP sees `$_GET`, then a literal '["reportPost"]'. You could, however use curly brackets like this: `{$_GET['"reportPost"]}`
Boldewyn
Which still leaves the problem of SQL injection, as well as surrounding user input with single quotes. Unless the input was already sanitized above the chunk of code we're looking at :-)
Ivan Vučica
A: 

why not just:

$query = mysql_query("select * from reports where contentID = $_GET[reportPost] and checked = 0");
$report = mysql_fetch_assoc($query);

if (mysql_num_rows($query)) {
echo 'This object is already been reported but not dealt  with.';
} else {
....
}
Digerdoden