forum rating

[WarMad]

in still new to coding and i was starting a forum rating mod and its not working properly it will let a user rate the topic as much as they want can anyone tell me what i did wrong

 

<?php
$i=('globals.php');
include "$i";
$_GET['LID'] = abs((int) $_GET['LID']);
$_GET['DID'] = abs((int) $_GET['DID']);
if($_GET['LID'])
{
global $ir, $userid, $h, $db;
$p = $db->query("SELECT ft_id, ft_name FROM forum_topics WHERE ft_id = ".$_GET['LID']);
$r = $db->fetch_row($p);
echo "You have liked the topic";
$db->query("UPDATE forum_topics SET ft_like=ft_like+1 WHERE ft_id={$r['ft_id']}");
$db->query("INSERT INTO forum_rating VALUES('{$ir['userid']}','{$r['ft_id']}',' 1',' 0')");
}
else if($_GET['DID'] == $tr['fr_topicid'])
{
 echo "you already rated this post";
}
else 
{
  return functionName();
}

function functionName()
{
global $ir, $userid, $h, $db;
$fr = $db->query("SELECT fr_userid, fr_topicid, fr_dislike FROM forum_rating WHERE fr_userid ={$ir['userid']}");
$tr = $db->fetch_row($fr);
if($ir['userid'] == $_GET['fr_userid'] || $tr['fr_dislike']==1)
{echo "you already rated this post";}
else {
$p = $db->query("SELECT ft_id, ft_name FROM forum_topics WHERE ft_id = ".$_GET['DID']);
$r = $db->fetch_row($p);
echo "You have disliked the topic";
$db->query("UPDATE forum_topics SET ft_dislike=ft_dislike+1 WHERE ft_id={$r['ft_id']}");
$db->query("INSERT INTO forum_rating VALUES('{$ir['userid']}','{$r['ft_id']}',' 0',' 1')");
}
} 

?>

[aliali786]

if($_GET['LID']) ? ? ?

Lets say 0 means they havent rated and 1 means they cant rate again

DID is either 0 or 1

