How can I make this php database class more secure?
I'm using the following PHP MySQL database class. I'm curious as to what I could do to make it more secure. I'm happy with it so far, but without suggesting to "use PDO" what can I do to improve this currently?
<?
class DbConnector {
public static function getInstance(){
static $instance = null;
if($instance === null){
$instance = new DbConnector();
}
return $instance;
}
var $theQuery;
var $link;
function DbConnector() {
$host = 'localhos开发者_如何转开发t';
$db = '';
$user = '';
$pass = '';
// connect to the db
$this->link = mysql_connect($host, $user, $pass);
mysql_select_db($db);
register_shutdown_function(array(&$this, 'close'));
}
function find($query) {
$ret = mysql_query($query, $this->link);
if (mysql_num_rows($ret) == 0)
return array();
$retArray = array();
while ($row = mysql_fetch_array($ret))
$retArray[] = $row;
return $retArray;
}
function insert($query) {
$ret = mysql_query($query, $this->link);
if (mysql_affected_rows() < 1)
return false;
return true;
}
function query($query) {
$this->theQuery = $query;
return mysql_query($query, $this->link);
}
function fetchArray($result) {
return mysql_fetch_array($result);
}
function close() {
mysql_close($this->link);
}
function exists($query) {
$ret = mysql_query($query, $this->link);
if (mysql_num_rows($ret) == 0)
return false;
}
function last_id($query) {
return mysql_insert_id($query);
}
}
?>
If you don't want to use something that would automatically escape strings for you, you should at least provide an escapeString($string)
method (which would call mysql_real_escape_string()
) that you can use to escape strings when composing the query.
If you want to start using PDO (highly recommended) then you will probably have different methods and won't need to include an escaping method.
As for general considerations:
- don't use
var
, but rather usepublic
,protected
orprivate
(var
has been deprecated as of PHP 5.0, and is currently an alias forpublic
) - the
__construct()
method is preferred instead of the method with the name of the class - it would be better not to connect in the constructor, but rather only connect when you really need to connect in order to perform a query; a constructor should do as little as possible, and generally only initialize properties
mysql_connect()
andmysql_select_db()
will returnfalse
if they cannot do what they're supposed to; you should throw an exception if that happens and catch it where you use the class- the
mysql_query()
call in thefind()
andexists()
methods might return something other than a resource (false
for example) so you should check for that before callingmysql_num_rows()
- if you're not using
$theQuery
at all, you should remove it - if you want to have access to
$theQuery
, you should make it private and provide agetQuery()
accssor method - if you're setting the
$link
property to the resource returned bymysql_connect()
, you should use that property in all yourmysql_*
calls, such as the one atlast_id()
- you should make
$link
private (as a rule of thumb, all properties should be private or protected, and accessor methods should be provided when needed - read about the concept of encapsulation) - even though all methods are by default
public
, having an explicitpublic
keyword before each of them will make things clearer
ok I hate PDO too, "my own code is my own risk, while others code is also your own risk". Anyway, if you will use this script without any cleaner I'm sure you will make your DB explode. you may want to take a look at others mysql database connection classes before starting your own:
take a look here
精彩评论