is statement ending code :/

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
Gemster
Forum Newbie
Posts: 13
Joined: Thu Jul 29, 2010 10:38 am

is statement ending code :/

Post by Gemster »

Hi, for some reason when this part of the file is activated by pressing a button called "Add" it stops on the last "}" and any code after that gets ignored like the end of a table </table> ect

Code: Select all

<?php
 $data = mysql_query("SELECT * FROM IP") 
 or die(mysql_error());
if($_POST['ipSubmit'] == 'Add')
{
if($ip == "") 
die("<hr>You must add an ip or host");
if($ipnote == "") 
die("<hr>You must add a reason.");
 while($bip = mysql_fetch_array( $data )) 
 {
if($ip == $bip['ip']) 
die("<hr>Ip already exsists.");
}
 mysql_query("INSERT INTO `IP` VALUES ('$ip', '$ipnote')"); 
Print "<hr>Ip has been successfully added to the blacklist database.";
}
?>
Idk why but any code after that gets ignored :/

Thanks
Gemster
User avatar
califdon
Jack of Zircons
Posts: 4484
Joined: Thu Nov 09, 2006 8:30 pm
Location: California, USA

Re: is statement ending code :/

Post by califdon »

Where does the value of $ip get assigned?
User avatar
Jonah Bron
DevNet Master
Posts: 2764
Joined: Thu Mar 15, 2007 6:28 pm
Location: Redding, California

Re: is statement ending code :/

Post by Jonah Bron »

Where are the variables $ip and $ipnote coming from?

Lets clean up the code a bit to see what it does:

Code: Select all

<?php

$data = mysql_query("SELECT * FROM IP") or die(mysql_error());

if($_POST['ipSubmit'] == 'Add') {
	if($ip == "")
		die("<hr>You must add an ip or host");

	if($ipnote == "")
		die("<hr>You must add a reason.");
 
	while($bip = mysql_fetch_array( $data )) {
		if($ip == $bip['ip'])
			die("<hr>Ip already exsists.");
	}
	mysql_query("INSERT INTO `IP` VALUES ('$ip', '$ipnote')");
	Print "<hr>Ip has been successfully added to the blacklist database.";
}

?>
It's not efficient to check each IP with PHP. Do it in the query. And there's no need to execute the query if the request is not valid, so lets place it inside of the main if statement. Plus, we have to protect the code against SQL injection by cleaning the input with mysql_real_escape_string().

Code: Select all

<?php

$ip = mysql_real_escape_string($ip);
$ipnote = mysql_real_escape_string($ip);

if($_POST['ipSubmit'] == 'Add') {
	if($ip == "")
		die("<hr>You must add an ip or host");

	if($ipnote == "")
		die("<hr>You must add a reason.");

	$result = mysql_query("SELECT * FROM IP WHERE ip = '$ip'") or die(mysql_error());
	
	if(mysql_num_rows($result) > 0)
		die("<hr>Ip already exsists.");
	
	mysql_query("INSERT INTO `IP` VALUES ('$ip', '$ipnote')");
	echo "<hr>Ip has been successfully added to the blacklist database.";
}

?>
http://php.net/mysql-real-escape-string
Gemster
Forum Newbie
Posts: 13
Joined: Thu Jul 29, 2010 10:38 am

Re: is statement ending code :/

Post by Gemster »

The $ip and $ipnote are part of the file at the top and it all works fine apart from the section i posted, well it does work but the last "}" close bracket stops any code below it from working.

Like say i have </table> below the last "}" it ignores it, basically it ignore any code below the last "}"

all the code works apart from that.

Thanks
Gemster
User avatar
Jonah Bron
DevNet Master
Posts: 2764
Joined: Thu Mar 15, 2007 6:28 pm
Location: Redding, California

Re: is statement ending code :/

Post by Jonah Bron »

Did you try the code I posted (the second one)?
Gemster
Forum Newbie
Posts: 13
Joined: Thu Jul 29, 2010 10:38 am

Re: is statement ending code :/

Post by Gemster »

Jonah Bron wrote:Did you try the code I posted (the second one)?
Yes ive tryed it but a few problems.

When it says ip already exsist it stops at the last "}" still and same with you must add a ip.

When no reason is entered it still carrys on with submitting the ip with no reason.

when it says Ip has been successfully added to the blacklist database., it does not update the list causeing me to refresh to see it.

Thanks
Gemster
User avatar
Jonah Bron
DevNet Master
Posts: 2764
Joined: Thu Mar 15, 2007 6:28 pm
Location: Redding, California

Re: is statement ending code :/

Post by Jonah Bron »

Gemster wrote:When it says ip already exsist it stops at the last "}" still and same with you must add a ip.
That's because you told it to. The die() function does the same thing as echo, and then it stops the script. If you don't want it to stop, you have to use echo instead, and turn the two if statements into an if...if... else statement.
Gemster wrote:When no reason is entered it still carrys on with submitting the ip with no reason.
Then we probably need a better way of checking it. Try using empty().
Gemster wrote:when it says Ip has been successfully added to the blacklist database., it does not update the list causeing me to refresh to see it.
Make sure that the query for getting the blacklisted IPs is after this code, not before.

If it still doesn't work, post the code with the changes I've mentioned.
Gemster
Forum Newbie
Posts: 13
Joined: Thu Jul 29, 2010 10:38 am

Re: is statement ending code :/

Post by Gemster »

Thanks Jonah Bron,

I decided to rewrite the whole file from scratch and make changed you suggested.

Anyways i have it all working now as i need it :D

Code: Select all

if($_POST['ipSubmit'] == 'Add')
{
if($ip == "") {
echo "<hr>You must add an ip or host";
} elseif($ipnote == "") {
echo "<hr>You must add a reason";
} elseif(mysql_num_rows($result) > 0) {
echo "<hr>Ip already exsists";
} else {
 mysql_query("INSERT INTO `IP` VALUES ('$ip', '$ipnote')"); 
Print "Ip has been successfully added to the blacklist database.";
}
}
I used ifelse and else for this to work and echo as you stated instead of die.

Just 1 thing tho, when i add an ip and reason, it works and adds it to the database but does not update the list untill i refresh the page, i have the code that lists the ips below the above code.

Thanks
Gemster
User avatar
Jonah Bron
DevNet Master
Posts: 2764
Joined: Thu Mar 15, 2007 6:28 pm
Location: Redding, California

Re: is statement ending code :/

Post by Jonah Bron »

You want it the other way around. Think about it for a moment... do you want to get the list of IPs before you update it? Of course not, that data is obsolete. You want to get the data after the IP is added.
Post Reply