2

I have to build reporting website in which SQL data will be imported using exported csv file from another system.

I built the code but I cannot proceed it, to import in SQL, I just have message QUERY FAILED, not error from SQL or connection with database.

Could someone review my code and write me where is my mistake? I user escape on data because i have come data like... "John's company":

<?php

if(isset($_POST['import'])) {

$filename = $_FILES['file']['tmp_name'];

if($_FILES['file']['size'] > 0 ) {
    $file = fopen($filename, 'r');

    while (($data = fgetcsv($file, 10000, ",")) !== FALSE ) {
        $data = array_map('mysqli_real_escape_string', $data);
        $query = "INSERT INTO purchasing ";
        $query .= "(
            purchase_req, 
            pr_date,  
            pr_item,  
            pr_created_by,  
            po_created_by,  
            purch_group,  
            purchase_ord,  
            po_date,  
            po_item,  
            po_state,  
            vendor_numb,  
            vendor_name,  
            mat_desc,  
            gl_acc,  
            cost_cent,  
            po_qty,  
            po_del_date,  
            po_delivered_qty,  
            po_to_be_del
        ) ";
        $query .= "VALUES ( 
            '" . $data[0] . "',  
            '" . $data[1] . "', 
            '" . $data[2] . "',  
            '" . $data[3] . "',  
            '" . $data[4] . "',  
            '" . $data[5] . "',  
            '" . $data[6] . "',  
            '" . $data[7] . "',  
            '" . $data[8] . "',  
            '" . $data[9] . "',  
            '" . $data[10] . "',  
            '" . $data[11] . "',  
            '" . $data[12] . "',  
            '" . $data[13] . "',  
            '" . $data[14] . "',  
            '" . $data[15] . "',  
            '" . $data[16] . "',  
            '" . $data[17] . "',  
            '" . $data[18] . "' 
        ) ";

        $send_to_database = mysqli_query($conn, $query);

        if(!$send_to_database) {
            die("QUERY FAILED <br>" . mysqli_error($conn));
        } else {
            header("Location: index.php");
        }
    }
    fclose($file);
}
}

?>

/// UPDATED CODE AFTER SUGGESTIONS

<?php

if(isset($_POST['import'])) {

$filename = $_FILES['file']['tmp_name'];

if($_FILES['file']['size'] > 0 ) {
    $file = fopen($filename, 'r');

    $conn = new mysqli("localhost", "####", "####", "####");

    if($conn->connect_error) {
        die("Connection failed: " . $conn->connect_error);
    }

$stmt = $mysqli->prepare("INSERT INTO purchasing (purchase_req, pr_date, pr_item, pr_created_by, po_created_by, purch_group, purchase_ord, po_date, po_item, po_state, vendor_numb, vendor_name, mat_desc, gl_acc, cost_cent, po_qty, po_del_date, po_delivered_qty, po_to_be_del) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?) ");

    fgetcsv($file, 10000, ",");
    while(($data = fgetcsv($file, 10000, ",")) !== FALSE) {
        $data = fgetcsv($file);

        $stmt->bind_param("sssssssssssssssssss", $data[0], $data[1], $data[2], $data[3], $data[4], $data[5], $data[6], $data[7], $data[8], $data[9], $data[10], $data[11], $data[12], $data[13], $data[14], $data[15], $data[16], $data[17], $data[18]);

        $stmt->execute();
    }
    fclose($file); 
}
}

?>

With the second case, i print the array but again i can't send it to SQL...

