HELP PLEASE !!

Cio22

Ok Just to set the record, i am fairly new to php so my skills dont compare to those of yours but that being said ....

I have a signup form split into two pages. On my second page, the user has the option to enter an email and upload a picture but when the code runs, it says success but the information doesnt show up in my database. I dont know what im doing wrong.
Heres my code:

<?php
session_start();

if ($_SESSION['login'] == "1"){

//if (!(isset($_SESSION['login']) && $_SESSION['login'] != '1')) {
//	header ("Location: index.php");
//}




if ($_SERVER['REQUEST_METHOD'] == 'POST'){

  $email=$_POST['email'];       
  $target = "pictures/";       
  $target = $target . basename( $_FILES['upload']['name']);       
  $pic= $_FILES  ['upload']['tmp_name'];       
  $uname=$_SESSION['username'];

  if($email = ''){
      $errorMessage = $errorMessage . "You must enter an Email." . "<BR>";
    }
    else {
          $errorMessage = "";
    }

   if(($_FILES["upload"]["type"] == "image/gif")
     || ($_FILES["upload"]["type"] == "image/jpeg")
     || ($_FILES["upload"]["type"] == "image/jpg")
     || ($_FILES["upload"]["type"] == "image/png" )
     && ($_FILES["upload"]["size"] < 25600)){
            $errorMessage ="";
      }
    else{
           $errorMessage = $errorMessage . "Picture must be a jpeg, gif or png file!" . "<BR>";
       }


                 if ($errorMessage == "") {


              $user_name = "user";
              $pass_word = "pass";
              $database = "a6918756_test";
              $server = "host";


              $db_handle = mysql_connect($server, $user_name, $pass_word);
              $db_found = mysql_select_db($database, $db_handle);


                                if ($db_found){

                                   $SQL = "UPDATE Accounts SET Email='$email', Picture='$pic' WHERE Username='$uname'";

                              	   $result = mysql_query($SQL);

                                       echo "Success";

                                  mysql_close($db_handle);
                                      }       

                                  else {
                                            echo $errorMessage;
                                         }
                 }
      }
}

else{
    header ("Location: index.php");
      }

?>

<html>
<body>
<center>
<FORM  NAME='form1' METHOD ='POST' ACTION ='test2.php'>
<BR>
Email:
<input type='TEXT' Value='' Name='email'>
<BR>
<BR>
<input type='File' Value='Browse' Name='upload'>
<BR>
<BR>
<input TYPE = 'Submit' Name = 'Submit'  VALUE = 'Submit'>
</center>
</FORM>
</body>
</html>

titan21

First things first - can you check your error logs to see if it is giving you any information?

bradgrafelman

Welcome to PHPBuilder!

Here are some issues I see with your code:

First, a general comment about all of the code: Find an indentation/whitespace style that you like and stick to it! Properly formatted code is much easier to read and analyze.

R an doml y spa c edcode , on th e other han d, i
s muc h ha
rder to re ad.
```
if ($_SESSION['login'] == "1"){ 

//if (!(isset($_SESSION['login']) && $_SESSION['login'] != '1')) { 
//    header ("Location: index.php"); 
//} 
```
Note that the commented out version of the if() statement is actually closer to what you should be using than the first is.

If $_SESSION['login'] is undefined, the first if() statement is going to generate an E_NOTICE level error message. However, note that the second if() statement should have been written with a logical OR rather than an AND in order for it to make sense (how can something not be set and have a value?).

If you wanted to keep the header() statement in the 'else' branch below, note you could simply invert the logical expression in the if() statement.
```
if ($_SERVER['REQUEST_METHOD'] == 'POST'){ 
```
Just because you know the user sent a POST request doesn't mean you can assume all of the fields you're expecting to use were actually contained in the POST'ed data. For that reason, I always define variables holding external data like so:
```
$email = isset($_POST['email']) ? $_POST['email'] : '';
```
This uses the ternary operator to set a default value (the empty string, here - could be any default value that is appropriate for the type of data you're expecting) as well as perhaps doing some general filtering of the incoming data (e.g. you could wrap the second $_POST['email'] with something like [man]intval/man/etc. if you were expecting numeric data).

      $target = "pictures/";        

      $target = $target . basename( $_FILES['upload']['name']);        

      $pic= $_FILES  ['upload']['tmp_name'];

Same comment here as above - you're assuming that $_FILES['upload'] exists; what if it doesn't?

```
if($email = ''){
```
Note here that you're assigning the empty string to the variable $email rather than comparing the two with each other.
```
$errorMessage = $errorMessage . "You must enter an Email." . "<BR>"; 
```
$errorMessage wasn't defined before this line, so attempting to append that error message to itself means you're trying to append a string to a variable that doesn't exist, which will generate an error message.

Speaking of error messages, you should have error_reporting set to E_ALL and either display_errors or log_errors enabled depending whether we're talking about your development or production environment, respectively. (And if you do, it would help us help you if you pasted all of the PHP error messages you're receiving, since you should at least be getting the one mentioned above.)
```
       if(($_FILES["upload"]["type"] == "image/gif") 
         || ($_FILES["upload"]["type"] == "image/jpeg") 
         || ($_FILES["upload"]["type"] == "image/jpg") 
         || ($_FILES["upload"]["type"] == "image/png" ) 
         && ($_FILES["upload"]["size"] < 25600))
```
Note the warning on the PHP manual page [man]features.file-upload.post-method[/man] in regards to the 'type' index:

