Can you guys improve this?

Trium918 · April 19, 2007

Problem: Create a registeration form that will populate

two different database tables using PHP and MySQL.

I implemented a html form and the php script handler.

The first script attempts to register the user by calling

the register() function. The register() register new person with db

or return an error. The code is working great but I am sure that

it can be improved.

Note: Initialized in another file. require_once(" ");

<?php
  // register_global = Off
  // inside the php.ini configuration file  
  // Registration Form
   $user_name = addslashes($_POST['user_name']);
   $email_address = addslashes($_POST['email_address']);
   $first_name  = addslashes($_POST['first_name']);
   $last_name  = addslashes($_POST['last_name']);
   $birth_month = addslashes($_POST['birth_month']);
   $birth_day = addslashes($_POST['birth_day']);
   $birth_year = addslashes($_POST['birth_year']);
   $gender = addslashes($_POST['gender']);

   $password = addslashes($_POST['password']);
   $cpassword = addslashes($_POST['cpassword']);
   
   $street_address = addslashes($_POST['street_address']);
   $city_county = addslashes($_POST['city_county']);
   $state = addslashes($_POST['state']);
   $postal_code  = addslashes($_POST['postal_code']);
   $contact_number = addslashes($_POST['contact_number']);
   
   $old_passwd = addslashes($_POST['old_passwd']);
   $new_passwd = addslashes($_POST['new_passwd']); 
   $new_passwd2 = addslashes($_POST['new_passwd2']);
?>

<?php
// attempt to register
   $reg_result = register($user_name,$first_name,$last_name,$gender,$birth_month,$birth_day,
   $birth_year,$contact_number,$email_address,$street_address, $city_county, $state, $postal_code);
   if ($reg_result)
   {
     // register session variable 
     $valid_user = $user_name;
     $_SESSION['valid_user'] = $valid_user; // instead of session_register("valid_user");

     // provide link to members page
     do_html_header("Registration successful");
     echo "Your registration was successful.  Go to the members page "
          ."to start setting up your bookmarks!";
     do_HTML_URL("member_home.php", "Go to members page");
   }
   else
   {
     // otherwise provide link back, tell them to try again
     do_html_header("Problem:");
     echo $reg_result; 
     do_html_footer();
     exit;
   }
?>

<?php
function register($user_name,$first_name,$last_name,$gender,$birth_month,$birth_day,
   $birth_year,$contact_number,$email_address,$street_address, $city_county, $state, $postal_code)
// register new person with db
// return true or error message
{
// connect to db
  $conn = db_connect();
  if (!$conn)
    return "Could not connect to database server - please try later.";

  // check if username is unique 
  $result = mysql_query("select * from members_info where user_name='$user_name'"); 
  if (!$result)
     return "Could not execute query". mysql_error();
  if (mysql_num_rows($result)>0) 
     return "That username is taken - go back and choose another one.";

  // if ok, put in db
  $result = mysql_query("INSERT INTO members_info (members_id,user_name,first_name,last_name,gender,birth_month,birth_day,
birth_year,contact_number,email_address,register_date) VALUES(NULL,'$user_name','$first_name','$last_name'
,'$gender','$birth_month','$birth_day','$birth_year','$contact_number','$email_address',Now())");

  if ($result){
$member = mysql_insert_id();

$query2 = "INSERT INTO members_address (address_id, members_id, street_address, city_county, state, postal_code)
            VALUES(NULL,'$member','$street_address','$city_county','$state','$postal_code')";

$result2 = mysql_query($query2) or die ("Error in Query ". mysql_error());;
  }

  if (!$result || !$result2)
    return "Could not register you  in database - please try again later.";

  return true;
}
?>

boo_lolly · April 19, 2007

first of all, this entire block of code you've got here:

<?php
  // register_global = Off
  // inside the php.ini configuration file  
  // Registration Form
   $user_name = addslashes($_POST['user_name']);
   $email_address = addslashes($_POST['email_address']);
   $first_name  = addslashes($_POST['first_name']);
   $last_name  = addslashes($_POST['last_name']);
   $birth_month = addslashes($_POST['birth_month']);
   $birth_day = addslashes($_POST['birth_day']);
   $birth_year = addslashes($_POST['birth_year']);
   $gender = addslashes($_POST['gender']);

   $password = addslashes($_POST['password']);
   $cpassword = addslashes($_POST['cpassword']);
   
   $street_address = addslashes($_POST['street_address']);
   $city_county = addslashes($_POST['city_county']);
   $state = addslashes($_POST['state']);
   $postal_code  = addslashes($_POST['postal_code']);
   $contact_number = addslashes($_POST['contact_number']);
   
   $old_passwd = addslashes($_POST['old_passwd']);
   $new_passwd = addslashes($_POST['new_passwd']); 
   $new_passwd2 = addslashes($_POST['new_passwd2']);
