PHP/MySQL Form Problem

Discussion in 'Web Design and Development' started by jordanste, Jan 13, 2008.

  1. jordanste macrumors member

    Joined:
    Feb 25, 2006
    #1
    I am trying to create a simple mailing list signup form from scratch using php and MySQL. Essentially, users provide their e-mail address (along with other information), and then PHP sends the info into a MySQL database, so i can send a mailer based on the information gathered. Sounds simple enough right? well im a touch new to PHP and i am having this problem where everytime the page that the forms are on loads, the MySQL database receives an empty row. in other words, the database is receiving the "data" from a bunch of empty forms everytime the page loads. and then the database receives real data when a user clicks the submit button. SO i end up with real data interspersed with a bunch of empty rows. Can anyone take a look at my code, and tell me if they can figure out why this is happening?

    im assuming i made a stupid n00b mistake. as this doesnt seem to be an overwhelming problem for many php developers.

    all help sincerely appreciated.

    Here is the HTML code for the form itself, incase the problem is in the form... i dont know.

    HTML:
    <p>
    <table cellspacing=2 cellpadding=0 border=0>
    	<tr>
    		<td>
    		<form action='mailinglist.php?register=yes' method=POST>
    		First Name:
    		</td>
    		<td>
    		<input type=text name='firstname' size=20 maxlength=30>
    		</td>
    	</tr>
    	<tr>
    		<td>
    		Last Name:
    		</td>
    		<td>
    		<input type=text name='lastname' size=20 maxlength=30>
    		</td>
    	</tr>
    	<tr>
    		<td>
    		City, State:
    		</td>
    		<td>
    		<input type=text name='city' size=15 maxlength=30>, <input type=text name='state' size=2 maxlength=2>
    		</td>
    	</tr>
    	<tr>
    		<td>
    		E-mail:
    		</td>
    		<td>
    		<input type=text name='email' size=20 maxlength=30>
    		</td>
    	</tr>
    	<tr>
    		<td>
    		<input type=submit value="Register!">
    		</form>				
    		</td>
    	</tr>
    </table>
    </p>
    Here is the PHP i am using to send the data gathered in the forms to the MySQL database.

    PHP:
    <?php

        $firstname
    =$_POST['firstname'];
        
    $lastname=$_POST['lastname'];
        
    $city=$_POST['city'];
        
    $state=$_POST['state'];
        
    $email=$_POST['email'];
        
    $register=$_GET['register'];

            
    $con=mysql_connect('localhost','root','root') or die ('Error connecting to mysql');
            
    mysql_select_db('mailinglist');
            
            
    $query="INSERT INTO mailinglist (id,firstname,lastname,city,state,email) VALUES (NULL , '$firstname', '$lastname', '$city', '$state', '$email')";
            
    mysql_query($query) or die('Error, insert query failed');

            
    mysql_close($con);
            
        if (
    $register=='yes') echo "Thank you for registering $firstname $lastname!"
    ?>
    Here is what the reslting table looks like in MySQL... all fields are empty except for "id" which cant be null.

    [​IMG]
     
  2. Nugget macrumors 65816

    Nugget

    Joined:
    Nov 24, 2002
    Location:
    Houston Texas USA
    #2
    I see two problems, one small and one huge:

    First, you should only be performing the insert if the user has supplied data with the form. This could be crudely accomplished by moving the inert logic into the conditional block of your if test. Something like:

    Code:
    if ($register=='yes') {
       $con=mysql_connect('localhost','root','root')
       . . .
       etc
       . . .
       echo "Thanks for registering"
    }
    
    Secondly, your code is at risk of sql injection attacks and needs to be validating and sanitizing the data supplied by the user.

    Imagine if someone registers and sets their last name to "'; delete from mailinglist; drop database mailinglist;". Your database is toast.
     
  3. bbarnhart macrumors 6502a

    bbarnhart

    Joined:
    Jan 16, 2002
    Location:
    Stilwell, Kansas
  4. jordanste thread starter macrumors member

    Joined:
    Feb 25, 2006
    #4
    thankyou for helping me out by taking time out of your day to shoot some advice to someone who isnt very code oriented.
     
  5. Nugget macrumors 65816

    Nugget

    Joined:
    Nov 24, 2002
    Location:
    Houston Texas USA
    #5
    These details can be arcane and non-obvious to people who don't grind code for a living.
     
  6. Thom_Edwards macrumors regular

    Joined:
    Apr 11, 2003
    #6
    At the beginning of your php code, I noticed that you used $_POST for all of your form variables when setting up the php variables except for 'register' where you use $_GET. I don't think that is what you mean to do.

    Also, if you have register_globals ON, you can open yourself up for some other problems if you have your php variables named the same as your form variables. Consider this code:
    PHP:
    <?php
    print $r" is the value";
    ?>

    <html>
    <body>
    <form action="" method="post">
    <input type="hidden" name="r" value="hello" />
    <input type="submit" />
    </form>
    </body>
    </html>
    If I add ?r=goodbye to the end of the URL, $r suddenly equals 'goodbye' in the php code on the first run through even though the $_POST['r'] value is ''. You do set the variables explicitly, so you're pretty OK. But if you forget one of them, you can get into a world of hurt. I usually do something like $mEmail = $_POST['email'] and then use $mEmail throughout the php code. Take a quick peek at http://www.php.net/manual/en/security.globals.php for more information.

    You said you weren't that code-oriented, so I thought I would drop a few tips. Hope this helps, and good luck!
     
  7. jordanste thread starter macrumors member

    Joined:
    Feb 25, 2006
    #7
    awesome, thanks. while security isdefinitely important to me, as it should be to any developer, i was really preoccupied with making sure it worked, before making sure it was secure. so thanks for answering my next question.

    thanks for your help everyone.
     

Share This Page