1

I'm trying to develop my functions in PHP (not OOP), to create a CRUD. The goal is to use the same function to any table, but I got stuck already in the first one. Can't figure how to do this.

What I have right now:

// function to avoid injections
function validate($link, $field){
    $valid = mysqli_real_escape_string($link, $field);
    $valid = strip_tags($valid);
    return $valid;
}

// validate input of array
function sqlWithArray($link,$array){
    $return = array();
    foreach($array as $field=>$val){
        $return[$field] = "'".validate($link, $val)."'";
    }
    return $return;
}

// Multi insert to any table
function InsertDB($link, $table, array $args){
    $rows = sqlWithArray($link,$args);
    $keys = "(".implode(array_keys($args)," ,").")";
    $values = " VALUES (".implode(array_values($args),", ").")";
    $query = "INSERT INTO $table $keys $values";

    return $link->execute();
}

I was try to using it as:

InsertDB($link, "test_table", $args); //$args is an array

But I keep getting the following error:

PHP Fatal error:  Uncaught Error: Call to undefined method mysqli::execute() in includes\functions.php:37

My 37 line is empty, but 36 and 38 are the following:

$query = "INSERT INTO $table $keys $values";

return $link->execute();

What I'm doing wrong here?

Dharman
  • 30,962
  • 25
  • 85
  • 135
Tiago
  • 625
  • 5
  • 16
  • 1
    Well. Why did you wrote `$link->execute();`? Where did you get that? – Your Common Sense Sep 10 '19 at 11:46
  • I'm used to prepared statements and that was the way I was doing it, but one function to each insert. In prepared statements I needed to execute the query. Any suggestions? – Tiago Sep 10 '19 at 11:48
  • I think you are not preparing the query – Lets-c-codeigniter Sep 10 '19 at 11:49
  • 1
    Well, you're creating the query as a string, but then you actually never do anything with it? – M. Eriksson Sep 10 '19 at 11:49
  • 3
    It was *not the way*, you have to execute a statement, not a connection. And, for goodness sake, **keep using prepared statements**. – Your Common Sense Sep 10 '19 at 11:50
  • I was trying to use prepared statements, but, how can I achieve the bind params information into it using a multiple table function? – Tiago Sep 10 '19 at 11:51
  • Well if you don't know it would be a good idea to ask, wouldn't it? – Your Common Sense Sep 10 '19 at 12:10
  • Isn't this post a question? I was doing a specific way. If there's a better way, I was waiting that someone could point me in that direction... – Tiago Sep 10 '19 at 12:25
  • Your question is not about prepared statements but about some peculiar code that doesn't use them. My point, if you don't know how to use prepared statements in your situation you must ask how to use prepared statements. As simple as that. – Your Common Sense Sep 10 '19 at 13:21
  • A side note: using PDO would probably be easier for you. There is also plenty of ready-made solutions which are secure and versatile. No need to reinvent the wheel. – Dharman Sep 11 '19 at 08:41

1 Answers1

4

Having such a function is a good idea per se. It indicates that you are a programmer in your heart, not just a tinkerer that writes PHP from ready made blocks like a Lego figure. Such a function can greatly improve your code.

But with great power comes great responsibility. Such a function is a constant danger of SQL injection, through table and field names. You should take care of that. Not to mention it should be properly implemented using prepared statements for the data.

First of all, you will need a general purpose function to execute an arbitrary MySQL query using a query and an array of parameters. I have a simple mysqli helper function for you. It will be a basic function to execute all prepared queries:

function prepared_query($mysqli, $sql, $params, $types = "")
{
    $types = $types ?: str_repeat("s", count($params));
    $stmt = $mysqli->prepare($sql);
    $stmt->bind_param($types, ...$params);
    $stmt->execute();
    return $stmt;
}

Now we can start constructing the SQL query dynamically. For this we will need a function that would escape identifiers

function escape_mysql_identifier($field){
    return "`".str_replace("`", "``", $field)."`";
}

It will make identifiers safe, at least as long as you are using Unocode.

Now we can proceed to creation of the correct SQL string. We will need to create an SQL with placeholders, like this:

INSERT INTO `staff` (`name`,`occupation`) VALUES (?,?)

So let's write a function that would create a query like this

function create_insert_query($table, $keys)
{
    $keys = array_map('escape_mysql_identifier', $keys);
    $fields = implode(",", $keys);
    $table = escape_mysql_identifier($table);
    $placeholders = str_repeat('?,', count($keys) - 1) . '?';
    return "INSERT INTO $table ($fields) VALUES ($placeholders)";
}

And finally we can write the long-sought crud function:

function crud_insert($conn, $table, $data) {
    $sql = create_insert_query($table, array_keys($data));
    prepared_query($conn, $sql, array_values($data));
}

called like this

$args = ['name' => "D'Artagnan", "occupation" => 'musketeer'];
crud_insert($link, "test_table", $args); 
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • Amazing answer. Thanks. Since I have some confusion in my head right now, I can use the same "logic" to create the other needed functions right? Thanks. – Tiago Sep 10 '19 at 14:06
  • 1
    Well update will require a bit more advanced string manipulation but I am sure you'd be able to do it. For selects please limit your crud to a simple primary key lookup function. A proper query builder is ten times more complex program than your whole crud, better leave it alone. – Your Common Sense Sep 10 '19 at 14:09
  • htmlspecialchars (ex: `$fname = htmlspecialchars($_POST["fname"], ENT_QUOTES, 'UTF-8');`) is another way of making POST values safer. That said, it's still possible to circumvent using htmlspecialchars alone, but adds a bit of a safety net in case you forget something down the line. I've gotten into the habit of declaring all of my POST values this way. – froggomad Sep 12 '19 at 00:06
  • @froggomad, you seems to be confusing HTML with SQL. The question here is about SQL and the function you mentioned doesn't "sanitize" anything related to SQL. If that's what you mentioned in your other comment, then your query is wide open to SQL injection – Your Common Sense Sep 12 '19 at 00:10
  • I'm clearing referring to POST variables and gave an example of declaring one, that's again clearly PHP... htmlspecialchars is a PHP function https://www.php.net/manual/en/function.htmlspecialchars.php that escapes nearly all of the malicious characters. No, it's not my only method of sanitizing for SQL injection as I mentioned, it's a safety net in case something is forgotten down the line – froggomad Sep 12 '19 at 03:10
  • @froggomad Sorry, but you couldn't be more wrong. [htmlspecialchars doesn't help against SQL injection](https://stackoverflow.com/q/22116934/285587). There is [no such thing as "malicious characters"](https://stackoverflow.com/a/2995163/). Bulk escaping of post variables is [wrong as well](https://stackoverflow.com/a/18573730/). There is no such thing as a "safety net". Too many cooks will spoil the broth, as they say. Your net will do more harm than good with your "net". ONLY [use prepared statements for the data and a white list for the everything else](https://stackoverflow.com/q/60174/) – Your Common Sense Sep 12 '19 at 07:45
  • It would seem you're right. Thanks for clearing up that misconception. – froggomad Sep 12 '19 at 17:34
  • It looks like your escape func is missing the closing quote – froggomad Sep 12 '19 at 17:35
  • @froggomad indeed, thank you! Although I strongly recommend a whitelisting approach, escaping is an acceptable tradeoff in such a case – Your Common Sense Sep 12 '19 at 17:37