?>

can be replaced with this:

<?php
        if(isset($_POST)){
                foreach($_POST as $key => $val){
                        $_POST[$key] = addslashes($val);
                }
        }
?>

no need to rename variables, just use them as they are in the post array. i'll look at the rest of your code in a little bit and get back to ya.

per1os · April 19, 2007

If you do want the variables to be named what the index is a modification to boo's code would work:

<?php
        if(isset($_POST)){
                foreach($_POST as $key => $val){
                        $$key = addslashes($val);
                }
        }
?>

Will create a new variable with the value.

Another side note, why are you using addslashes?

If it is because of the DB entry I would highly suggest taking a look at this:

The PHP directive magic_quotes_gpc is on by default, and it essentially runs addslashes() on all GET, POST, and COOKIE data. Do not use addslashes() on strings that have already been escaped with magic_quotes_gpc as you'll then do double escaping. The function get_magic_quotes_gpc() may come in handy for checking this.

http://us2.php.net/addslashes

Glyde · April 19, 2007

If it's for a DB, I'd do this:

<?php
        if (isset($_POST)){
                foreach($_POST as $key => $val){
                        $$key = mysql_real_escape_string(get_magic_quotes_gpc() ? stripslashes($val) : $val);
                }
        }
?>

Trium918 · April 19, 2007

If it's for a DB, I'd do this:

<?php
        if (isset($_POST)){
                foreach($_POST as $key => $val){
                        $$key = mysql_real_escape_string(get_magic_quotes_gpc() ? stripslashes($val) : $val);
                }
        }
?>

Yes, but I thought add going into the database and strip

when pulling out.

<?php
  if (isset($_POST)){
                foreach($_POST as $key => $val){
                        $$key = mysql_real_escape_string(get_magic_quotes_gpc() ? addslashes($val) : $val);
                }
        }

  for ($i = 0; $i < $num_results; $i++) {
  $row = mysql_fetch_array($result);
  
         $name = htmlspecialchars( stripslashes($row["name"]));
   	    // Output
	echo " <tr>
			<td>$name</td>

      }// End of For loop
?>

boo_lolly · April 19, 2007

If you do want the variables to be named what the index is a modification to boo's code would work:
<?php
        if(isset($_POST)){
                foreach($_POST as $key => $val){
                        $$key = addslashes($val);
                }
        }
?>
Will create a new variable with the value.

neat trick homeboy!

Glyde · April 19, 2007

First of all, mysql_real_escape_string will do the addslashes for you. Also, magic_quotes_gpc will add slashes to all $_GET,$_POST, and $_COOKIE data as stated before. Basically, my code:

1) Checks to see if magic_quotes_gpc is enabled, if it is, it strips the slashes

2) Takes the string with no slashes and runs it through mysql_real_escape_string

3) Slashes are added with this function

4) It is assigned to a variable

Here's an example

$_POST['test'] = "Some test string with \'slashes\'";

1) $_POST['test'] becomes "Some test string with 'slashes'";

2) $_POST['test'] becomes "Some test string with \'slashes\'"; (by mysql_real_escape_string)

3) $_POST['test'] becomes assigned to $test

MadTechie · April 19, 2007

as a note i am not sure if this is the correct section for this, as its not a "problem"

per1os · April 19, 2007

Yes, but I thought add going into the database and strip

when pulling out.

A very very common mistake. The rule of thumb is you should NEVER stripslashes on data coming out of a DB. That is why you do not want to double on the slashes. What happens is when the data is sent to the DB the original escaped slashes are removed, so the data in the DB should not have any escape characters when viewing which in return you should never have to stripslashes on data coming out of the database.

boo_lolly · April 19, 2007

Yes, but I thought add going into the database and strip

when pulling out.

