REST API in clean PHP

Question

I tried to create REST API in PHP because I would like to learn the basics before I will start use frameworks to create apps. I created a Model with CRUD methods which can be used in every model to avoid repeating code. Class Model will be inherited by classes like Product, Measurement etc. I want to create diet application for android and web app. I tried to separate backend from front and mobile app but because im newbie tried to use clean PHP instead using frameworks. I wonder is that a good way because when I was looking where crud methods should be placed (in model or controller) I saw a lot of answers in both ways. My model looks like:

<?php
class Model
{
    protected $connection;
    protected $tableName;

    /**
     * Model constructor.
     * @param $connection
     */
    public function __construct($connection)
    {
        $this->connection = $connection;
    }

    public function read($id)
    {
        $sql = "SELECT * FROM `$this->tableName` WHERE id = ?";
        $stmt = $this->connection->prepare($sql);
        $stmt->execute([$id]);
        $result = $stmt->fetchAll(\PDO::FETCH_ASSOC);
        return json_encode($result);
    }

    public function create($data)
    {
        $params = [];
        $values = "";
        foreach ($data as $key => $value) {
            $params[] = $key;
            $values .= "'$value',";
        }
        $params = implode(",", $params);
        $values = rtrim($values, ",");
        $sql = "INSERT INTO $this->tableName ($params) VALUES ($values)";
        $stmt = $this->connection->prepare($sql);
        $stmt->execute();
        return json_encode("Record created");
    }

    public function readAll()
    {
        $sql = "SELECT * FROM " . $this->tableName;
        $stmt = $this->connection->prepare($sql);
        $stmt->execute();
        $result = $stmt->fetchAll(\PDO::FETCH_ASSOC);
        return json_encode($result);
    }

    public function update($data, $id)
    {
        $set = "";
        foreach ($data as $key => $value) {
            $set .= $key . '=' . '"' . $value . '",';
        }
        $set = rtrim($set, " ,");
        $sql = "UPDATE $this->tableName SET $set WHERE id = $id";
        $stmt = $this->connection->prepare($sql);
        $stmt->execute();
        return json_encode("Record updated");
    }

    public function delete($id)
    {
        $sql = "DELETE FROM `$this->tableName` WHERE id = ?";
        $stmt = $this->connection->prepare($sql);
        $stmt->execute([$id]);
        return json_encode("Record deleted");
    }
}

And then I extend Model for example Product like below:

<?php
include_once 'Model.php';
class Product extends Model
{
    protected $tableName = 'products';
}

My controllers are currently empty. I dont know where add methods to validate data. I was watching tutorials and their way to print json data was creating url like /api/product/read.php and they created all methods in that way. But I think thats wrong way because if I understood well concept of rest api we should use http methods like get/post etc instead using another urls to every method. Then I created a directory api and added file products.php. In that file created switch to handle http methods and use proper methods that are inherited from Model class and I dont need to rewrite that methods for every model.

 <?php
include_once '../../bootstrap.php';
include_once '../model/Measurement.php';
include_once '../config/DatabaseConnector.php';

header("Access-Control-Allow-Origin: *");
header("Content-Type: application/json; charset=UTF-8");
header("Access-Control-Allow-Methods: GET,POST,PUT,DELETE");
header("Access-Control-Max-Age: 3600");
header("Access-Control-Allow-Headers: Content-Type, Access-Control-Allow-Headers, Authorization, X-Requested-With");

$url = $_SERVER['REQUEST_URI'];
$urls = explode('/', $url);
$id = end($urls);

$connection = new DatabaseConnector();
$products = new Product($connection->getConnection());

$input = (array) json_decode(file_get_contents('php://input'), TRUE);

switch ($_SERVER['REQUEST_METHOD']){
    case 'GET': if ($id !== "products.php")
        print_r($products->read($id));
    else
        print_r($products->readAll());
        break;
    case 'POST': print_r($products->create($input));
        break;
    case 'PUT': print_r($products->update($input, $id));
        break;
    case 'DELETE': print_r($products->delete($id));
        break;
}

I will be extremally thankful if someone will rate my code, points out my mistakes and give me an example where to create validate methods. Thank you so much, have a good day!

Answer 1

The idea is very good and the structure is correct. Do not listen to anyone who would say that a crud method should be implemented in the controller. All the controller's job is to get the input and to call the CRUD method from the model. So in this regard your code is all right.

However, your model code suffers from the opposite mix-up: it shouldn't format the output. That's the controller's job. Do not return any status from your CRUD methods. Your CRUD methods should only return a raw data when applicable and let the controller to format the output.

But you are overlooking a very serious vulnerability here - an SQL injection.

You should really learn about this matter, it's a very critical issue. Nobody cares how clean or efficient your code is if it imposes a critical threat to the whole application.

To mitigate this issue you should look into two places

• use the real protection instead of cargo cult prepared statements
• besides the table name, throw in a class property with alist of columns in the table, and add the field names to the SQL strictly from this list only.

For the actual implementation you can look into similar CRUD implementation I posted recently