if($_GET['DID'] == 0){

blah blah

else if($_GET['DID'] == 1){

echo "you have already rated this post";

} else {

return functionName();

}

[Script47]

in still new to coding and i was starting a forum rating mod and its not working properly it will let a user rate the topic as much as they want can anyone tell me what i did wrong

 

<?php
$i=('globals.php');
include "$i";
$_GET['LID'] = abs((int) $_GET['LID']);
$_GET['DID'] = abs((int) $_GET['DID']);
if($_GET['LID'])
{
global $ir, $userid, $h, $db;
$p = $db->query("SELECT ft_id, ft_name FROM forum_topics WHERE ft_id = ".$_GET['LID']);
$r = $db->fetch_row($p);
echo "You have liked the topic";
$db->query("UPDATE forum_topics SET ft_like=ft_like+1 WHERE ft_id={$r['ft_id']}");
$db->query("INSERT INTO forum_rating VALUES('{$ir['userid']}','{$r['ft_id']}',' 1',' 0')");
}
else if($_GET['DID'] == $tr['fr_topicid'])
{
 echo "you already rated this post";
}
else 
{
  return functionName();
}

function functionName()
{
global $ir, $userid, $h, $db;
$fr = $db->query("SELECT fr_userid, fr_topicid, fr_dislike FROM forum_rating WHERE fr_userid ={$ir['userid']}");
$tr = $db->fetch_row($fr);
if($ir['userid'] == $_GET['fr_userid'] || $tr['fr_dislike']==1)
{echo "you already rated this post";}
else {
$p = $db->query("SELECT ft_id, ft_name FROM forum_topics WHERE ft_id = ".$_GET['DID']);
$r = $db->fetch_row($p);
echo "You have disliked the topic";
$db->query("UPDATE forum_topics SET ft_dislike=ft_dislike+1 WHERE ft_id={$r['ft_id']}");
$db->query("INSERT INTO forum_rating VALUES('{$ir['userid']}','{$r['ft_id']}',' 0',' 1')");
}
} 

?>

 

$i=('globals.php');
include "$i";

 

You do know you could this is one line?

 

include 'globals.php';

 

$_GET['LID'] = abs((int) $_GET['LID']);
$_GET['DID'] = abs((int) $_GET['DID']);

 

Don't use MCCode security ways.

 

$LID = $_GET['LID']; // Add your security.
$DID = $_GET['DID']; // Add your security.

 

Do this instead of typing $_* each time. So what you are saying is: I want the value of $_GET['LID']; to be stored in $LID. It's shorter to type out and easier.

Security:

htmlspecialchars()

Escaping Data

 

Add 0 on to the end of the variable.

That's what I was told and he explained it and it makes perfect sense.

Validation:

ctype_digit()

empty()

isset()

Logical Steps:

 

1. Forum Column - Add a row to the users table called vote_limit or what ever you wish to name it.
   
2. Give it default 0.
   
3. 0 = Not voted. 1 = Voted.
   
4. Cron - Day cron which runs and updates that row and changes it to 0.
   
5. Now you need to check the main file where they vote to see IF they have 0 or 1 in their row.
   
6. Make sure you update their row when they vote to 1.
   
7. If they have 0 let them vote else give them error message.
   

 

Personal Opinion:

Before I actually start making a mod I have a vague idea what I'm doing. Then I plan out the SQL's I'm going to need. So before I write any code I do the SQL's first as I find it's easier to then finish making the script. This is just me personally, I mean everyone is different. ;)

If you need any more help don't hesitate to ask. :)

Edited December 6, 2013 by Script47

[Magictallguy]

[php]
$_GET['LID'] = abs((int) $_GET['LID']);
$_GET['DID'] = abs((int) $_GET['DID']);

 

Don't use MCCode security ways.

 

$LID = $_GET['LID']; // Add your security.
$DID = $_GET['DID']; // Add your security.

 

What?

I'm sorry... What?

The MC Craps version there, surprisingly enough, was more secure and less work on the server.

It's more efficient to secure the data you're using instead of variablising it every chance you get

 

Security:

htmlspecialchars()

Escaping Data

That's what I was told and he explained it and it makes perfect sense.

Validation:

ctype_digit()

empty()

isset()

With the examples posted above, I take it that you misunderstood Alain..

• String
  • Input: escape
    
  • Output: htmlspecialchars/htmlentities
    

  [*]Numeric

  • Input: validate using either ctype_digit/abs/intval
    
  • Output: Shouldn't be necessary if you validated the input properly

   

[KyleMassacre]

I will agree here with MTG abs ((int) ) is decent enough ensuring your getting an absolute value of an (int). What Alain says/does a lot of the times instead of utilizing some functions installed in php is something along the lines of:

$_GET["int"] + 0;

This is because for

1. its pretty simple to do instead of typcasting ints because you will always get an integer.

2. sometimes and I mean sometimes maybe you want or need a negative.

abs () will not allow a negative value to be returned since it converts it to absolute. But just like everything there is a time and place for what to use. for example ctype doesn't allow for strings or negative numbers where as Alains example will allow for both

$int ="-33blah";
echo $int +0;
//returns (int) -33

So ensure you use what your system needs.

Edited December 7, 2013 by KyleMassacre

[Coly010]

Little bit off topic talking about his security problems?

Its hard to understand what is actually wrong with the mod. Can you explain more about where your problem actually lies?

The only thing I can pick from your code, without seeing the SQL is that maybe you shouldn't INSERT INTO, and rather create two columns for the forum_topics called `l_rating` and `d_rating` that way everytime someone likes it or dislikes it you simply have to have a bit of SQL:

 

UPDATE forum_topics SET l_rating = (l_rating + 1) WHERE ft_id = '$ft_id'

UPDATE forum_topics SET d_rating = (d_rating + 1) WHERE ft_id = '$ft_id'

 

or something similar, then a bit of maths can get you the percentage rating:

 

$likes = $db->query("SELECT l_rating FROM forum_topics WHERE ft_id = '$ft_id'");
$dislikes = $db->query("SELECT d_rating FROM forum_topics WHERE ft_id = '$ft_id'");

$total_votes = ($likes + $dislikes);

$percentage = ($likes / total_votes ) * 100;

echo $percentage; 

 

Untested but that should give you the like rating percentage of total votes.

Misread your post, you have got the problem stated. ok. I dont know your SQL however i think something similar to this might be effective:

table: forum_topic_votes

ftv_id  int(11) NOT_NULL  A_I
ft_id int(11) NOT_NULL
userid int(11) NOT_NULL
likes int(11) NOT_NULL
dislike int(11) NOT_NULL

 

then i dont know what your page before looks like but i'm assuming it'll have something like this in it:

 

echo "<a href='forum_rate.php?action=like&ft_id=" . $ft_id . "'>Like</a><br />
<a href='forum_rate.php?action=dislike&ft_id=" . $ft_id . "'>Dislike</a><br />

 

then your forum_rate.php file:

 

<?php
include 'globals.php';
$ft_id = abs((int)$_GET['ft_id']) + 0; // People arguing about security, there's all of it together
$action = $db->real_escape_string($_GET['action']); //  <-- not 100% sure about that 
switch($action)
{

  case like:
  like();
  break;

  case dislike:
  dislike();
  break;

}

function like()
{
  global $userid, $h, $db, $ft_id;

  $sql = "SELECT * FROM forum_topic_votes WHERE userid = '$userid' AND ft_id = '$ft_id'";
  $q = $db->num_rows($sql);
  if( $q != 0 )
  {
     echo "You have already voted!";
     exit;
  }
  else
  {
      // Stop users from rating more than once
      $insert = "INSERT INTO forum_topic_votes(ft_id, userid, likes) VALUES ('$ft_id', '$userid', '1')";
      $db->query($insert);
      // Update the like ratings
      $db->query("UPDATE forum_topics SET l_rating = l_rating+1 WHERE ft_id = '$ft_id'");
      echo " You have liked this topic!";
  }

}

function dislike()
{
  global $userid, $h, $db, $ft_id;
  $sql = "SELECT * FROM forum_topic_votes WHERE userid = '$userid' AND ft_id = '$ft_id'";
  $q = $db->num_rows($sql);
  if( $q != 0 )
  {
     echo "You have already voted!";
     exit;
  }
  else
  {
      // Stop users from rating more than once
      $insert = "INSERT INTO forum_topic_votes(ft_id, userid, dislike) VALUES ('$ft_id', '$userid', '1')";
      $db->query($insert);
      // Update the dislike ratings
      $db->query("UPDATE forum_topics SET d_rating = d_rating+1 WHERE ft_id = '$ft_id'");
      echo " You have disliked this topic!";
  }
}
?>

 

100% untested, but it should work and if it doesn't, then the theory in it should be easily incorporated into code.

For once, I think I was actually helpful :P

Edited December 7, 2013 by Coly010