我正在尝试通过创建此个人CMS来深入学习PHP。我尝试利用OOP概念,并尽我所能...

我尝试设计此CMS的方法是在页面中始终包含“模块”以最小化代码。我广泛使用POST和GET变量来确定将到达哪个模块。索引页面将是包含和访问模块的主要页面。

我的问题:


当代码越来越多时,这种设计是否方便?更复杂?更多POST请求等。
我是否可能存在任何安全漏洞?
是否存在性能问题? (没有注意到)
我是否正确使用代码,因为这是我真正的PHP项目?
我希望对此代码进行一些深入的批评。

配置文件:

<?php
    class cms {
        private $dbhost     = "";
        private $dbuser     = "";
        private $dbpass     = "";
        private $dbdb       = "";
        private $dbc;
        public $website;
        public $user;
        public $theme;

        function __construct() {
            $this->session();
            $this->getWebsite();
        }
        function connect() {
            $dbc = mysqli_connect($this->dbhost, $this->dbuser, $this->dbpass, $this->dbdb) or die("Error " . mysqli_error($dbc));
            return $dbc;
        }
        function session() {
            if(!isset($_SESSION)) {
                session_start();
            }
        }
        function getWebsite() {
            $dbc = $this->connect();
            $stmt = $dbc->prepare("SELECT * FROM website");
            $stmt->execute();
            $result = $stmt->get_result();
            $this->website = $result->fetch_assoc();
            $stmt->close();
            $this->theme = $this->website['theme'];
        }
    }
?>


Index.php文件:

<?php
    require_once('config/config.inc.php');
    $cms = new cms();
    include("template/" . $cms->theme . "/head.php");
    $moduleArray = array_diff(scandir('module/', 1), array('..', '.', 'admin'));

    if(($_SERVER['REQUEST_METHOD'] === 'GET' || $_SERVER['REQUEST_METHOD'] === 'POST') && !empty($_GET['page'])) {
        $p = $_GET['page'];
        $ext = ".inc.php";

        if (get_magic_quotes_gpc()) {
            $p = stripslashes($p);
        }

        if(in_array($p . $ext, $moduleArray)) { 
            include("module/" . $p . $ext);
        }
        else {
            header('Location: /');
        }
    } // end (GET || POST) && GET
    else {
        foreach($moduleArray as $module) {
            $name = substr($module, 0, -8);
            echo " <a href=\"index.php?page=$name\"> $name </a><br />";
        }
    } // end ELSE

    include("template/" . $cms->theme . "/foot.php");
?>


演示模块:Page.inc.php

>
<?php
    $dbc = $cms->connect();

    if(isset($_GET['page'])) {
        if(isset($_GET['title'])) {
            $t = $_GET['title'];

            if (get_magic_quotes_gpc()) {
                $t = stripslashes($username);
            }

            if ($stmt = $dbc->prepare("SELECT title, content_main FROM pages WHERE url=?")) {       
                $stmt->bind_param('s', $t);
                $stmt->execute();
                $stmt->store_result();
                $stmt->bind_result($title, $content_main);

                if($stmt->fetch()) {
                    echo "<h2>$title</h2><p>$content_main</p>";
                }
                else {
                    echo "<h2>404</h2><p>The page doesn't exist!</p>";
                }

                $stmt->free_result();
                $stmt->close();
            }
            else {
                echo "error";
            }
        }
        else {
            if ($stmt = $dbc->prepare("SELECT title, url FROM pages")) {        
                $stmt->execute();
                $stmt->store_result();
                $stmt->bind_result($title, $url);

                while ($stmt->fetch()) {
                    echo "<a href=\"index.php?page=page&title=$url\">$title</a></br />";
                }

                $stmt->free_result();
                $stmt->close();
            }
        }
    }
    else {
        header('Location: ../');
    }
?>


#1 楼

您使用的开发

