FUDforum
Fast Uncompromising Discussions. FUDforum will get your users talking.

Home » General » PHP discussions » Script problem (Scritpt doesn't update database in email validation)
Show: Today's Messages :: Polls :: Message Navigator
Switch to threaded view of this topic Create a new topic Submit Reply
Script problem [message #165233] Fri, 13 May 2011 12:40 Go to next message
anemic-cinema is currently offline  anemic-cinema   France
Messages: 4
Registered: May 2011
Location: Paris
Karma: 0
Junior Member
Hi everybody, I'm a newbie to this forum. My name is Andrea and I'm Italian.
So, to get to the point i want to learn php and I'm trying to write an email validation script but I don't understand where I make a mistake.
I've already created a page (inscription.php) where someone can register himself and then an email is sent to his email box.
Register page creates and store in mysql database an activation key that it is also sent to the user
The Url included in the message calls another php page (verify.php) that compares the activation key with that stored in mysql and actives user account.
It seems to run but when i check the database, it isn't updated.
Could anyone suggest me the way to solve it?
Thank you very much and sorry if my English isn't good.

<?php
mysql_connect("localhost", "emailval", "emailval") or die(mysql_error());

mysql_select_db("emailval") or die(mysql_error());

$queryString = $_SERVER['QUERY_STRING'];

if(isset ($queryString)) {

    	$query = "SELECT * FROM membre"; 

	$result = mysql_query($query) or die(mysql_error());
	
	while($row = mysql_fetch_array($result)) {

	if ($queryString == $row["activationkey"]){

	echo "Congratulations!" . $row["login"] . 
	" is now the proud new owner of an TEST.com account.";

       	$sql="UPDATE membre SET activationkey = '', 
	status='activated' WHERE (id = $row[id])";

	} else {

         echo "error";

  	}

}

}
    
?>
Re: Script problem [message #165238 is a reply to message #165233] Sat, 14 May 2011 12:43 Go to previous messageGo to next message
naudefj is currently offline  naudefj   South Africa
Messages: 3771
Registered: December 2004
Karma: 28
Senior Member
Administrator
Core Developer
You never executed the UPDATE statement.

Add something like this after the $sql = ... line:
mysql_query($sql);

Re: Script problem [message #165239 is a reply to message #165238] Sat, 14 May 2011 13:07 Go to previous messageGo to next message
anemic-cinema is currently offline  anemic-cinema   France
Messages: 4
Registered: May 2011
Location: Paris
Karma: 0
Junior Member
Hi thank you, I found a solution this morning. Smile

Could you give me your opinion about the script? How can i improve it?

<?php
$base = mysql_connect ('localhost', 'emailval', 'emailval');

mysql_select_db ('emailval', $base);

$queryString = ($_GET['code']);
	 
$result = mysql_query("SELECT * FROM membre") or die(mysql_error());

$row = mysql_fetch_array($result);

	if  ($row['activationkey'] == $queryString) {

	echo "Congratulations!" . $row["login"] . "";


 	$sql="UPDATE membre SET  status='activated' WHERE (id = $row[id])"; 
	$req = mysql_query($sql) or die('Erreur SQL !'.$sql.'<br />'.mysql_error());

	

	
  	}  else {

    	die('Error: ' . mysql_error());

	}
?>


Thank you very much!
Re: Script problem [message #165240 is a reply to message #165239] Sun, 15 May 2011 16:50 Go to previous messageGo to next message
naudefj is currently offline  naudefj   South Africa
Messages: 3771
Registered: December 2004
Karma: 28
Senior Member
Administrator
Core Developer
Not bad, but you need to pay attention to the following:

1. Proper code indentation - your code is difficult to read.
2. Use consistent white space around concatenations - this look like a cut and paste job.
2. Use singe quotes where possible and appropriate - you switch between singe and double quotes without apparent reason.
4. Why append an empty string in: echo "Congratulations!" . $row["login"] . "";?
5. Don't store 'activated' in the DB (too long). Shorten it to STATUS='A' or even better, an a bit value of an integer.
6. You may want to close the DB connection when you are done.

Best of luck!
Re: Script problem [message #165241 is a reply to message #165240] Sun, 15 May 2011 20:23 Go to previous messageGo to next message
anemic-cinema is currently offline  anemic-cinema   France
Messages: 4
Registered: May 2011
Location: Paris
Karma: 0
Junior Member
Hi, thank you! I found your advices very helpful and I tried to follow them.
Now the script is something like that:

<?php

// Connect to database
$base = mysql_connect ('localhost', 'emailval', 'emailval');
mysql_select_db ('emailval', $base);

// Get Activation Key in the email URL
$queryString = ($_GET['code']);
$result = mysql_query('SELECT * FROM membre') or die(mysql_error());
$row = mysql_fetch_array($result);

        // Compare sent activationKey with stored activationKey
        if  ($row['activationkey'] == $queryString) {
                
                // Update databse
                $sql = "UPDATE membre SET  status='1' WHERE (id = $row[id])";
                $req = mysql_query($sql) or die('Erreur SQL !'.$sql.'<br />'.mysql_error());
                
                // Congratulations on registration
                echo "Congratulations!." . $row['login'] ." You are a new membre of TEST.COM ";
                
                // Close connection
                mysql_close ($base);
                include("index.php");
        } else {

        // Get an error
        die('Error: ' . mysql_error());
        }

?>




What do you think about it? Is it right?

Thank you very much!
Re: Script problem [message #165249 is a reply to message #165241] Tue, 17 May 2011 03:01 Go to previous messageGo to next message
Ernesto is currently offline  Ernesto   Sweden
Messages: 413
Registered: August 2005
Karma: 0
Senior Member
Now you are looping through you entire database until you find a match, it would make much mroe sense to not select anything at all.

First, you grab the $_GET, then you make sure it is clean.

$queryString = mysql_real_escape_string($_GET['code']);

(Read about SQL injections)

After that, you just
$sql = 'UPDATE membre SET status=1 WHERE activationkey = '.$queryString;


With your way you take ALL members from the database, loop though them in PHP until you find the match. That is not a good way, PHP is not made for looping that amount of data, however, that is JUST what an SQL server is made for, it can do it in a 100th of the time with a 100th of the effort. Imagine if you have a million users, it would take PHP days to loop it and it would take huge power to bring all the data from the database to the PHP layer.


Re: Script problem [message #165260 is a reply to message #165249] Tue, 17 May 2011 16:26 Go to previous message
anemic-cinema is currently offline  anemic-cinema   France
Messages: 4
Registered: May 2011
Location: Paris
Karma: 0
Junior Member
Hi Ernesto, thank you very much. Your advice was very useful.
  Switch to threaded view of this topic Create a new topic Submit Reply
Previous Topic: Usenet group import
Next Topic: Choose PHP Framework Tips
Goto Forum:
  

-=] Back to Top [=-
[ Syndicate this forum (XML) ] [ RSS ]

Current Time: Thu Mar 28 09:18:51 GMT 2024

Total time taken to generate the page: 0.03065 seconds