csv data import

[freecrm]

I've just had a user issue relating to my data import function.

The function imports from a csv file.

As part of the data prep, I have

$logo = str_replace('"', '', $data[3]); 

to get rid of the double quotes.

This users issue was that some of his data has single quotes, which was breaking out of the loop and throwing a MySQL error.

Simple question...

Do I simply add another str_replace to get rid of the single quotes or will this impact on the delimiters within the csv file?

[misson]

Sounds like you have a security leak. Perform better cleanup on the fields before insertion (e.g. use mysql_real_escape_string()) and you won't have the problem.

Do I simply add another str_replace to get rid of the single quotes or will this impact on the delimiters within the csv file?

No. You want to preserve the users' input; they won't like it when characters go missing. For instance, a user should be able to store a name like "O'Connor".

[freecrm]

Arrrghh....

I'm getting confused here and found out I don't really understand this down to the roots!

Just for reference, I have a test page at www.freecrm.x10hosting.com/test.php

This is pretty much what I have

function format_db_string($string)
        {
           $string = str_replace('"', '', $string);//to get rid of double quotes
           $string = trim($string);//to get rid of spaces and carriage returns
            if (!get_magic_quotes_gpc())// to make safe
               {
            $string = addslashes($string);
               }
            return $string;
        }
        


$test=$_POST['textarea'];
echo "Raw Data = ".$test."<br/>";

$safe=format_db_string($test);
echo "Safe = ".$safe."<br/>"; 

Why isn't this working and what exaclty is mysql_real_escape_string??

The array is split by exploding on \n and then exploding on ",".

[misson]

Why isn't this working

One problem is that magic quotes (except for magic_quotes_sybase) will escape double quotes. You remove the double quotes but not the escape characters.

Other than that, what isn't working? State what you expect and what you get.

and what exaclty is mysql_real_escape_string??

If you ever want to know what a PHP function is, look it up in the PHP manual.

The array is split by exploding on \n and then exploding on ",".

Instead of writing your own CSV parser, use the PHP fgetcsv function.

[freecrm]

Thanks Misson - helpful as always.

I have now done some serious reading up on form security and now understand the reasoning behind malicious injections.

Fortunately, the majority of my site is not at risk.

I also understand that addslashes is not a favourable route (not quite sure why), so I have changed the code to

if (get_magic_quotes_gpc())// check if active
               { 
            $string = stripslashes($string); //remove escapes
               } 

I also now partially get the mysql_real_escape_string, although the php manual isn't the best of places to understand something in plain english!

I'm now looking at incorporating the fgetcsv function... and will keep you posted.

[misson]

I also understand that addslashes is not a favourable route (not quite sure why),

mysql_real_escape_string escapes characters that addslashes doesn't because it targets characters special in MySQL. If you're not using MySQL, mysql_real_escape_string would be less desirable. Furthermore, addslashes may have problems with some multibyte encodings, while mysql_real_escape_string takes into account the encoding used for the DB connection.

The safest approach would be to use PDO prepared statements, but PDO isn't enabled on the free servers for some reason.

[freecrm]

Now I think I'm getting somewhere...

again, this page is at www.freecrm.x10hosting.com/test.php

I have tested it with single and double quotes and \\.

Before I start coding in the MySQL insert query, I just wanted to check I'm on the right lines.

I have commented all the way through.

<form action="" method="post" enctype="multipart/form-data" name="form1" id="form1">
<p>
<input type="file" name="uploadedfile" />
</p>
<p>
<input type="submit" name="Submit" value="Submit" />
<input name="issubmitted" type="hidden" id="issubmitted" value="y" />
</p>
</form>
<?php

function make_safe($string){//create function

$string = str_replace('"', '', $string);//to get rid of double quotes 
$string = trim($string);//to get rid of spaces and carriage returns 
if (get_magic_quotes_gpc())// if system setting magic quotes is on
{ 
$string = stripslashes($string);//strip out escape slashes 
}
mysql_real_escape_string($string);// target all other special characters
return $string; //produce result
}

if ($_POST['issubmitted'] == "y"){//check if submitted

$row = 1;//set row as first

$handle = fopen($_FILES['uploadedfile']['tmp_name'], "r");//open read only connection

$data = fgetcsv($handle);// get data from open file

while ($data !== FALSE) {//while there is data..

$num = count($data);//count data fields in row
echo "<br/><br/>$num fields in line $row: <br />\n";// print fields in row
$row++;// increase row number
for ($i=0; $i < $num; $i++) { //loop through index from 0

echo $data[$i] . " : ";//echo field according to index or insert into database
}

$data = fgetcsv($handle);//get data from open file whilst looping
}
fclose($handle);// close open file connection

}
?>