oop&design-patterns标签已经在这里说明需要做什么。目前,这是一个结构化代码(带有某些类语法),并且很难扩展。即使它看起来还不是一个大项目,并且您在这里不需要深层的对象结构/层,但我认为您还是应该学习一些技巧。您会注意到的第一个优点是减少了重复代码。分离请求处理,对页面内容进行建模并将其进一步提取到将处理显示的“查看”代码将使开发变得更加容易。我认为如何以oop方式编写此代码?应该是这里的问题。

安全性

您显示的代码中没有太多安全隐患。使用准备好的语句将保护您免受SQL注入的侵害,并且我不希望当前显示的内容由站点所有者提供(但是在此,所有者的识别很重要)。如果您先验证请求(如上所述),那么即使是注入本身也不会成为问题。我并不是说您应该删除准备好的语句-即使有时只是出于良好实践的缘故也要坚持使用。

性能

无需使用目录扫描和文件白名单检查请求是否命中文件之一。我会简单地使用file_exists()(如果需要,可以使用某些条件)。再说一次:这可能是请求验证的一部分。

将页面内容存储在数据库中是一种过大的imo,因为您直接将其显示在屏幕上。为什么不将其写为文件?除了(可能通过另一种方式解决)FULLTEXT搜索之外,您实际上也不会对它进行任何数据处理方面的操作(排序,分组...)。简单的include将替换db SELECTwebsite表也是如此-它看起来像一个(半)恒定配置,可以存储在文件中(至少作为数据库缓存)。

评论


\ $ \ begingroup \ $
我有一个问题,您提到每次访问页面时读取数据库都是一个坏主意。如何解决呢?我可以使用会话,但是有cookie的需要,如何在没有cookie的情况下存储数据?每个用户在服务器上的文件?
\ $ \ endgroup \ $
–HelpNeeder
2014年7月25日下午5:35

\ $ \ begingroup \ $
不仅实施CMS内容管理员,实施用户帐户也会发生很大变化。在这种情况下,很难避免使用会话/ cookie,并且使用db和它是合理的。这个主题太广泛了,无法在此处进行详细介绍,但是有很多教程和脚本对此进行了介绍。我建议您不要在此问题上追求“更智能”的解决方案,而应接受流行的解决方案。还请注意,由于您的用户提供的内容可能会尝试某些讨厌的操作,因此需要更高的安全级别。
\ $ \ endgroup \ $
–颤抖
2014年7月25日在16:38



#2 楼

好吧,我将首先回答您所提出的问题,然后视情况是否覆盖而定,我可能会在末尾附加更多内容。我们会看到,我还没有真正阅读您的问题!

所以,让我们抓住第一个...


代码变得越来越复杂?更多POST请求等。


否。目前,我看不出有任何扩展的方法。一旦它“变得越来越复杂”,您就会遇到很多问题,尤其是在网站体系结构方面。

面向对象范例的一部分是称为SOLID的原理。如果我们从代表单一职责的S开始,我们会立即注意到您的代码中存在多个缺陷。例如,您的cms类正在处理数据库连接,数据库查询和会话处理。所以说真的,这立即告诉我们甚至没有必要cms!它告诉您您确实需要数据库连接处理程序,查询实用程序和单独的会话处理类。但是,甚至可以进一步细分那些类,以防止拥有功能强大的“ Manager”类。

SOLID中的第二个原则是打开和关闭代码。这意味着可以扩展,不能修改。我现在在您的代码中都看不到。我们无法建立在cms或索引文件或演示页面上。更改这些类肯定会很痛苦,并且由于我们甚至需要更改它们,因此您的代码并未关闭。

一种可能的解决方案是KISS。因为您的班级要负责很多事情,所以很难分开。这使得难以更改页面唯一信息。给功能上的呼吸空间,不要指望事物总是适合同一类,因为那不是OOP。

研究SOLID的其余部分,这是一个很大的话题,我可以指出排除其他错误,但最好继续前进!





那么,您担心什么?我说这是因为,如果您是唯一看到此站点的人,威胁将有所不同,而不是攻击者所面临的威胁知道您的确切数据库方案。

