broken code after register globals are off

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
decoy1
Forum Commoner
Posts: 50
Joined: Fri Feb 21, 2003 1:33 pm
Location: St. Louis

broken code after register globals are off

Post by decoy1 »

Hi,

The following (abbreviated) code used to work before register globals was turned off, now it doesn't. Actually the update part works, the delete part doesn't. I've gone through a ton of code and was able to fix everything but this.

The while loop justs pulls the links from the db and displays them. I pass the variable $task=delete_link in the querystring to invoke the top block of code. Simple stuff, but not working after upgrading to 4.2

Code: Select all

if($task == 'delete_link')  
{ 
  $sql = "DELETE FROM links WHERE linkid = $linkid";
  $result = mysql_query($sql); 
  if(!$result)
  {
    echo("ERROR: " . mysql_error() . "\n$sql\n");
    exit();
  }
}

$sql = "SELECT * FROM links order by linkid asc"; 
$result = mysql_query($sql);
while($myrow = mysql_fetch_array($result))
{ 
  $linkname = $myrow["name"]; 
  $linkid = $myrow["linkid"]; 
  echo "<tr>"; 
  echo "<td>$linkname</td>\n"; 
  echo "<td><A HREF="update.php?linkid=$linkid" class=mediumbold>Edit</a></td>"; 
  echo "<td><A HREF="admin.php?linkid=$linkid&task=delete_link" class=mediumbold>Delete</a></td>"; 
  echo "</tr>"; 
}
Can someone point me in the right direction?

Thanks :)
McGruff
DevNet Master
Posts: 2893
Joined: Thu Jan 30, 2003 8:26 pm
Location: Glasgow, Scotland

Post by McGruff »

With register globals off, you need to write query string vars like $task in this way: $_GET['task']

So replace:

if($task == 'delete_link')

with..

if($_GET['task'] == 'delete_link')

Also, where does $linkid come from? If it's in the global scope you can get it into a function scope with $GLOBALS['linkid']. I don't know the rest of your code though - that might not be necessary.
decoy1
Forum Commoner
Posts: 50
Joined: Fri Feb 21, 2003 1:33 pm
Location: St. Louis

Post by decoy1 »

Hi there,

The linkid comes directly from the query. You can see in my first post. Also, I tried doing

Code: Select all

if($_GET['task'] == 'delete_link')
from the start thinking that was the problem, but always got and sql error on that line?
User avatar
twigletmac
Her Royal Site Adminness
Posts: 5371
Joined: Tue Apr 23, 2002 2:21 am
Location: Essex, UK

Post by twigletmac »

What was the error you got?

Mac
decoy1
Forum Commoner
Posts: 50
Joined: Fri Feb 21, 2003 1:33 pm
Location: St. Louis

Post by decoy1 »

Hi,
This is the error...
ERROR: You have an error in your SQL syntax near '' at line 1 DELETE FROM links WHERE linkid =

This happens when I use

Code: Select all

if($_GET['task'] == 'delete_link')
Obviously it doesn't recognize $linkid when I use that. Driving me nuts
User avatar
volka
DevNet Evangelist
Posts: 8391
Joined: Tue May 07, 2002 9:48 am
Location: Berlin, ger

Post by volka »

just think about it. if you had to replace
if($task == 'delete_link')
by

Code: Select all

if($_GET['task'] == 'delete_link')
what need's to be done with
$sql = "DELETE FROM links WHERE linkid = $linkid";
?
You have 1 guess ;)
Just one tip: start from

Code: Select all

$sql = "DELETE FROM links WHERE linkid = " . $linkid;
otherwise you might get another error if you implement the changes straight forward.

and please read: Sticky: Before Post Read: Concerning Passing Variables in PHP 4.2+
decoy1
Forum Commoner
Posts: 50
Joined: Fri Feb 21, 2003 1:33 pm
Location: St. Louis

Post by decoy1 »

Hi,

I read the sticky posts and it didn't help. There were a ton of things broken when I first upgraded and I've got them all addressed, its just this one thing. I don't know if I need to sleep on it or what, but It's not registering.

Could you tell me how to fix it? :D
User avatar
volka
DevNet Evangelist
Posts: 8391
Joined: Tue May 07, 2002 9:48 am
Location: Berlin, ger

Post by volka »

try

Code: Select all

$sql = "DELETE FROM links WHERE linkid=".$_GET['linkid'];
or even better

Code: Select all

$sql = "DELETE FROM links WHERE linkid=".(int)$_GET['linkid'];
(linkid is a numerical field in your table?)
decoy1
Forum Commoner
Posts: 50
Joined: Fri Feb 21, 2003 1:33 pm
Location: St. Louis

Post by decoy1 »

Man...that worked and boy do I feel stupid :oops:

After your last post I immediately did this

Code: Select all

$sql = "DELETE FROM links WHERE linkid=".$_GET['linkid'];
But wasn't using

Code: Select all

if($_GET['task'] == 'delete_link')
and THEN did it vice versa, never using them at the same time. I'ts so obvious now.

I'll slink off now. Thanks to you and everyone else for your time, much appreciated.
McGruff
DevNet Master
Posts: 2893
Joined: Thu Jan 30, 2003 8:26 pm
Location: Glasgow, Scotland

Post by McGruff »

One tiny amendment:

$sql = "DELETE FROM links WHERE linkid='".$_GET['linkid']."'";

I half-remember being advised always to enclose column vars in single quotes (even numerical input) for security reasons. Think it's something to do with query injection ie forcing mysql to treat the item as a string so that any ";" are neutralised.

I guess the (int) cast achieves much the same thing though - just thought I'd mention another option.
User avatar
volka
DevNet Evangelist
Posts: 8391
Joined: Tue May 07, 2002 9:48 am
Location: Berlin, ger

Post by volka »

if you're passing user input as a string to mysql, you should use something like mysql_escape_string()

http://www.php.net/manual/en/function.m ... string.php
Post Reply