Is there a way around having to duplicate the $data = fgetcsv($handle); ?

[misson]

...
function make_safe($string){//create function
    $string = str_replace('"', '', $string);//to get rid of double quotes 

mysql_real_escape_string() will sanitize any double quotes in the input so you don't need to remove them. Is there another reason why you're removing them?

for ($i=0; $i < $num; $i++) { //loop through index from 0
    echo $data[$i] . " : ";//echo field according to index or insert into database
} 

Make sure you call make_safe on each field from the CSV file before you insert them. I'm assuming you're not making the call here because you're just echoing the data.

For the echo, a single implode(" : ", $data) call would be better than the loop.

Is there a way around having to duplicate the $data = fgetcsv($handle); ?

Yes, but many of the alternatives (gotos [available in PHP >= 5.3], exceptions) are worse in my opinion. You could use a for loop, which will at least bring the calls to fgetcsv() next to each other:

for ($data = fgetcsv($handle); $data !== FALSE; $data = fgetcsv($handle)) {
    ...
} 

You could also break out of the loop when $data is false:

while (TRUE) {
    $data = fgetcsv($handle);
    if ($data === FALSE) {
        break;
    }
    ...
} 

Before you pick a particular approach, make sure you have a good reason. The three choices will impact readability much more than anything else.

[freecrm]

Thanks

I've now completed a data preview page (with make_safe even on echo) and tested it. The data is then put into <td>'s and it works well with all sorts of nasty data!

The final part is the insert itself and I can't get my head around inserting from the array.

//create function to make db safe    
function make_safe($string){

$string=strip_tags($string);// strip out tags
if (get_magic_quotes_gpc())// if system setting magic quotes is on
{ 
$string = stripslashes($string);//strip out escape slashes 
}
mysql_real_escape_string($string);// sanitise string
return $string; //produce result
}

$handle = fopen($new_file_path, "r");//open read only connection

$data = fgetcsv($handle);// get data from open file

//start row count
$row=1;

while ($data !== FALSE) {//while there is data..

$fields = count($data);//count data fields in row

for ($i=0; $i < $fields; $i++) { //loop through fields from 0    

//define column values and sanitise
$groupcode = $_SESSION['MM_GroupCode'];
$memid = createRandomString();
$username = createRandomString();
$level = "Unvalidated";
$password = createRandomString();
$createdwhen = time();
$createdby = $_SESSION['MM_Username'];
$editedwhen = time();
$editedby = $_SESSION['MM_Username'];
$accno = make_safe($data[0]);
$accman = make_safe($data[1]);
$company = make_safe($data[2]);
$logo = make_safe($data[3]);
$title = make_safe($data[4]);
$firstname = make_safe($data[5]);
$lastname = make_safe($data[6]);
$dear = make_safe($data[7]);
$position = make_safe($data[8]);
$dept = make_safe($data[9]);
$add1 = make_safe($data[10]);
$add2 = make_safe($data[11]);
$add3 = make_safe($data[12]);
$city = make_safe($data[13]);
$county = make_safe($data[14]);
$postcode = make_safe($data[15]);
$country = make_safe($data[16]);
$homeadd1 = make_safe($data[17]);
$homeadd2 = make_safe($data[18]);
$homeadd3 = make_safe($data[19]);
$homecity = make_safe($data[20]);
$homecounty = make_safe($data[21]);
$homepostcode = make_safe($data[22]);
$homecountry = make_safe($data[23]);
$switchboard = make_safe($data[24]);
$directtel = make_safe($data[25]);
$fax = make_safe($data[26]);
$mobile = make_safe($data[27]);
$hometel = make_safe($data[28]);
$email = make_safe($data[29]);
$homeemail = make_safe($data[30]);
$website = make_safe($data[31]);
$umbcomp = make_safe($data[32]);
$subsidiaries = make_safe($data[33]);
$brands = make_safe($data[34]);
$tps = make_safe($data[35]);
$fps = make_safe($data[36]);
$mps = make_safe($data[37]);
$thirdpp = make_safe($data[38]);
$newsp = make_safe($data[39]);
$socgrade = make_safe($data[40]);
$acorndem = make_safe($data[41]);
$usertz = "UTC";
$usertf = "Y-m-d H:i:s";
$info = make_safe($data[42]);
} //close loop
      
//insert into db
$insert = "INSERT INTO CONTACTS (GROUPCODE, MEMID, USERNAME, LEVEL, PASSWORD, CREATEDWHEN, CREATEDBY, EDITEDWHEN, EDITEDBY, ACCNO, ACCMAN, COMPANY, LOGO, TITLE, FIRSTNAME, LASTNAME, DEAR, POSITION, DEPT, ADD1, ADD2, ADD3, CITY, COUNTY, POSTCODE, COUNTRY, HOMEADD1, HOMEADD2, HOMEADD3, HOMECITY, HOMECOUNTY, HOMEPOSTCODE, HOMECOUNTRY, SWITCHBOARD, DIRECTTEL, FAX, MOBILE, HOMETEL, EMAIL, HOMEEMAIL, WEBSITE, UMBCOMP, SUBSIDIARIES, BRANDS, TPS, FPS, MPS, 3RDPP, NEWSP, SOCGRADE, ACORNDEM, USERTZ, USERTF, INFO)
VALUES ('$groupcode', '$memid', '$username', '$level', '$password', '$createdwhen', '$createdby', '$editedwhen', '$editedby', '$accno', '$accman', '$company', '$logo', '$title', '$firstname', '$lastname', '$dear', '$position', '$dept', '$add1', '$add2', '$add3', '$city', '$county', '$postcode', '$country', '$homeadd1', '$homeadd2', '$homeadd3', '$homecity', '$homecounty', '$homepostcode', '$homecountry', '$switchboard', '$directtel', '$fax', '$mobile', '$hometel', '$email', '$homeemail', '$website', '$umbcomp', '$subsidiaries', '$brands', '$tps', '$fps', '$mps', '$thirdpp', '$newsp', '$socgrade', '$acorndem', '$usertz', '$usertf', '$info');";

mysql_query($insert) OR die(mysql_error());

echo "Record ".$row." successfully imported with ".$fields." fields.<br>";


$row++;// increase row number        
$data = fgetcsv($handle);//get data from open file whilst looping
}
fclose($handle);// close open file connection 

.. but I'm not even going to test this because it won't work!!! Arrghh.

On the $row loop, I was trying to define each field value, in preparation for one insert statement, but thinking about it, this is defining every value in the row within a field loop!

But I need to create a loop to access the field data in the row array (data['$i'])

This probably needs a small amendment but I'm getting tired and can't figure it out.

Help??

[misson]

On the $row loop, I was trying to define each field value, in preparation for one insert statement, but thinking about it, this is defining every value in the row within a field loop!

But I need to create a loop to access the field data in the row array (data['$i'])

So you're having problems defining the mapping of the CSV fields to the database fields? What's generating the CSV file? Could you add a header that defines the fields?

If you find the need to create innumerable variables, you probably want an associative array. If you need to set the variables in a loop, you definitely need an associative array. Of course, you're mapping CSV fields to DB fields based solely on index, so you don't need all those variables. Try:

//create function to make db safe     
function make_safe($string){
    if (get_magic_quotes_gpc())// if system setting magic quotes is on 
    {  
        $string = stripslashes($string);//strip out escape slashes  
    } 
    return mysql_real_escape_string(strip_tags($string));// sanitise string 
} 

$handle = fopen($new_file_path, "r");//open read only connection 

$groupcode = $_SESSION['MM_GroupCode']; 
$level = "Unvalidated"; 
$createdby = $_SESSION['MM_Username']; 
$editedby = $_SESSION['MM_Username'];
$usertz = "UTC"; 
$usertf = "Y-m-d H:i:s";
$memid = createRandomString();
$username = createRandomString(); 
$password = createRandomString();

for ($row=1, $data = fgetcsv($handle);
       $data !== FALSE;
       ++$row, $data = fgetcsv($handle)) 
{ //while there is data.. 
    $numFields = count($data);//count data fields in row 
    $createdwhen = time(); 
    $editedwhen = time();
    $data = array_map('make_safe', $data);
       
//insert into db 
    $insert = "INSERT INTO CONTACTS (GROUPCODE, MEMID, USERNAME, LEVEL, PASSWORD, 
            CREATEDWHEN, CREATEDBY, EDITEDWHEN, EDITEDBY, USERTZ, USERTF, 
            ACCNO, ACCMAN, COMPANY, LOGO, TITLE, FIRSTNAME, LASTNAME, DEAR, POSITION, DEPT, 
            ADD1, ADD2, ADD3, CITY, COUNTY, POSTCODE, COUNTRY, 
            HOMEADD1, HOMEADD2, HOMEADD3, HOMECITY, HOMECOUNTY, HOMEPOSTCODE, HOMECOUNTRY, 
            SWITCHBOARD, DIRECTTEL, FAX, MOBILE, HOMETEL, EMAIL, HOMEEMAIL, WEBSITE, 
            UMBCOMP, SUBSIDIARIES, BRANDS, TPS, FPS, MPS, 3RDPP, NEWSP, SOCGRADE, ACORNDEM, INFO) 
        VALUES ('$groupcode', '$memid', '$username', '$level', '$password', 
            '$createdwhen', '$createdby', '$editedwhen', '$editedby', '$usertz', '$usertf', '"
            . implode("', '", $data) . "');"; 

    mysql_query($insert) OR die(mysql_error()); 

    echo "Record ".$row." successfully imported with ".$numFields." fields.<br>"; 
} 

fclose($handle);// close open file connection 

In the above code, the order of the fields is implicit in the INSERT statement. If you want to allow the 1st line of the CSV file to define the field order, try:

//create function to make db safe     
function make_safe($string){ 
    if (get_magic_quotes_gpc())// if system setting magic quotes is on 
    {  
        $string = stripslashes($string);//strip out escape slashes  
    } 
    return mysql_real_escape_string(strip_tags($string));// sanitise string 
} 

// return true iff arrays are permutations of each other
function is_permutation($arr1, $arr2) {
    return !(count(array_diff($arr1, $arr2)) 
             || count(array_diff($arr1, $arr2))
            );
}

$handle = fopen($new_file_path, "r");//open read only connection 

$groupcode = $_SESSION['MM_GroupCode']; 
$level = "Unvalidated"; 
$createdby = $_SESSION['MM_Username']; 
$editedby = $_SESSION['MM_Username'];
$usertz = "UTC"; 
$usertf = "Y-m-d H:i:s";
$memid = createRandomString();
$username = createRandomString(); 
$password = createRandomString();

// order of fields in $fields is same as order of fields in CSV file
$fields = array('ACCNO', 'ACCMAN', 'COMPANY', 'LOGO', 'TITLE', 
    'FIRSTNAME', 'LASTNAME', 'DEAR', 'POSITION', 'DEPT', 
    'ADD1', 'ADD2', 'ADD3', 'CITY', 'COUNTY', 'POSTCODE', 'COUNTRY', 
    'HOMEADD1', 'HOMEADD2', 'HOMEADD3', 'HOMECITY', 'HOMECOUNTY', 'HOMEPOSTCODE', 'HOMECOUNTRY', 
    'SWITCHBOARD', 'DIRECTTEL', 'FAX', 'MOBILE', 'HOMETEL', 'EMAIL', 'HOMEEMAIL', 'WEBSITE', 
    'UMBCOMP', 'SUBSIDIARIES', 'BRANDS', 'TPS', 'FPS', 'MPS', '3RDPP', 'NEWSP', 'SOCGRADE', 'ACORNDEM', 'INFO');
// get 1st line
$data = fgetcsv($handle);

/* Test if 1st line is a permutation of $fields.  If so, it defines the field order 
for the file.  If you want to allow some fields to remain unset, use:
  !count(array_diff($data, $fields))
instead of "is_permutation".  If you want to allow only some fields to remain unset, 
put the mandatory fields in $mandatory and test:
  !(count(array_diff($data, $fields)) || count(array_diff($mandatory, $data)))
*/
if ($data !== FALSE && is_permutation($data, $fields)) 
{
    // 1st line of CSV file is a list of field names that defines field order
    $fields=$data;
    $data = fgetcsv($handle);    
}
$numFields = count($fields);
$csvFields = implode(', ', $fields);
$row=1;
while ($data !== FALSE) {//while there is data.. 
    $fieldCount = count($data);//count data fields in row 
    if ($fieldCount == $numFields) {
        $createdwhen = time(); 
        $editedwhen = time();
        $data = array_map('make_safe', $data);
        //insert into db 
        $insert = "INSERT INTO CONTACTS (GROUPCODE, MEMID, USERNAME, LEVEL, PASSWORD, 
            CREATEDWHEN, CREATEDBY, EDITEDWHEN, EDITEDBY, USERTZ, USERTF, 
            $csvFields) 
          VALUES ('$groupcode', '$memid', '$username', '$level', '$password', 
            '$createdwhen', '$createdby', '$editedwhen', '$editedby', '$usertz', '$usertf', '"
            . implode("', '", $data) . "');"; 

        if (mysql_query($insert)) {
            echo "Record ".$row." successfully imported with ".$fieldCount." fields.<br/>"; 
        } else {
            echo "Failed to insert record $row: ", mysql_error(),".<br/>\n";
        }
    } else {
        echo "Record $row is too short: should have $numFields fields but only has $fieldCount.<br/>\n";
    }
    ++$row;
    $data = fgetcsv($handle);
} 

fclose($handle);// close open file connection 

If you wanted to use associative arrays, try $data = array_combine($fields, array_map('make_safe', $data)).

Also, please make sure the code you post is indent by block level. It's much easier to read.