我会指出一些问题,但是由于我不是专业人员,所以我无法进行完整的安全审核。


使用.inc.php是一种约定,但可以认为是不好的约定。如果您忽略了随附的.php,则存在与使用它相关的威胁。幸运的是,您看起来不错!不过请小心。
从PHP文档开始,不建议使用魔术引号函数。避免使用其中的任何一种,因为支持可能会出错,并使您容易受到不同类型的攻击。<​​br />然后,用斜杠将其删除。但是,这仅在魔术引号有效的情况下。如果不是,则攻击者可以对他想要的任何页面进行include(具有正确的权限)。我会避免给用户这么多的功能。

stripslashes($username)没有意义。我不确定如果使用准备好的语句为什么要这么做。最重要的是,我什至不知道你从哪里得到$username。这是无效代码,也有安全隐患。
通过数据库注入到页面中的所有代码最好是有史以来最干净的代码。如果攻击者能够将页面数据插入数据库,则他们可能会将恶意HTML / JavaScript插入每个站点用户将看到的页面中。非常糟糕。

这是我可以立即发现的东西,但是我确定我确实错过了一些东西。


是否有任何性能问题?


这很难说。您为什么不分析代码并查看它是否在任何时候出现瓶颈?然后检查这些部分,并了解有关重构它们的信息。

据我所知,数据库看起来效率很低,并且您进行了许多不必要的调用。不正确的连接存储和缓存在这里也很普遍。每次加载页面时,都会启动一个新的数据库连接,然后执行一组全新的查询。主题会在每次加载单个页面时发生变化,还是可以存储的东西?


我是否正确使用代码,因为这是我真正的PHP项目?


“正确”是什么意思?我什至不知道该怎么回答!您在问这是否值得生产吗?如果是这样,我会自动拒绝所有要指出的要点。


我希望对此代码进行一些强烈的批评。


很好,因为我还有很多! :)


首先,遵循PHP编码样式标准。首先,我将向您介绍几乎官方的PHP-FIG标准。重要的是,每一段代码都必须一致且可读。
避免使用诸如$dbhost之类的缩写形式,例如“数据库”。它不会缩短任何好处。与$dbpass中的“密码”相同。

$dbdb很荒谬。显然,这需要一个新名称。

$dbc毫无意义或清晰性。给您可变的上下文!
public $website之类的东西不应该公开。封装并为它们提供getter和setter。
您的构造函数不应创建这些类型的变量。整个类都需要重新考虑,然后应用依赖注入原理,从SOLID中的D了解更多。
使您的方法具有可视性。保留默认值被认为是不好的做法。

connect()是不好的名字。连接到什么? cms连接到什么?哦!也许它需要连接到Google Patents API!

session()确实不属于这里,我们讨论了这个问题。

getWebsite()非常模糊。这需要大量工作。

$moduleArray具有匈牙利符号,并且似乎对此感到困惑。
您真的需要取消单个字符变量($p$t
如果您的代码需要诸如// end (GET || POST) && GET之类的注释,则您的条件太长。我不需要注释就可以了解代码的不同阶段。
总体而言,Index.php承担着很多责任,这些责任可以外包给不同的文件/类。

$dbc = $cms->connect(); $cms在哪里定义;这是无效代码。
我看到您有4个层次的嵌套,需要进行分解。通过简单的重构,或者如果可能的话,通过保护子句。

echo "error";看起来很可怕。添加用户友好和适当的错误处理。 Stack Exchange网络非常适合查找有关如何执行此操作的问题。
<a href=\"index.php?page=page&title=$url\">$title</a></br />这样的文字很难阅读。要么从PHP中删除HTML(然后在其他地方将其圆顶),要么使用类似sprintf的函数。


好吧,这就是我可以挤出的东西!希望您知道需要做什么:)我很乐意回答任何问题。

评论


\ $ \ begingroup \ $
太棒了!我需要一点时间来消化,但是我非常感谢您的答复。谢谢。
\ $ \ endgroup \ $
–HelpNeeder
2014年7月24日在4:12

