PHP注册脚本

Question

所以我想为我的网站写这个脚本。 它看起来相当混乱和破碎。 也许有人可以帮我整理一下，解释一下可能不正确。 另外，有没有办法让它更短，对我来说看起来有点不安全。 谢谢。

<?php 

class Register 
{ 
    private $username; 
    private $password; 
    private $password2; 
    private $passmd5; 
    private $email; 
    private $email2; 

    private $errors; 
    private $rtoken; 

    public function __construct() 
    { 
     $this->errors = array(); 

     $this->username = $this->filter($_POST['ruser']); 
     $this->password = $this->filter($_POST['rpass']); 
     $this->password2 = $this->filter($_POST['rpass2']); 
     $this->email  = $this->filter($_POST['remail']); 
     $this->email2 = $this->filter($_POST['remail2']); 
     $this->rtoken = $_POST['rtoken']; 

     $this->passmd5 = md5($this->password); 
    } 

    public function process() 
    { 
     if ($this->valid_rtoken() && $this->valid_data()) 
      $this->register(); 

     return count($this->errors) ? 0 : 1; 
    } 

    public function filter($var) 
    { 
     return preg_replace('/[^a-zA-Z0-9@.]/', '', $var); 
    } 
    public function register() 
    { 
     mysql_query("INSERT INTO users(username,password,email) VALUES ('{$this->username}','{$this->passmd5}','{$this->email}')"); 

     if (mysql_affected_rows() < 1) 
      $this->errors[] = '<font color="red">Database error</font>'; 
    } 

    public function user_exists() 
    { 
     $data = mysql_query("SELECT ID FROM users WHERE username = '{$this->username}'"); 

     return mysql_num_rows($data) ? 1 : 0; 
    } 

    public function email_exists() 
    { 
     $data = mysql_query("SELECT ID FROM users WHERE email = '{$this->email}'"); 

     return mysql_num_rows($data) ? 1 : 0; 
    } 

    public function show_errors() 
    { 
     echo ""; 

     foreach ($this->errors as $key => $value) 
      echo $value . "<br>"; 
    } 

    public function valid_data() 
    { 
     if ($this->user_exists()) 
      $this->errors[] = '<font color="red">Username Exists</font>'; 
     if ($this->email_exists()) 
      $this->errors[] = '<font color="red">email exists</font>'; 
     if (empty($this->username)) 
      $this->errors[] = '<font color="red">check your username</font>'; 
     if (empty($this->password)) 
      $this->errors[] = '<font color="red">check your password</font>'; 
     if ($this->password != $this->password2) 
      $this->errors[] = '<font color="red">Passwords do not match</font>'; 
     if (empty($this->email) || !eregi('^[a-zA-Z0-9._-]+@[a-zA-Z0-9._-]+\.[a-zA-Z]{2,4}$', $this->email)) 
      $this->errors[] = '<font color="red">Check your email</font>'; 
     if ($this->email != $this->email2) 
      $this->errors[] = '<font color="red">Emails do not match</font>'; 

     return count($this->errors) ? 0 : 1; 
    } 


    public function valid_rtoken() 
    { 
     if (!isset($_SESSION['rtoken']) || $this->rtoken != $_SESSION['rtoken']) 
      $this->errors[] = '<font color="red">Check</font>'; 

     return count($this->errors) ? 0 : 1; 
    } 
} 

?> 

Answer 1

Swoosh，总是有更好的方法来编写代码。我希望挑战你重新思考为什么你的代码很长，为什么它可能不安全，而不是为你重写你的代码。希望你会这样学习更多。

首先，用MD5对密码进行哈希处理是完全不安全的。请忘记MD5曾经存在，请不要使用SHA1来保证任何安全。相反，你应该使用bcrypt或者PBKDF2（使用SHA2或者Whirlpool和〜2000 + rounds）。我建议你参考我对Secure Hash and Salt for PHP Passwords的回答，以获得有关文章和图书馆的提示和链接，以帮助实现更好的安全性。

其次，从PHP 5.5起，mysql_ *函数是deprecated。使用MySQLi将帮助您，但您应该使用一致的接口（如PDO）来处理连接，并查询转义/筛选。虽然你可能不会在PHP 5.5中构建你的软件，但如果一个主机决定升级你的PHP版本，你将无法控制它。尽可能多地优化未来的兼容性。另外，PDO有一些漂亮的功能，在its documentation中有解释。

第三，你不应该使用正则表达式来“过滤”你的查询变量。你可以做的最安全的事情是在任何数字上使用intval/floatval，并通过你使用的数据库库，如mysql_escape_string（或mysqli_real_escape_string）或使用prepared statements（它将为你清理变量）转义剩下的部分。

第四，你正在把显示逻辑放到你的对象中。想一想：这个对象有什么目的/作用？它处理注册逻辑吗？它是否存储注册数据？这里使用Single Responsibility Principle是个好主意。看起来这个对象应该像一个混合模型控制器一样工作，其中包含表示信息。您可以将其扩展到RegistrationModel和RegistrationController类来处理临时存储数据，或者甚至决定执行其他操作。但请记住，班级承担的责任越多，就有越多的方式需要打破。

另外，通过使注册为private的所有属性，您不能有多种注册方式。如果你想通过OAuth（例如Twitter或Facebook）登录进程，但需要重用注册表中的某些逻辑，该怎么办？这些属性至少应该受到保护，以便您可以继承它们，甚至是公共的，以便另一个对象可以与它们进行交互（例如通知用户它们的注册成功的对象）。