Martin M
  • 111
  • 1
  • 12
  • Don't escape it, instead prepare it. Right now you are wide open to SQL injection from the file, It can happen that way easily. In fact this error `. i user escape on data because i have come data like... "John's company"` is doing sql Injection, which is why your query is broken. Your query is something like this `..."VALUES ('John's company', ...)` see how the apos completes the first single quote `VALUES ('John' [s company'], ...)` then you have `s company'` just chilling there. – ArtisticPhoenix Mar 02 '19 at 21:26
  • That is because you are injecting a `'` into your SQL, you can avoid all this and more by simply using a prepared statement.... I would post some code, but there is probably a billion examples on the web how to do it. – ArtisticPhoenix Mar 02 '19 at 21:31
  • What do you mean when you say "prepare it" i understand your point that i have problem because of data type in my csv, but how can i prevent it? How can i proceed this data in SQL? Could you please wrote my one line of code? – Martin M Mar 02 '19 at 21:32
  • 1
    http://php.net/manual/en/mysqli.prepare.php `Prepares the SQL query, and returns a statement handle to be used for further operations on the statement. The query must consist of a single SQL statement. The parameter markers must be bound to application variables using mysqli_stmt_bind_param() and/or mysqli_stmt_bind_result() before executing the statement or fetching rows. ` Basically instead of concatinating the data into the sql, you use a place holder, then send the data later. `VALUES ( ?, ?, ?, ?)` – ArtisticPhoenix Mar 02 '19 at 21:33
  • 1
    as you wont have any single quotes in your sql `VALUES ( ?, ?, ?, ?)` then you don't have to worry about escaping them, because the DB knows that this is the SQL, and this is the DATA when you prepare it. `Could you please wrote my one line of code?` - No, just Google "MySqli Prepared statements" or such. – ArtisticPhoenix Mar 02 '19 at 21:36
  • Note: The object-oriented interface to `mysqli` is significantly less verbose, making code easier to read and audit, and is not easily confused with the obsolete `mysql_query` interface. Before you get too invested in the procedural style it’s worth switching over. Example: `$db = new mysqli(…)` and `$db->prepare("…")` The procedural interface is an artifact from the PHP 4 era when `mysqli` API was introduced and ideally should not be used in new code. – tadman Mar 02 '19 at 21:49
  • If you can, load the CSV directly into the MySQL server with `LOAD DATA INFILE`. If you can't, **use [prepared statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php)** as this sort of escaping is extremely risky. – tadman Mar 02 '19 at 21:50
  • Note: A lot of problems can be detected and resolved by [enabling exceptions in `mysqli`](https://stackoverflow.com/questions/14578243/turning-query-errors-to-exceptions-in-mysqli) so any mistakes made aren’t easily ignored. Many return values cannot be ignored, you must pay attention to each one. Exceptions don’t require individual checking, they can be caught at a higher level in the code. – tadman Mar 02 '19 at 21:50
  • I have read the PHP documentation, and i try to prepare the statement before data assigning, i test the data and it's ok, am able to print the arrays but i again i cannot send it to database... Updated code – Martin M Mar 02 '19 at 23:23

2 Answers2

1

Why not simply use the MySQL LOAD DATA INFILE syntax to load the file at once, with a single SQL query?

This MySQL buit-in is an efficient method to load a CSV file, while preventing from SQL injection and handling quoting issues. It also avoids opening and looping through the file with php. As per MySQL documentation:

When loading a table from a text file, use LOAD DATA. This is usually 20 times faster than using INSERT statements.

This should as simple as:

LOAD DATA INFILE '$filename'
    INTO TABLE purchasing(
        purchase_req, 
        pr_date, pr_item, 
        pr_created_by, 
        po_created_by, 
        purch_group, 
        purchase_ord, 
        po_date, 
        po_item, 
        po_state, 
        vendor_numb, 
        vendor_name, 
        mat_desc, 
        gl_acc, 
        cost_cent, 
        po_qty, 
        po_del_date, 
        po_delivered_qty, 
        po_to_be_del
    )
    FIELDS TERMINATED BY ',' 
    OPTIONALLY ENCLOSED BY '"'
    LINES TERMINATED BY '\r\n' -- or maybe '\n'?
;

NB : the OPTIONALLY ENCLOSED BY '"' option should handle field values like "John's company". There are many more useful options to LOAD DATA INFILE, that are described in the above linked documentation.

GMB
  • 216,147
  • 25
  • 84
  • 135
1

With your revised code, you're using prepare and bound parameters but you're missing out on one of the benefits of prepared statements: you can prepare once and execute many times with different values.

By the way, when you bind, you need a control string with as many characters as the number of parameters. In this case, you need a string of length 19.

$stmt = $mysqli->prepare("INSERT INTO purchasing (purchase_req, pr_date, pr_item, 
  pr_created_by, po_created_by, purch_group, purchase_ord, po_date, po_item, po_state, 
  vendor_numb, vendor_name, mat_desc, gl_acc, cost_cent, po_qty, po_del_date, 
  po_delivered_qty, po_to_be_del) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?) ");

