PHP OOP Learning Curve - MySQL Object Failure

Question

I'm currently learning PHP and I'm new to OOP. I'm trying to create an object to handle MySQL queries and connections.

This is what I've created so far:

class MySQLDatabase {

    private $connection;

    function __construct() {
        $this->open_connection();
    }

    public function open_connection() {
        $this->connection = mysqli_connect(DB_SERVER, DB_USER, DB_PASS, DB_NAME);
        if(mysqli_connect_errno()) {
            die(
                "Database connection failed: " . mysqli_connect_error() . 
                " (" . mysqli_connect_errno() . ")"
            );
        } 
    }

    public function close_connection() {
        if(isset($this->connection)) {
            mysqli_close($this->connection);
            unset($this->connection);
        }
    }

    public function query($sql) {
        $cleaned_sql = mysqli::real_escape_string($sql);
        $result = mysqli_query($this->connection, $cleaned_sql);
        $this->confirm_query($result);
        return $result;
    }

    public function mysql_prep($string) {           
        $escaped_string = mysqli_real_escape_string($this->connection, $string);
        return $escaped_string;
    }

    private function confirm_query($result) {
        if (!$result) {
        die("Database query failed.");
        }
    }
}

And on the public-facing side (doing a test to make sure things work as expected):

$sql = "INSERT INTO users (id, username, password, first_name, last_name) ";
$sql .= "VALUES (1, 'jbloggs', 'secretpwd', 'Joe', 'Bloggs')";
$result = $database->query($sql);

Currently, I just get the output of: Database query failed.

The issue seems to be something to do with my mysql_prep function, as when I remove that all works fine.

Any advice is greatly welcomed.

Thanks in advance! Alex.

*"Currently, I just get the output of: Database query failed."* - You need to find out what the real error was. — Funk Forty Niner, Sep 22 '16 at 20:04
@Fred-ii- Error appears to be related to my mysql_prep function, as when this is removed things appear to work fine. — FruitWinder, Sep 22 '16 at 20:08
If you are learning OOP then wrapping an existing database access object is that last thing you shoud be doing. Wait till you have a good idea about OO before attempting something that most good programmers get wrong — RiggsFolly, Sep 22 '16 at 20:10
Also Your script is at risk of [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Have a look at what happened to [Little Bobby Tables](http://bobby-tables.com/) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) — RiggsFolly, Sep 22 '16 at 20:12
@RiggsFolly Not putting anything in to production, have a long way to go before fully at grasps with it, clearly. Just doing a bit of learning with it and testing myself to see what I am able to do, but yes completely understand that it shouldn't be taken lightly. — FruitWinder, Sep 22 '16 at 20:13

Marc B · Accepted Answer · 2016-09-22T20:11:16.697

You're running your ENTIRE query through the escape function, which is exactly the WRONG thing to do. That eliminates any quotes necessary for the query to be syntactically correct.

Consider this:

INSERT INTO foo (bar) VALUES ('baz')

Since you're escaping the entire thing, you're sending this to the database:

INSERT INTO foo (bar) VALUES (\'bar\')

since those quotes are escape, they're not quotes anymore. they're plaintext characters, and the DB is looking for a field named 'bar' to get a value from. You can't look up a field in a record for insertion, because you're INSERTING the record and it doesn't exist yet. And 'bar' is unlikely to exist in your table definition.

So your query fails with a syntax error, and since you have a fixed/unchanging/useless "failed" message, you never get told the reason WHY it failed.

At least change your die() to something like

die("Query failed: " . mysqli_error($this->connection));

Whoops, derp moment. Thanks very much, makes a lot more sense and duly noted :-) — FruitWinder, Sep 22 '16 at 20:10

PHP OOP Learning Curve - MySQL Object Failure

1 Answers1