3
\$\begingroup\$

Here I am parsing a JSON array and inserting it into a MySQL database. The JSON Array comes from my android code.

This is how my JSON array looks like:

["{custInfo=Ujwal  9975022560, rate=24000, weight=21.00000, desc=GENTS ANGTHI 22k NO STONE, makingAmt=200, vat=RS.3064.38, itemTotal=51073, sum_total=RS.156283.38, barcode=BQSP78BB, net_rate=24200, date=2015-11-30, invoiceNo=1, bill_type=Invoice}",
"{custInfo=Ujwal  9975022560, rate=24000, weight=21.00000, desc=GENTS ANGTHI 22k NO STONE, makingAmt=200, vat=RS.3064.38, itemTotal=51073, sum_total=RS.156283.38, barcode=BQSP78BB, net_rate=24200, date=2015-11-30, invoiceNo=1, bill_type=Invoice}",
"{custInfo=Ujwal  9975022560, rate=24000, weight=21.00000, desc=GENTS ANGTHI 22k NO STONE, makingAmt=200, vat=RS.3064.38, itemTotal=51073, sum_total=RS.156283.38, barcode=BQSP78BB, net_rate=24200, date=2015-11-30, invoiceNo=1, bill_type=Invoice}"]

This is my php code to parse and insert the data.

<?php
require "init.php";
$json = file_get_contents('php://input'); 
//$data = json_decode($json,true);
// remove the ,true so the data is not all converted to arrays
$data = json_decode($json);
// Now process the array of objects
foreach ( $data as $inv ) {
    $custInfo = $inv->custInfo;
    $rate =     $inv->rate;
    $weight=    $inv->weight;
    $desc=      $inv->desc;
    $makingAmt= $inv->makingAmt;
    $vat=       $inv->vat;
    $itemTotal= $inv->itemTotal;
    $sum_total= $inv->sum_total;
    $barcode=   $inv->barcode;
    $net_rate=  $inv->net_rate;
    $date=      $inv->date;
    $invoiceNo= $inv->invoiceNo;
    $bill_type= $inv->bill_type;
    $sql = "INSERT INTO selected_items 
             (custInfo, invoiceNo, barcode, desc, 
              weight, rate, makingAmt,net_rate,
              itemTotal,vat,sum_total,bill_type,date) 
            VALUES
             ('$custInfo','$invoiceNo','$barcode','$desc',
              '$weight','$rate','$makingAmt','$net_rate',
              '$itemTotal','$vat','$sum_total','$bill_type','$date')";
    $res = mysqli_query($sql,$con);
echo $res;
    if(!$res){
        $result = new stdClass();
        $result->status = false;
        $result->msg = mysql_error();
        echo json_encode($result);
        exit;
    }
}
?>
\$\endgroup\$
0

2 Answers 2

5
\$\begingroup\$

You are open to SQL Injection. To prevent this, use prepared statements.

You also should not mix mysql_ and mysqli_ functions; always use mysqli_ (or PDO).

Having variables that are only accessed once is only useful in a limited number of cases, eg when you want to give something a nice name to increase readability. But this is not the case with your variables, so they are really not needed and actually decrease readability. Jut use $inv->custInfo etc directly. This will also severely shorten your code.

\$\endgroup\$
5
\$\begingroup\$

You've got a few things going on there, let's take them one by one:

1. Prevent SQL Injection vulnerability

Using the mysqli library is a step ahead from the (soon to be absolutely deprecated) mysql one, and you can and should make use of prepared statements as they will offer a convenient layer for evading SQL injection attacks (without deeply elaborating on this, this is the practice of inserting malicious code snippets into database queries which would unwantedly expose command upon your database to the attacker).

// Insert question mark and column name for all variables
$SQL = "INSERT INTO selected_items (custInfo, ... ) VALUES (?, ...);";
if ($stmt = $mysqli->prepare($SQL)) {
    // Do binding for all variables
    $stmt->bind_param("s", $custInfo);


    $stmt->execute();
}

The benefit of using built-in prepared statements is utilization of input sanitization which "automatically strips" malicious substrings.

Personal comment: PDO library allows for more versatile data binding, which we could make use of in the below next point.

2. Loopify handling of variables

Depending on where you want to take this, you could set up some kind of a config for "white-listed" (allowed) variables, and based on that loop through your variables.

$allowedVariables = array(
     'custInfo',
     'rate',
     'weight',
     ...
);

If that is in your "config.php", and we'll assume you may use PDO, then we could modify your code to something like:

// init.php containing your PDO definition, along the line of:
// $dbObj = new PDO("mysql:host=<host>;dbname=<dbname>", "<rootname>", "<rootpass>");
require "init.php";
include "config.php";
$json = file_get_contents('php://input'); 

$data = json_decode($json);

try{
    $sql = "INSERT INTO selected_items (". implode(',', $allowedVariables) .") VALUES (". substr(str_repeat('?,', sizeof($allowedVariables)), 0, -1) .");";
    $prepStatement = $dbObj->prepare($sql);

    foreach ( $data as $inv ) {
        $res = $prepStatement->execute(array_values($inv));
    }

}catch(Exception $ex){
    //Error handling goes here
}

If you wished to proceed with mysqli library, something along the following lines could be done:

try{
    $sql = "INSERT INTO selected_items (". implode(',', $allowedVariables) .") VALUES (". substr(str_repeat('?,', sizeof($allowedVariables)), 0, -1) .");";
    $prepStatement = mysqli_prepare($con, $sql);

    // Slight workaround with *call_user_func_array*, as *mysqli_stmt_bind_param* doesn't accept variables to be bound as array, which we will need to dynamically build up the variable list
    $parameterArray = array();
    $variableTypeString = str_repeat("s", sizeof($allowedVariables));
    $parameterArray[] = &$variableTypeString;
    foreach( $allowedVariables AS $varName){
        $parameterArray[] = &$inv[$varName];
    }
    call_user_func_array(array($prepStatement, 'bind_param'), $parameterArray);

    foreach ( $data as $inv ) {
        mysqli_stmt_execute($prepStatement);
}catch(Exception $ex){
    //Error handling goes here
}

}

Note be: although mysqli supports the object-based approach of usage, but I've kept the classic way of calling the methods and personally, PDO library provides a nicer interface for handling your day-to-day DB tasks. I'm not on a LAMP enabled environment right now, so I couldn't test the above code snippets, but should work.

  • Edit: moved statement preparation outside input data loop
\$\endgroup\$
3
  • 1
    \$\begingroup\$ To increase performance, you could take the prepared statement outside the loop, so that it only gets prepared once instead of each time. \$\endgroup\$
    – tim
    Commented Nov 30, 2015 at 16:31
  • \$\begingroup\$ @tim, I was too caught up with the cumbersome mysqli function names, valid point, updated the answer \$\endgroup\$
    – Gabor
    Commented Nov 30, 2015 at 16:33
  • \$\begingroup\$ @Gabor thanks alot can you post the edited code please. \$\endgroup\$ Commented Dec 1, 2015 at 8:19

Not the answer you're looking for? Browse other questions tagged or ask your own question.