while(($data = fgetcsv($file, 10000, ",")) !== FALSE) {
    $data = fgetcsv($file);

    $stmt->bind_param("sssssssssssssssssss", $data[0], $data[1], $data[2], $data[3], $data[4], $data[5], 
     $data[6], $data[7], $data[8], $data[9], $data[10], $data[11], $data[12],
     $data[13], $data[14], $data[15], $data[16], $data[17], $data[18]);

    $stmt->execute();
}

That's a little bit more efficient because MySQL doesn't have to re-parse the SQL statement for every row of your CSV input.

Tip: I prefer using PDO instead of mysqli, because it has a nicer interface for using prepared statements. You don't have to bind any variables. PDO does it for you if you pass the array of data as an argument to execute():

$stmt = $pdo->prepare("INSERT INTO purchasing (purchase_req, pr_date, pr_item, 
  pr_created_by, po_created_by, purch_group, purchase_ord, po_date, po_item, po_state, 
  vendor_numb, vendor_name, mat_desc, gl_acc, cost_cent, po_qty, po_del_date, 
  po_delivered_qty, po_to_be_del) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?) ");

while(($data = fgetcsv($file, 10000, ",")) !== FALSE) {
    $data = fgetcsv($file);
    $stmt->execute($data);
}

I am frequently puzzled why more PHP developers don't use PDO.

PDO got an undeserved reputation as being something exotic, because it wasn't included in default PHP installations way back in the PHP 5.0 days. But ever since PHP 5.1, PDO has been part of the official PHP distribution. It's strange that developers avoid it, even 15 years later!


One thing you need to get into the habit of doing is checking for errors. This is true in PHP and other languages too.

Mysqli functions return FALSE if there's an error. Then you should log the error so you can use that for troubleshooting.

Like this:

$stmt = $mysqli->prepare(...);
if ($stmt === false) {
    error_log($mysqli->error);
    die("Internal error");
}

$ok = $stmt->execute();
if ($ok === false) {
    error_log($stmt->error);
    die("Internal error");
}

You need to check for errors after every call to prepare() or execute(). It's a pain, but otherwise how will you know what went wrong?

Keep a window open tailing your http error log while you are developing and testing code, so you can see the errors if they are logged.

It's a good practice to output a generic error message to the user, because you don't want to reveal too much about your code to the user. But log more technical information to the error log so you can troubleshoot it yourself.

Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • I am new in PHP so i am still learning, but i will read about PDO also, thanks for your suggestion! By the way, i change the code, but it still not work, i have no idea where is my mistake, i have server error 500 now... code updated – Martin M Mar 03 '19 at 00:03
  • "PHP Fatal error: Call to a member function prepare() on null in" my upload.php file... line 16... this line contain my $stmt = $mysqli->prepare line of code – Martin M Mar 03 '19 at 03:43
  • Sorry, your connection is in a variable called `$conn`. Your second code sample has both `$conn` and `$mysqli`. You only need to use one of those, whichever was the variable for the connection that you made with `mysqli_connect()`. It doesn't matter what the variable is named, you just have to use the one that you made for the connection. – Bill Karwin Mar 03 '19 at 03:58
  • And my example of `$pdo->prepare()` is meant to assume that one had previously created a PDO connection and stored it in a variable `$pdo`. Creating a PDO connection is documented here: http://php.net/manual/en/pdo.construct.php Again, the name of the variable is up to you. – Bill Karwin Mar 03 '19 at 03:59
  • It cannot be too easy... i have to learn much more :) Thank you a lot! – Martin M Mar 03 '19 at 04:13