7 Replies - 366 Views - Last Post: 07 November 2013 - 06:01 PM Rate Topic: -----

#1 ILoveJava  Icon User is offline

  • D.I.C Regular

Reputation: 29
  • View blog
  • Posts: 389
  • Joined: 12-March 12

query executing several times

Posted 07 November 2013 - 09:44 AM

Hey guys. I'm trying to make a registration form, but i've run into a snag with my coding, I've gone over it many times, but I just can't figure out where exactly I've gone wrong. I want it to check the database for copies, and then submit the data only if it is original for name & email. Instead of doing this, it submits data every check it does (3 entries in databse, it'll enter this data 3 times.).

<!-- Registration.html -->
<html>
<head>
<title>User Registration</title>
</head>

<body>
		<form action="includes/process.php?x=user_reg" method="post" />
		<input name="username" type="text" value="Username" onfocus="if(this.value=='Username') this.value='';" onblur="if(this.value=='') this.value='Username';"/><br />
		<input name="password" type="password" value="Password" onfocus="if(this.value=='Password') this.value='';" onblur="if(this.value=='') this.value='Password';"/><br />
		<input name="name" type="text" value="Name" onfocus="if(this.value=='Name') this.value='';" onblur="if(this.value=='') this.value='Name';"/><br />
		<input name="age" type="text" value="Date of Birth" onfocus="if(this.value=='Date of Birth') this.value='';" onblur="if(this.value=='') this.value='Date of Birth';" /><br />
		<input name="email" type="text" value="Email" onfocus="if(this.value=='Email') this.value='';" onblur="if(this.value=='') this.value='Email';" /><br />
		<input type="hidden" name="level" value="1" />
		<input type="submit" />
		</form>
</body>
</html>


<?php
//process.php
	include("config.php");
	// checks to see if variable 'x' is set
	if (isset($_GET['x'])) {
		//x is set, $_GET[''] will grab what it's set to and switch accordingly
		$x = $_GET['x'];
			switch($x) {
				case "user_reg":
					// variablizes posted information
					$username = stripslashes($_POST['username']);
					$password = stripslashes($_POST['password']);
					$name = stripslashes($_POST['name']);
					$age = stripslashes($_POST['age']);
					$email = stripslashes($_POST['email']);
					$level = stripslashes($_POST['level']);
					
					$password = md5($password);

					while($row = mysql_fetch_array($users_result)) {
						$dbUsername = $row['username'];
						$dbEmail = $row['email'];
							if($dbUsername == $username /*|| $dbEmail == $email*/) {
								echo("This username/email has already been used.<br />");
							}
					mysql_query("INSERT INTO users(id, username, password, name, age, email, level) VALUES(null, '$username', '$password', '$name', '$age', '$email', '$level')") or die(mysql_error());
					echo("You have registered successfully, you can now login");
					}
				break;
			}
	}
?>


Is This A Good Question/Topic? 0
  • +

Replies To: query executing several times

#2 Peter O  Icon User is offline

  • D.I.C Head

Reputation: 77
  • View blog
  • Posts: 182
  • Joined: 19-October 13

Re: query executing several times

Posted 07 November 2013 - 09:56 AM

You have the the insert query inside the loop.
Was This Post Helpful? 0
  • +
  • -

#3 Dormilich  Icon User is offline

  • 痛覚残留
  • member icon

Reputation: 3541
  • View blog
  • Posts: 10,251
  • Joined: 08-June 10

Re: query executing several times

Posted 07 November 2013 - 10:04 AM

View PostILoveJava, on 07 November 2013 - 05:44 PM, said:

I want it to check the database for copies, and then submit the data only if it is original for name & email.

CREATE TABLE mytable
(
  name VARCHAR NOT NULL UNIQUE,
  email VARCHAR NOT NULL UNIQUE,
  -- and all the other fields
)


this way you simply cannot insert duplicate names and emails
Was This Post Helpful? 4
  • +
  • -

#4 astonecipher  Icon User is offline

  • Major DIC Head
  • member icon

Reputation: 704
  • View blog
  • Posts: 3,027
  • Joined: 03-December 12

Re: query executing several times

Posted 07 November 2013 - 10:18 AM

This section of code is your problem:

20	                    while($row = mysql_fetch_array($users_result)) {
21	                        $dbUsername = $row['username'];
22	                        $dbEmail = $row['email'];
23	                            if($dbUsername == $username /*|| $dbEmail == $email*/) {
24	                                echo("This username/email has already been used.<br />");
25	                            }
26	                    mysql_query("INSERT INTO users(id, username, password, name, age, email, level) VALUES(null, '$username', '$password', '$name', '$age', '$email', '$level')") or die(mysql_error());
27	                    echo("You have registered successfully, you can now login");
28	                    }




Go with Dormilich's table so you know you will have an error on insert, code to prevent it but,just as a backup set those fields as unique. And why do you have a switch statement with one case?
Was This Post Helpful? 1
  • +
  • -

#5 Atli  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 3719
  • View blog
  • Posts: 5,991
  • Joined: 08-June 10

Re: query executing several times

Posted 07 November 2013 - 11:14 AM

