Tutorial doesn't work, maybe outdated?

Mike3333 · Mar 19, 2005

Very new to PHP!! I found this on the internet and it doesn't seem to be passing the "ID" variable correctly? The script is suppose to initially display every employee in the database with a link. When you click on the link it's suppose to show you more info than just the Name and it's not working, it just keeps displaying the name. It does seem to capture the variable in the link however. Can someone help?

<html>

<body>

<?php

$db = mysql_connect("localhost", "root");

mysql_select_db("mydb",$db);

if ($id) {

// query the DB

$sql = "SELECT * FROM employees WHERE id=$id";

$result = mysql_query($sql);

$myrow = mysql_fetch_array($result);

?>

<form method="post" action="<?php echo $PHP_SELF?>">

<input type=hidden name="id" value="<?php echo $myrow["id"] ?>">

First name:<input type="Text" name="first" value="<?php echo $myrow["first"] ?>"><br>

Last name:<input type="Text" name="last" value="<?php echo $myrow["last"] ?>"><br>

Address:<input type="Text" name="address" value="<?php echo $myrow["address"] ?>"><br>

Position:<input type="Text" name="position" value="<?php echo $myrow["position"] ?>"><br>

<input type="Submit" name="submit" value="Enter information">

</form>

<?php

} else {

// display list of employees

$result = mysql_query("SELECT * FROM employees",$db);

while ($myrow = mysql_fetch_array($result)) {

printf("<a href=\"%s?id=%s\">%s %s</a><br>\n", $PHP_SELF, $myrow["id"], $myrow["first"], $myrow["last"]);

}

}

?>

</body>

</html>

jpadie · Mar 19, 2005

this was posted when register_globals was (by default) set to off. this is now understood to be a security hole and now is reversed.

you should address the id variable in the post array as $_POST['id'] and in the get array as $_GET['id']

$_POST and $_GET are superglobals. they are available in the scope of all functions.

so your script should be

Code:

<html>

<body>

<?php

$db = mysql_connect("localhost", "root"); //this means you are not using a password - not very wise

mysql_select_db("mydb",$db);

if (isset($_POST['id'])) {

  //clean the string to avoid sql injection
  if (!get_magic_quotes_gpc())
  {
    $id = mysql_real_escape_string(trim($_POST['id']));
  }
// query the DB

  $sql = "SELECT * FROM employees WHERE id='$id'";

  $result = mysql_query($sql)
    or die ("error in query ". mysql_error());    

  $myrow = mysql_fetch_assoc($result); //need to change this to mysql_fetch_assoc to return an associative array

  ?>

  <form method="post" action="<?=$_SERVER['PHP_SELF']?>">

  <input type=hidden name="id" value="<?php echo $myrow["id"] ?>">

  First name:<input type="Text" name="first" value="<?php echo $myrow["first"] ?>"><br>

  Last name:<input type="Text" name="last" value="<?php echo $myrow["last"] ?>"><br>

  Address:<input type="Text" name="address" value="<?php echo $myrow["address"] ?>"><br>

  Position:<input type="Text" name="position" value="<?php echo $myrow["position"] ?>"><br>

  <input type="Submit" name="submit" value="Enter information">

  </form>


  <?php


} else {

  // display list of employees

  $result = mysql_query("SELECT * FROM employees",$db);

  while ($myrow = mysql_fetch_assoc($result)) {

    printf("<a href=\"%s?id=%s\">%s %s</a><br>\n", $SERVER['PHP_SELF'], $myrow["id"], $myrow["first"], $myrow["last"]);

  }

}


?>

</body>

</html>

Mike3333 · Mar 19, 2005

Thank you very much for your help. It's still not working, but you said we needed to address the $_GET['id']
and I don't see in your updated code where you actually do that?

I tried this...

// query the DB

$sql = "SELECT * FROM employees WHERE id=($_GET['$id'])";

$result = mysql_query($sql)

But that didn't help.

jpadie · Mar 19, 2005

Mike

you would only use the GET array if the form were passed using the get method. it is not - it uses the post method.

what is the error message you are receiving?

could you change the db code to the following:

Code:

$db = mysql_connect("localhost", "root")
  or die ("error connecting to database server. ". mysql_error()); //this means you are not using a password - not very wise

mysql_select_db("mydb",$db)
  or die ("error connecting to database.". mysql_error());

and also add the following as the first line after the first php tag

Code:

error_reporting(E_ALL);

thanks
Justin

Mike3333 · Mar 19, 2005

Thanks for working with me on this Justin, I am using a password for the root user, I just copied the orignal code to make it easier.
Funny thing is that I don't get an error on the page, it just doesn't seem to goto the record I click on. Here's the url if you'd like to see how it's acting.

http://www.collisioncraftsmen.com/SelectLink.php3

jpadie · Mar 19, 2005

ok. so in one part of the script you are using a GET method and the other a POST method. the good news is that this still can be made to work although there are better ways to code it as you get more into the language.

change the line

Code:

if (isset($_POST['id'])) {

to

Code:

if (isset($_REQUEST['id'])) {

and the line

Code:

    $id = mysql_real_escape_string(trim($_POST['id']));

to

Code:

    $id = mysql_real_escape_string(trim($_REQUEST['id']));

jpadie · Mar 19, 2005

have just tested your server by shoving a post variable at it. it still doesn't work but this time fails with a sql error. you may be using an older version of php. just to be sure could you change the mysql_real_escape_string line to mysql_escape_string

thanks
Justin

Mike3333 · Mar 19, 2005

Cool!! That took me to what I wanted to see, but with out the data being displayed in the fields. Here's the script again, and if you look at the link I posted before you'll see how it's behaving.

mysql_select_db("MySQL",$db);

if (isset($_REQUEST['id'])) {

//clean the string to avoid sql injection
if (!get_magic_quotes_gpc())
{
$id = mysql_real_escape_string(trim($_REQUEST['id']));
}
// query the DB

$sql = "SELECT * FROM employees WHERE id='$id'";

$result = mysql_query($sql)
or die ("error in query ". mysql_error());

$myrow = mysql_fetch_assoc($result); //need to change this to mysql_fetch_assoc to return an associative array

?>

<form method="post" action="<?=$_SERVER['PHP_SELF']?>">

<input type=hidden name="id" value="<?php echo $myrow["id"] ?>">

First name:<input type="Text" name="first" value="<?php echo $myrow["first"] ?>"><br>

Last name:<input type="Text" name="last" value="<?php echo $myrow["last"] ?>"><br>

Address:<input type="Text" name="address" value="<?php echo $myrow["address"] ?>"><br>

Position:<input type="Text" name="position" value="<?php echo $myrow["position"] ?>"><br>

<input type="Submit" name="submit" value="Enter information">

</form>

<?php

} else {

// display list of employees

$result = mysql_query("SELECT * FROM employees",$db);

while ($myrow = mysql_fetch_assoc($result)) {

printf("<a href=\"%s?id=%s\">%s %s</a><br>\n", $SERVER['PHP_SELF'], $myrow["id"], $myrow["first"], $myrow["last"]);

}

}

?>

jpadie · Mar 19, 2005

version incompatibility. our posts crossed. change the line i referred to above.

Mike3333 · Mar 19, 2005

Did that, it's still not pulling the data

jpadie · Mar 19, 2005

sorry - am being very thick.

exchange these lines

Code:

 if (!get_magic_quotes_gpc())
  {
     $id = mysql_real_escape_string(trim($_REQUEST['id']));
  }

for

Code:

 if (!get_magic_quotes_gpc())
  {
     $id = mysql_real_escape_string(trim($_REQUEST['id']));
  }
else
  { 
     $id = $_REQUEST['id'];
  }

jpadie · Mar 19, 2005

great - i can see it's all working now.

you now need to write the submission of the edited data back into the mysql database. key thing to remember is to clean each incoming variable before using it in the query.

because we are using $_REQUEST you will need to identify the incoming form as the data modifier. for an easy solution inside the current test for the $_REQUEST['id] test to see whether the button has been pressed (isset ($_POST['submit']). if it has been pressed then clean up and write the data (remember it will all be in the $_POST array).

Mike3333 · Mar 19, 2005

You are the MAN, Justin!!! I hope you'll be around for the rest of my outdated tutorial!!

Is this part really blocking SQL Injection? Should this be on all of my PHP pages?

I'm natively an ASP programmer and know how to protect from injection in ASP, would be nice to know.

Do I need to ask to have my PHP upgraded?

Thanks for all you're help today, but I need a DRINK after all this!!

hee-hee

jpadie · Mar 19, 2005

the easiest way to prevent sql injection harming your data is to ensure that every variable is escaped.

i do this like so:

Code:

foreach ($_POST as $key=$val)
{
  $tmparray[$key] = mysql_real_escape_string(trim($val));
// note that you can only use the mysql_real... after the db connection is created. if this isn't possible use mysql_escape_string instead
}

once the strings are clean i then cheat mercillessly by using

Code:

extract ($tmparray);
$tmparray = "";
[code]

this means that $tmparray['id'] becomes just $id which is useful for rapidly shoving it all into a query.

due to the schoolboy error that i missed above i'm not longer sure that you are using an outdated php install. you can check by running the function phpinfo().

you do currently have magic_quotes_gpc turned on though.  I hate this (as do most coders, I believe).  it automatically escapes some characters in data that is contained in the get/post/cookie superglobals.  it makes building portable code more difficult and it encourages lazy programming.  most importantly it escapes the characters with a slash which is fine for most databases but not for all. so if you are using a database abstraction layer you need to be sure to unslash all your incoming data and then re-escape it properly according to your database's needs. yeuch. 

if i were to campaign my ISP to do anything, it would be to turn off magic_quotes_gpc (by the way you may be able to do this yourself through an htaccess file.  have a play.

have fun.
Justin

Tek-Tips is the largest IT community on the Internet today!

Members share and learn making Tek-Tips Forums the best source of peer-reviewed technical information on the Internet!

Tutorial doesn't work, maybe outdated?

Mike3333

Programmer

jpadie

Technical User

Mike3333

Programmer

jpadie

Technical User

Mike3333

Programmer

jpadie

Technical User

jpadie

Technical User

Mike3333

Programmer

jpadie

Technical User

Mike3333

Programmer

jpadie

Technical User

jpadie

Technical User

Mike3333

Programmer

jpadie

Technical User

Similar threads

Part and Inventory Search

Sponsor