<?php
  if (isset($_POST)){
                foreach($_POST as $key => $val){
                        $$key = mysql_real_escape_string(get_magic_quotes_gpc() ? addslashes($val) : $val);
                }
        }

  for ($i = 0; $i < $num_results; $i++) {
  $row = mysql_fetch_array($result);
  
         $name = htmlspecialchars( stripslashes($row["name"]));
   	    // Output
	echo " <tr>
			<td>$name</td>

      }// End of For loop
?>

add/stripslashes is old news. search google for 'php+sanitize' and you'll find what you need. there are all kinds of reasons to purify your user input values and all kinds of ways to do it. strip slashes and add slashes is a hassle. don't use it.

Trium918 · April 19, 2007

How can I improve the second two second

two script?

I implemented a html form and the php script handler.

The first script attempts to register the user by calling

the register() function. The register() register new person with db

or return an error.

<?php
// attempt to register
   $reg_result = register($user_name,$first_name,$last_name,$gender,$birth_month,$birth_day,
   $birth_year,$contact_number,$email_address,$street_address, $city_county, $state, $postal_code);
   if ($reg_result)
   {
     // register session variable 
     $valid_user = $user_name;
     $_SESSION['valid_user'] = $valid_user; // instead of session_register("valid_user");

     // provide link to members page
     do_html_header("Registration successful");
     echo "Your registration was successful.  Go to the members page "
          ."to start setting up your bookmarks!";
     do_HTML_URL("member_home.php", "Go to members page");
   }
   else
   {
     // otherwise provide link back, tell them to try again
     do_html_header("Problem:");
     echo $reg_result; 
     do_html_footer();
     exit;
   }
?>

<?php
function register($user_name,$first_name,$last_name,$gender,$birth_month,$birth_day,
   $birth_year,$contact_number,$email_address,$street_address, $city_county, $state, $postal_code)
// register new person with db
// return true or error message
{
// connect to db
  $conn = db_connect();
  if (!$conn)
    return "Could not connect to database server - please try later.";

  // check if username is unique 
  $result = mysql_query("select * from members_info where user_name='$user_name'"); 
  if (!$result)
     return "Could not execute query". mysql_error();
  if (mysql_num_rows($result)>0) 
     return "That username is taken - go back and choose another one.";

  // if ok, put in db
  $result = mysql_query("INSERT INTO members_info (members_id,user_name,first_name,last_name,gender,birth_month,birth_day,
birth_year,contact_number,email_address,register_date) VALUES(NULL,'$user_name','$first_name','$last_name'
,'$gender','$birth_month','$birth_day','$birth_year','$contact_number','$email_address',Now())");

  if ($result){
$member = mysql_insert_id();

$query2 = "INSERT INTO members_address (address_id, members_id, street_address, city_county, state, postal_code)
            VALUES(NULL,'$member','$street_address','$city_county','$state','$postal_code')";

$result2 = mysql_query($query2) or die ("Error in Query ". mysql_error());;
  }

  if (!$result || !$result2)
    return "Could not register you  in database - please try again later.";

  return true;
}
?>

MadTechie · April 19, 2007

Rules

2. Don't ask someone to write or re-write a script for you, unless you are posting a message to the PHPFreelancing Forum. The forums are not the place to request XYZ script. This is a community of people learning PHP, and not a script location service. Try searching sourceforge, phpclasses, hotscripts, or Google.

Trium918 · April 19, 2007

Rules

2. Don't ask someone to write or re-write a script for you, unless you are posting a message to the PHPFreelancing Forum. The forums are not the place to request XYZ script. This is a community of people learning PHP, and not a script location service. Try searching sourceforge, phpclasses, hotscripts, or Google.

lol, its funny! My title should have been Can you guys guide me to impove this.

I am a person who is learning also. I don't want no one to write a script for me, but

I am asking for you all to tell me wheather my code is secured or not. What I am missing.

phpbeginner · April 20, 2007

Ummm.....yeah, I find it funny at times myself. I think this should be renamed php experts as many are quick to state we are wanting them to create code for us. I am trying to learn also Trium918.....not a good place to ask newbie questions unless you want to be ridiculed.

Trium918 · April 20, 2007

Ummm.....yeah, I find it funny at times myself. I think this should be renamed php experts as many are quick to state we are wanting them to create code for us. I am trying to learn also Trium918.....not a good place to ask newbie questions unless you want to be ridiculed.

You got that right!! I just want someone to tell