PHP Manual wrote:
This mime type is however not checked on the PHP side and therefore don't take its value for granted.
In other words, I could easily upload 'my_favorite_virus.exe' and simply lie to your script and claim it is of type "image/gif" rather than the malicious executable that it actually is.

If you really want to check if you've received a valid image, consider using [man]getimagesize[/man] or perhaps the [man]Fileinfo[/man] library of functions.
```
       if(($_FILES["upload"]["type"] == "image/gif") 
         || ($_FILES["upload"]["type"] == "image/jpeg") 
         || ($_FILES["upload"]["type"] == "image/jpg") 
         || ($_FILES["upload"]["type"] == "image/png" ) 
         && ($_FILES["upload"]["size"] < 25600)){ 
                $errorMessage =""; 
          } 
```
Here, you're clearing out the $errorMessage variable... but what if the e-mail was left blank and an error was generated above? If so, you've just lost that error message without ever handling it.

Instead, initialize $errorMessage earlier in the script and only append values to it if an error actually occurs; if there is no error at a particular step, do nothing with the variable's contents.
Speaking of appending strings, note that it's easier to use the ".=" concatenation operator, e.g.:
```
$string .= 'This string is being appended to $string\'s previous value.';
```

                  $db_handle = mysql_connect($server, $user_name, $pass_word); 
                  $db_found = mysql_select_db($database, $db_handle); 


                                if ($db_found){

Note that if you truly want this code to be error-resistant, you might also want to first check to see if [man]mysql_connect[/man]() succeeded before you attempt to select a database.

```
$SQL = "UPDATE Accounts SET Email='$email', Picture='$pic' WHERE Username='$uname'"; 
```
User-supplied data should never be placed directly into a SQL query string, else your code will be vulnerable to SQL injection attacks and/or just plain SQL errors. Instead, you must first sanitize the data with a function such as [man]mysql_real_escape_string/man (for string data) or use prepared statements. See this manual page for more info: [man]security.database.sql-injection[/man].
```
                                         $result = mysql_query($SQL);

                                       echo "Success"; 
```
Why do you echo "Success" without checking to see if the SQL query was actually successful or not?

For one, you should always check to see if [man]mysql_query/man returned boolean FALSE (indicating an error has occurred) and, if so, outputting or logging (again, depending upon which environment you're working in) the error message returned by the MySQL server (see [man]mysql_error/man) and perhaps the SQL query itself (to aid in debugging).
```
                                      else { 
                                                echo $errorMessage; 
                                             }
```
This echo statement will never produce any output. In addition, if $errorMessage actually contains an error message, your code will never output it.

Why? Note that the above 'else' statement matches up with the if ($db_found) statement which is wrapped inside of:
```
if ($errorMessage == "") { 
```
Perhaps you meant for the 'else' statement to be paired with this latter if() statement?

Note that this problem would have been easily detected (or perhaps never would have occurred in the first place) if you had followed the advice I gave in #1 above. 😉

<html> 
<body> 
<center> 
<FORM  NAME='form1' METHOD ='POST' ACTION ='test2.php'> 
<BR> 
Email: 
<input type='TEXT' Value='' Name='email'> 
<BR> 
<BR> 
<input type='File' Value='Browse' Name='upload'> 
<BR> 
<BR> 
<input TYPE = 'Submit' Name = 'Submit'  VALUE = 'Submit'> 
</center> 
</FORM> 
</body> 
</html>

Yikes... once you fix the above errors, you might want to start working on writing valid HTML markup.

Derokorian

bradgrafelman wrote:
R an doml y spa c edcode , on th e other han d, i
s muc h ha
rder to re ad.

This took me a good 2 minutes to read.

titan21

Derokorian;10991012 wrote:
This took me a good 2 minutes to read.

Just that bit or the whole post?? 🙂

Cio22

lol thank you very much , i really appreciate you taking your time to respond. Ive been trying to teach myself the basics of php but its really hard when you have no one guiding you. should a signup script normally be one page or does it create a security issue when spread out through different pages?

bradgrafelman

Cio22;10991181 wrote:
should a signup script normally be one page

What's "normal" ? Even better, who cares? 😉 The number of "pages" a sign-up process takes is at best nothing more than a display issue/preference, so do whatever you feel best fits your application's needs.

Cio22;10991181 wrote:
or does it create a security issue when spread out through different pages?

Not sure what the question is... you're talking about page layout and security of (data?), but the two topics seem orthogonal to me.

If you're talking about the storage of data when moving from one page to another, and whether or not that data can be altered, then that's a different matter altogether (one that could probably be avoided simply by using [man]sessions[/man] rather that cookies or whatnot).

Cio22

thats the code for the first page of the signup , maybe this will help

<?PHP
session_start();
//if (!(isset($_SESSION['login']) && $_SESSION['login'] != '')) {
	//header ("Location: login3.php");
//}

$uname = "";
$pword = "";
$errorMessage = "";
$num_rows = 0;

function quote_smart($value, $handle) {

   if (get_magic_quotes_gpc()) {
       $value = stripslashes($value);
   }

   if (!is_numeric($value)) {
       $value = "'" . mysql_real_escape_string($value, $handle) . "'";
   }
   return $value;
}

if ($_SERVER['REQUEST_METHOD'] == 'POST'){

//====================================================================
//	GET THE CHOSEN U AND P, AND CHECK IT FOR DANGEROUS CHARCTERS
//====================================================================
$uname = $_POST['username'];
$pword = $_POST['password'];
$email = $_POST['email'];


$uname = htmlspecialchars($uname);
$pword = htmlspecialchars($pword);

//====================================================================
//	CHECK TO SEE IF U AND P ARE OF THE CORRECT LENGTH
//	A MALICIOUS USER MIGHT TRY TO PASS A STRING THAT IS TOO LONG
//	if no errors occur, then $errorMessage will be blank
//====================================================================


$uLength = strlen($uname);
$pLength = strlen($pword);



if ($uLength >= 10 && $uLength <= 20) {
	$errorMessage = "";
}
else {
	$errorMessage = $errorMessage . "Username must be between 10 and 20 characters" . "<BR>";
}

if ($pLength >= 8 && $pLength <= 16) {
	$errorMessage = "";
}
else {
	$errorMessage = $errorMessage . "Password must be between 8 and 16 characters" . "<BR>";
}

 if ($_POST['password'] == $_POST['confirm']){		
            $errorMessage = "";	
    }	
    else {		 
            $errorMessage= $errorMessage . "Passwords do NOT match"."<BR>";	
    }	


//test to see if $errorMessage is blank
//if it is, then we can go ahead with the rest of the code
//if it's not, we can display the error

//====================================================================
//	Write to the database
//====================================================================
if ($errorMessage == "") {

$user_name = "a6918756_test";
$pass_word = "manual24";
$database = "a6918756_test";
$server = "mysql17.**********.com";

$db_handle = mysql_connect($server, $user_name, $pass_word);
$db_found = mysql_select_db($database, $db_handle);

if ($db_found) {

	$uname = quote_smart($uname, $db_handle);
	$pword = quote_smart($pword, $db_handle);

//====================================================================
//	CHECK THAT THE USERNAME IS NOT TAKEN
//====================================================================

	$SQL = "SELECT * FROM Accounts WHERE Username = $uname";
	$result = mysql_query($SQL);
	$num_rows = mysql_num_rows($result);

	if ($num_rows > 0) {
		$errorMessage = "Username already taken";
	}

	else {

		$SQL = "INSERT INTO Accounts (Username, Password) VALUES ($uname, md5($pword))";

		$result = mysql_query($SQL);

		mysql_close($db_handle);

	//=================================================================================
	//	START THE SESSION AND PUT SOMETHING INTO THE SESSION VARIABLE CALLED login
	//	SEND USER TO A DIFFERENT PAGE AFTER SIGN UP
	//=================================================================================


		session_start();
                    $_SESSION['username'] =$_POST['username'];
		$_SESSION['login'] = "1";

		header ("Location: test2.php");

	}

}
else {
	$errorMessage = "Database Not Found";
}




}
    else{
      header( 'refresh: 3; url=http://kiqspot.com/test.php' );
     }
}

?>


<html>
	<head>
	<title>Join KiQSpoT</title>


</head>
<body>


<FORM NAME ="form1" METHOD ="POST" ACTION ="test.php">
<center>
Username:
<BR>
<INPUT TYPE = 'TEXT' Name ='username'  value="" maxlength="20">
<BR>
<BR>
Password:
<BR> 
<INPUT TYPE = 'Password' Name ='password'  value="" maxlength="16">
<BR>
<BR>
Confirm Password:
<BR>
<INPUT TYPE='Password' name='confirm' value="" maxlength="16">
<BR>
<BR>
<input type="reset" name='reset'/>
<INPUT TYPE = "Submit" Name = "Submit"  VALUE = "Register">
</center>
</FORM>
<P>

<?PHP print $errorMessage; ?>

</body>
</html>

Cio22

Yea the storage of data , the problem i often get is the script will run with no errors but the data submitted isnt saved into sql. or the file uploaded isnt moved into my directory. i tried having all forms on one page but the data wasnt being stored so i decided to split them up . the first page was working fine but the second one give me problems.