<?php
public static function addUser($username, $email, $password)
{
try
{
$db = Database::getInstance();
$sql = sprintf("INSERT INTO `users` (`name`, `email`, `password`, `date`, `role_id`, `active`, `new`)
VALUES ('%s', '%s', '%s', NOW(), %d, %d, %d)", $db->real_escape_string($username), $db->real_escape_string($email), md5($password), _ROLE_USER, 0, 1);
$resultset = $db->query($sql);
if (!$resultset) // Database error
{
throw new Exception(i18n::t("MySQL error").": ".$db->error, _EXCEPTION_ERROR);
}
else
{
return true;
}
}
catch (Exception $e)
{
Messages::registerMessage($e->getMessage(), $e->getCode());
return false;
}
}
?>
Refactorings
No refactoring yet !
Tj Holowaychuk
July 26, 2009, July 26, 2009 20:10, permalink
No rating.
Login to rate!
Ew, lol use some kind of API that escapes for you... really lame to call real_escape_string on everything. I wouldnt even have the try{} block, should probably handle that above. if you used some sort of ORM, or made one quickly, and used LLP it could be as simple as:
<?llp
---
LLP is Less-Lame-PHP, not finished yet though:
http://github.com/visionmedia/llp/tree/master
---
- addUser(username, email, password) {
User.create([
name : username,
email : email,
password : password.to_hash
date : ...
])
}
gordon0580.myopenid.com
September 21, 2009, September 21, 2009 04:06, permalink
No rating.
Login to rate!
In cakePHP, you can do this.
<?php
class User extends AppModel {
var $name = 'User';
}
?>
<?php
class UserController extends AppController {
var $name = 'User';
function add() {
if(!empty($this->data)) {
if ($this->User->save($this->data)) {
$this->Session->setFlash(__('The User has been saved', true));
$this->redirect(array('controller' => 'users', 'action'=>'index'));
} else {
$this->Session->setFlash(__('The User could not be saved. Please, try again.', true));
}
}
}
}
?>
Martins
October 19, 2009, October 19, 2009 15:58, permalink
No rating.
Login to rate!
Improve this by validating input and throw away error-reporting to other classes/functions/controllers.
<?php
class User {
public static function addUser($username, $email, $password) {
$db = Database::getInstance();
$config = Config::getInstance();
if (empty($username)) {
throw new InvalidArgumentException("Empty username");
}
// other rules here, for example, length
if (empty($email)) {
throw new InvalidArgumentException("Empty email");
}
if (filter_var($email, FILTER_VALIDATE_EMAIL)) {
throw new InvalidArgumentException("Wrong email format");
}
if (empty($password)) {
throw new InvalidArgumentException("Empty password");
}
if (strlen(utf8_decode($a))<6) { // Better get minimal length from configuration
throw new InvalidArgumentException("Password shorter than 6 symbols");
}
$sql = sprintf("INSERT INTO `users` (`name`, `email`, `password`, `date`, `role_id`, `active`, `new`)
VALUES ('%s', '%s', '%s', NOW(), %d, %d, %d)", $db->real_escape_string($username), $db->real_escape_string($email), md5($password), $config->user->new->role, 0, 1);
$id = $db->insert($sql);
return new User($id);
}
}
Is this user registration function secure? Are the $db->real_escape_string functions necessary? How can I improve this? Thanks.