me how I can improve it, not them.

Glyde · April 20, 2007

There's a difference between writing and optimizing code. Writing code implies you have nothing and are creating it from scratch. Optimizing it implies that the user already has the code structure and functionality, and wants it to work better. I would say that optimization belongs here, writing does not.

MadTechie · April 20, 2007

well i saw a post asking the same thing.. it didn't go far

see here

DanDaBeginner · April 20, 2007

sorry to interrupt guys.. BUT IS THIS TRUE?

A very very common mistake. The rule of thumb is you should NEVER stripslashes on data coming out of a DB. That is why you do not want to double on the slashes. What happens is when the data is sent to the DB the original escaped slashes are removed, so the data in the DB should not have any escape characters when viewing which in return you should never have to stripslashes on data coming out of the database.

I thought I was wrong with my code because no slashes saving with the data that I escape using mysql_real_escape_string....

thanx frost!

Trium918 · April 20, 2007

well i saw a post asking the same thing.. it didn't go far

see here

I am not asking for anyone to write any code for me.

I would you stop saying that. I just want to know what

I can do to make my code more secure and easier to

maintain.

roopurt18 · April 20, 2007

I just want to know what
I can do to make my code more secure and easier to

maintain.

Code optimization often entails undoing maintainability for a small portion of code.

Maintainability and security are two different topics. There are plenty of books you can purchase about PHP or MySQL security. Maintainability is a broader software design topic, so you'd want a book that deals with software design in general to learn more about that.

Trium918 · April 20, 2007

I just want to know what
I can do to make my code more secure and easier to

maintain.

Code optimization often entails undoing maintainability for a small portion of code.

Maintainability and security are two different topics. There are plenty of books you can purchase about PHP or MySQL security. Maintainability is a broader software design topic, so you'd want a book that deals with software design in general to learn more about that.

Thanks, but I think that this subject is some where

in space. All I wanted was for someone to tell me

how could I improve my code. Not for someone esle

to write it are purchasing a php book when I already

have 4.

Question: I want to know if my code is good or does

it need to be improved? Yes or No question

<?php
// attempt to register
   $reg_result = register($user_name,$first_name,$last_name,$gender,$birth_month,$birth_day,
   $birth_year,$contact_number,$email_address,$street_address, $city_county, $state, $postal_code);
   if ($reg_result)
   {
     // register session variable 
     $valid_user = $user_name;
     $_SESSION['valid_user'] = $valid_user; // instead of session_register("valid_user");

     // provide link to members page
     do_html_header("Registration successful");
     echo "Your registration was successful.  Go to the members page "
          ."to start setting up your bookmarks!";
     do_HTML_URL("member_home.php", "Go to members page");
   }
   else
   {
     // otherwise provide link back, tell them to try again
     do_html_header("Problem:");
     echo $reg_result; 
     do_html_footer();
     exit;
   }
?>

The register() function!!!

<?php
function register($user_name,$first_name,$last_name,$gender,$birth_month,$birth_day,
   $birth_year,$contact_number,$email_address,$street_address, $city_county, $state, $postal_code)
// register new person with db
// return true or error message
{
// connect to db
  $conn = db_connect();
  if (!$conn)
    return "Could not connect to database server - please try later.";

  // check if username is unique 
  $result = mysql_query("select * from members_info where user_name='$user_name'"); 
  if (!$result)
     return "Could not execute query". mysql_error();
  if (mysql_num_rows($result)>0) 
     return "That username is taken - go back and choose another one.";

  // if ok, put in db
  $result = mysql_query("INSERT INTO members_info (members_id,user_name,first_name,last_name,gender,birth_month,birth_day,
birth_year,contact_number,email_address,register_date) VALUES(NULL,'$user_name','$first_name','$last_name'
,'$gender','$birth_month','$birth_day','$birth_year','$contact_number','$email_address',Now())");

  if ($result){
$member = mysql_insert_id();

$query2 = "INSERT INTO members_address (address_id, members_id, street_address, city_county, state, postal_code)
            VALUES(NULL,'$member','$street_address','$city_county','$state','$postal_code')";

$result2 = mysql_query($query2) or die ("Error in Query ". mysql_error());;
  }

  if (!$result || !$result2)
    return "Could not register you  in database - please try again later.";

  return true;
}
?>

Sign In

Can you guys improve this?

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Important Information