Insert data PHP/PDO for user registration

Question

Im trying to create a user registration page using PDO, I've never worked with this before so im having trouble understanding how the values are inserted into my table.

Can anybody see whats going wrong with my code?

<?php

include_once ('/_includes/classes/connection.class.php');

$firstname = $_POST['firstname'];
$lastname = $_POST['lastname'];
$email = $_POST['email'];
$password = $_POST['password'];
$accounttype = $_POST['accounttype'];

$query = "INSERT INTO users(firstname,lastname,email,password,accounttype) VALUES ($firstname,$lastname,$email,$password,$accounttype)";
echo $query;
$count = $dbh->exec($query);
$dbh = null;

?>

<?php

$dsn = 'mysql:host=localhost;dbname=site.co.uk';
$username = 'access@site.co.uk';
$password = 'password';
$options = array(
    PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8',
); 
$dbh = new PDO($dsn, $username, $password, $options);

$firstname = $_POST['firstname'];
$lastname = $_POST['lastname'];
$email = $_POST['email'];
$userpassword = $_POST['password'];
$accounttype = $_POST['accounttype'];

$query = "INSERT INTO users(firstname,lastname,email,password,accounttype) VALUES (:firstname,:lastname,:email,:password,:accounttype)";
$stmt = $dbh->prepare($query);
$stmt->bindParam(':firstname', $firstname);
$stmt->bindParam(':lastname', $lastname);
$stmt->bindParam(':email', $email);
$stmt->bindParam(':password', $userpassword);
$stmt->bindParam(':accounttype', $accounttype);
$stmt->execute();

?>

You may want to think about having a primary key if you're going to have a database of different users. — sooper, Jan 19 '13 at 18:49

score 2 · Answer 1 · edited May 23 '17 at 10:34

Never do it the way you didt, cause of SQL Injections

Use prepared statements.

http://php.net/manual/de/pdo.prepared-statements.php

<?php

require_once ('_includes/classes/connection.class.php');

$stmt = $dbh->prepare('INSERT INTO users (firstname,lastname,email,password,accounttype) VALUES (:firstname,:lastname,:email,:password,:accounttype)');
$stmt->execute(array($_POST));

What is going wrong is, that you forget the quotes for your values. But in case of prepared statements you dont need quotes.

http://www.w3schools.com/sql/sql_insert.asp

And please dont save plain text passwords into the database, use hashes

Secure hash and salt for PHP passwords

Better use "require" or "require_once" if you realy need the includet file.

http://php.net/manual/en/function.require.php

Your Common Sense · Answer 2 · 2013-01-19T19:00:39.907

2

include_once ('/_includes/classes/connection.class.php'); will never include anything. There are no _includes directory in the root of the local filesystem
As you didn't mention any errors - so, you don't have proper error reporting set.

Can anybody see whats going wrong with my code?

There can be other errors but watching code is not the way to go. One have to run the code, to debug it, and watch the errors occurred.

the only thing I have to add as a side note - your code is as WET as a flood. Look at it: you're writing every field name SIX times!

$firstname = $_POST['firstname']; - 2x
(firstname) VALUES (:firstname)` - 2x
bindParam(':firstname', $firstname); - 2x

with total of 6 repetitions

edited Jan 19 '13 at 19:00

answered Jan 19 '13 at 18:48

Your Common Sense

152,517
33
193
313

What do you mean by every field names 5 times @YourCommonSense? – Liam Jan 19 '13 at 18:58
I dont understand why that's a bad thing though? Is there something else I could be doing which is more beneficial? Sorry if I sound naive im just new to this and struggling to understand certain problems etc... – Liam Jan 19 '13 at 19:06

score 1 · Answer 3 · answered Jan 19 '13 at 18:47

1

You need to bind the values rather than use string concatenation.

  $dsn = 'mysql:host=localhost;dbname=mydb';
  $username = 'myun';
  $password = 'mypw';
  $options = array(
      PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8',
  ); 
  $dbh = new PDO($dsn, $username, $password, $options);
  $query = "INSERT INTO users(firstname,lastname,email,password,accounttype) VALUES
        (:firstname,:lastname,:email,:password,:accounttype)";
  $stmt = $dbh->prepare($query);
  $stmt->bindParam(':firstname', $firstname);
  $stmt->bindParam(':lastname', $lastname);
  //etc
  $stmt->execute();

answered Jan 19 '13 at 18:47

Vincent Ramdhanie

98,815
22
134
183

This doesn't seem to work @VincentRamdhanie, I've updated my question with my complete code... – Liam Jan 19 '13 at 18:56
When you say "doesn't work" in what way do you mean? What is teh error message? – Vincent Ramdhanie Jan 21 '13 at 03:16

score 0 · Answer 4 · answered Jan 19 '13 at 19:08

You should really use prepared statements. You should do something more like this, assuming:

$dbh is a PDO object
You've checked the $_POST array to ensure it contains all the fields you think it should
You fix your include statement
You choose to save hashed passwords in your database instead of storing them plaintext

Your code would be more like this.

<?php

error_reporting(E_ALL);

include_once ('_includes/classes/connection.class.php');

$_POST['password'] = hash('md5', $_POST['password']);

$statement = $dbh->prepare("INSERT INTO 
    users(firstname,lastname,email,password,accounttype) 
    VALUES (:firstname, :lastname, :email, :password, :accounttype)");
if ($statement->execute($_POST) !== true) {
    // there was some kind of error
    // perhaps $statement->errorInfo() will tell you something
}

Insert data PHP/PDO for user registration

4 Answers4