\ $ \ begingroup \ $
我不是要求“强烈批评”的人;)
\ $ \ endgroup \ $
– Alex L
2014年7月24日在4:13

\ $ \ begingroup \ $
超级好。这就是我的意思。 :P
\ $ \ endgroup \ $
–HelpNeeder
2014年7月24日在4:14

\ $ \ begingroup \ $
因此,使用.inc.php时,不会出现.inc的安全问题。 (.inc的问题是服务器通常没有配置为将其视为PHP。当文件名末尾带有.php时,这不是问题。
\ $ \ endgroup \ $
– cHao
14年7月24日在8:51

\ $ \ begingroup \ $
@chao我知道扩展名有些奇怪,不幸的是我误读了该页面。谢谢!
\ $ \ endgroup \ $
– Alex L
2014年7月24日9:01



#3 楼

这是我的第一篇评论。让我们看一下您的代码。

一个类,一个任务。

首先,我建议您使用名称空间。

Namespaces对于大型项目非常方便,将会进一步扩展。单击此处获取文档。我建议您使用命名空间的原因是因为您必须简化代码,而不是仅使用一个类来处理所有内容。也许,您可以有一类存储程序的逻辑/变量,而另一类存储所有处理用户交互并调用Mapper的功能,等等。让我们看一下可以使用的示例代码。

namespace CMS;
class UserHandler {

}
class Mapper {

}
class CMSData {

}
/* In this below section, we instantiate each class according to the way we will pass their dependencies to the class. */

$CMSData     = new CMSData();
$Mapper      = new Mapper($CMSData);
$UserHandler = new UserHandler($CMSData, $Mapper);


如果您不了解依赖注入,请单击此处。

在在上面的代码中,我们创建了一个名为CMSData的类来承载所有数据,并且每个其他类都可以访问该类。因此,当您想对数据存储进行任何操作时,您的类将是CMSData,您可以在其中更改内容。 UserHandler类用于处理用户交互。让我们举个例子,假设您的$_GET告诉您URL已更改,并且必须加载新页面,因此将在handlePageLoad中调用一个函数,例如index.php,还包含一个请求页面的参数传递给UserHandler类。 UserHandler类调用Mapper类中的函数,该类会进行有效性检查等,并在必要时将一些信息存储在CMSData中,该信息中有大量可用的变量。


这种设计是否可以当代码变得越来越复杂时派上用场吗?更多POST请求等。


,除非您进行组织,否则您当前的设计将过于混乱。如果您按照我建议的方式以通用方式进行编码,那么使用更复杂的代码就可以了。


我可能有任何安全漏洞吗?


不,只要您以安全的方式接受用户输入,就不会。看来您做对了。


任何性能问题? (没有注意到)


我没有发现任何错误。只要确保您尝试以更好的逻辑简化代码即可。


我是否正确地利用了代码,因为这是我真正的PHP项目?


我想要建议您,仅允许您的Mapper类访问数据库,而其他页面则必须直接为Mapper类调用函数以进行与数据库有关的工作,并且您创建了可以在此处使用的不同函数,这将允许您可以使程序易于管理。例如,您有一个新主题,而您想展示的东西则有所不同,您所要做的就是去上课,并相应地编辑所有内容,而不必遍历查找代码。 >

我希望对此代码进行深入的批评。


为什么不在函数中使用像public, private and protected这样的关键字?您仅在变量上执行此操作。

评论


\ $ \ begingroup \ $
最后两点让我想起了这里的另一个答案;)很高兴看到桌子转身,看着你回答而不是问! :)
\ $ \ endgroup \ $
– Alex L
2014年7月24日在7:35

\ $ \ begingroup \ $
很棒的一点!我将尝试做更多关于这一切的阅读!谢谢!
\ $ \ endgroup \ $
–HelpNeeder
2014年7月24日在7:36

\ $ \ begingroup \ $
@AlexL,既然您已声明,我将其删除。不知道
\ $ \ endgroup \ $
–废弃的帐户
2014年7月24日在7:39