1

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

<?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;
    }
}

?>
4

1 回答 1

3

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

首先，使用 MD5 散列密码是完全不安全的。请忘记 MD5 曾经存在过，并且请不要使用 SHA1 来确保任何安全。相反，您应该使用 bcrypt 或 PBKDF2（使用 SHA2 或 Whirlpool 以及 ~2000+ 轮）。我建议您参考我对Secure Hash and Salt for PHP Passwords的回答，以获取帮助实现更好的安全性的文章和库的提示和链接。

其次，从 PHP 5.5开始不推荐使用mysql_* 函数。使用 MySQLi 可以解决问题，但您应该使用一致的接口（例如PDO）来处理连接和查询转义/过滤。虽然您可能不会在 PHP 5.5 中构建您的软件，但您将无法始终控制主机是否决定升级您的 PHP 版本。尽可能优化未来的兼容性。另外，PDO 有一些在其文档中解释的漂亮特性。

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

第四，您将显示逻辑放入您的对象中。想一想：这个对象实现了什么目的/作用？它是否处理注册逻辑？它是否存储注册数据？在这里使用单一职责原则是个好主意。看起来该对象应该像一个混合模型控制器一样工作，其中包含表示信息。您可以将其扩展为 RegistrationModel 和 RegistrationController 类来处理临时存储数据，甚至决定做其他事情。但请记住，你的班级承担的责任越多，它必须打破的方式就越多。

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

于 2013-02-13T18:19:30.537 回答