A few additional pointers on your code:

  • MD5 should not be used for password hashing. It's old and weak and easily broken. Especially unsalted. - Use the new password_hash function instead. If you aren't using PHP 5.5 yet, you can use this library to add support for those functions in 5.4 and 5.3. - If you are using 5.2 or lower... upgrade.


  • You really shouldn't be using the stripslashes function on the user input like that. If you want to know why, read up on the deprecated magic_quotes feature. The stripslashes function was used a lot years ago to undo the effects of that feature, but it's no longer used, and therefore the function should no longer be used. - If your server actually has magic_quotes enabled, you really really need to switch hosts.

  • Avoid this kind of switch layout for your code, where you use switches to execute the correct functionality based on a variable, and then put all the actual code into the case. Trust me when I tell you, that will end up looking like crap, and making your code horrible to maintain. (I'm actually working on maintaining/replacing a project set up like this, so I know what I'm talking about!)

    If you really want to use switches, at least put each case block into a function, to make things a little less chaotic and difficult to manage.

Was This Post Helpful? 1
  • +
  • -

#6 ILoveJava  Icon User is offline

  • D.I.C Regular

Reputation: 29
  • View blog
  • Posts: 389
  • Joined: 12-March 12

Re: query executing several times

Posted 07 November 2013 - 05:20 PM

View PostDormilich, on 08 November 2013 - 01:04 AM, said:

View PostILoveJava, on 07 November 2013 - 05:44 PM, said:

I want it to check the database for copies, and then submit the data only if it is original for name & email.

CREATE TABLE mytable
(
  name VARCHAR NOT NULL UNIQUE,
  email VARCHAR NOT NULL UNIQUE,
  -- and all the other fields
)


this way you simply cannot insert duplicate names and emails

I never thought of setting them as unique. Thank you!

View Postastonecipher, on 08 November 2013 - 01:18 AM, said:

This section of code is your problem:

20	                    while($row = mysql_fetch_array($users_result)) {
21	                        $dbUsername = $row['username'];
22	                        $dbEmail = $row['email'];
23	                            if($dbUsername == $username /*|| $dbEmail == $email*/) {
24	                                echo("This username/email has already been used.<br />");
25	                            }
26	                    mysql_query("INSERT INTO users(id, username, password, name, age, email, level) VALUES(null, '$username', '$password', '$name', '$age', '$email', '$level')") or die(mysql_error());
27	                    echo("You have registered successfully, you can now login");
28	                    }




Go with Dormilich's table so you know you will have an error on insert, code to prevent it but,just as a backup set those fields as unique. And why do you have a switch statement with one case?

I have it set as one case because I plan to use it for more as time goes on.

View PostAtli, on 08 November 2013 - 02:14 AM, said:

A few additional pointers on your code:

  • MD5 should not be used for password hashing. It's old and weak and easily broken. Especially unsalted. - Use the new password_hash function instead. If you aren't using PHP 5.5 yet, you can use this library to add support for those functions in 5.4 and 5.3. - If you are using 5.2 or lower... upgrade.


  • You really shouldn't be using the stripslashes function on the user input like that. If you want to know why, read up on the deprecated magic_quotes feature. The stripslashes function was used a lot years ago to undo the effects of that feature, but it's no longer used, and therefore the function should no longer be used. - If your server actually has magic_quotes enabled, you really really need to switch hosts.

  • Avoid this kind of switch layout for your code, where you use switches to execute the correct functionality based on a variable, and then put all the actual code into the case. Trust me when I tell you, that will end up looking like crap, and making your code horrible to maintain. (I'm actually working on maintaining/replacing a project set up like this, so I know what I'm talking about!)

    If you really want to use switches, at least put each case block into a function, to make things a little less chaotic and difficult to manage.

Thank you for all your suggestions, and the reason I'm using "stripslashes();" is because when I started learning PHP (around 7 years ago) it was pretty much pushed onto me to use it when cleaning up data for entering, along with mysql_real_escape_string.
Was This Post Helpful? 0
  • +
  • -

#7 Atli  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 3719
  • View blog
  • Posts: 5,991
  • Joined: 08-June 10

Re: query executing several times

Posted 07 November 2013 - 05:46 PM

View PostILoveJava, on 08 November 2013 - 12:20 AM, said:

Thank you for all your suggestions, and the reason I'm using "stripslashes();" is because when I started learning PHP (around 7 years ago) it was pretty much pushed onto me to use it when cleaning up data for entering, along with mysql_real_escape_string.

I understand, it was a similar situation when I was first using it. However 7 years is a very long time in the world of software development. The practice of escaping user input and injecting it into SQL queries is obsolete now, and the magic_quotes feature has been removed, so neither function is relevant anymore.

The old MySQL API functions - like mysql_query and mysql_real_escape_string - are deprecated, and should be replaced by either PDO or MySQLi. Both of those support parameterized queries, which negates the possibility of SQL injection completely, removing the need for escaping the input.
Was This Post Helpful? 0
  • +
  • -

#8 ILoveJava  Icon User is offline

  • D.I.C Regular

Reputation: 29
  • View blog
  • Posts: 389
  • Joined: 12-March 12

Re: query executing several times

Posted 07 November 2013 - 06:01 PM

Fair enough. It's been a very long time since I've actually gotten hard into coding, and the site I used to use to learn and reference (tizag.com) is now so outdated that I can't use it.
Time to re-learn everything I once knew decently well. /cry.
Was This Post Helpful? 0
  • +
  • -

Page 1 of 1