Populating a mysql table from user-uploaded data

[lurtnowski]

On my site, I have

					<form class="float-end" action="materials/upload_materials_engine.php" enctype="multipart/form-data" method="POST">
					<input class="form-control form-control-lg me-5" id="formFileLg" name="Materials" type="file" accept=".csv" style='width:18em; display:inline'>
					<button class="btn btn-primary btn-lg" type="submit"><span class="icon-upload-alt"></span>&nbsp;&nbsp;Upload</button>
					</form>

I’mtrying to allow people to upload a csv file to insert materials. This s what I ave imy php page, is this good so far?

<?php
session_start();
$connect = mysqli_connect("localhost","root","aat");

echo '<pre>';print_r($_POST);echo '</pre>';


	if($_SERVER["REQUEST_METHOD"] == "POST") {
		
	  $handle = fopen($_FILES['Materials']['tmpP_name'],'r');
	  while($date = fgetcsv($handle))  {
 		  $material_name = mysqli_real_escape_string($connect, $data[0]);
 		  $material_type = mysqli_real_escape_string($connect, $data[1]);
 		  $material_height = mysqli_real_escape_string($connect, $data[2]);
 		  $material_manufacturer = mysqli_real_escape_string($connect, $data[3]);
 		  $material_model = mysqli_real_escape_string($connect, $data[4]);
		
		  $sql = "INSERT INTO materials 
		  (name,type,height,anufacturer,model)
		  VALUES
		  ('".$material_name."','".$material_type."',".$material_height.",'".$material_manufacturer."','".$material_model."')";
		  
		  echo $sql."<br>";
		  mysqli_query($connect, $sql);
	  }
			//die();
	fclose($handle);

	header("location: upload_materials_success.php");

	}

?>

Heres an example csv file
materials.csv (1.3 KB)

[SamA74]

@lurtnowski

is this good so far?

I’m afraid not.
I would recommend using prepared statements for this. Firstly, you can do away with mysqli_real_escape_string() which no one does anymore and insert user submitted data safely by letting SQL know what the query structure will be, before anyone can inject anything nasty into it.
Secondly when inserting within a (while) loop it’s more efficient to prepare just onece before the loop, then execute within the loop, swapping the data to the next row each time.
And please make the move to PDO also, you won’t regret it.

[lurtnowski]

ok, thanks…ill give it a shot

[mabismad]

@lurtnowski

allow people

What people, ones you trust or just anyone? Do you have user authentication and user permissions systems?

Data submitted to your site can come from anywhere, not just your form, can be set to anything, and cannot be trusted. You must have error handling and validation for all data submitted to your site.

You must -

1. test if the upload worked before referencing any of the uploaded file information. you must test if the $_FILES array is not empty (it will be empty if the total size of the form data exceeds the post_max_size setting) and that the [‘error’] element is a zero - UPLOAD_ERR_OK
2. there’s no guarantee that the data is in a proper csv format or that there aren’t empty lines in the file. you must test that the fgetcsv() call returned data and there are the correct number of fields present in each line.
3. if any field must match a particular data type or format, you must validate the data.
4. since you did not set the character set (at all) to match your database tables when you made the database connection, character conversion can occur in the data and more importantly, the mysqli_real_escape_string() calls can fail to protect against sql special characters in a value from being able to break the sql query syntax, which is how sql injection is accomplished. you need to set the character set to match your database tables when you make the connection.
5. as already stated, you should use a prepared query, prepared once before the start of any looping, then simply supply each set of tested and validated data when you executed the query.
6. if there’s a chance of duplicate data, you must handle that, either by ignoring the duplicate, updating any changed values instead, or handling and displaying/logging the duplicate.

[lurtnowski]

is this better?

<?php
session_start();

include 'db/pdo_conn.php'; 

	if($_SERVER["REQUEST_METHOD"] == "POST") {
		
	  $handle = fopen($_FILES['Materials']['tmpP_name'],'r');
	  
	  $sql = "INSERT INTO materials (name,type,height,anufacturer,model) VALUES (:material_name,:material_type,:material_height,:material_manufacturer,:material_model)";

		while($date = fgetcsv($handle))  {
		  
		  $statement = $pdo->prepare($sql);

		  $statement->execute([
			':material_name' => $data[0],
			':material_type' => $data[1],
			':material_height' => $data[2],
			':material_manufacturer' => $data[3],
			':material_model' => $data[4]
		   ]);

	  }

	fclose($handle);

	header("location: upload_materials_success.php");

	}

?>

Ill have a login system, so only an admin can upload the csv
would his work?

if(!file_exists($_FILES['Materials']['tmp_name']) || !is_uploaded_file($_FILES['Materials']['tmp_name'])) 
    {
        echo 'No upload';
    }   
    else
   {
     echo 'upload';
   }

then put the whille loop in the else?
I can test fgetcsv() to make sure everything is good?

[SamA74]

@lurtnowski

		while($date = fgetcsv($handle))  {
		  
		  $statement = $pdo->prepare($sql);

Prepare before the loop, that way it only has to happen once and can be re-used multiple times.

[lurtnowski]

it worked

I thing though
Is there a way to ignore the top line in the csv file

[rpkamp]

Yes, just call fgetcsv and ignore the result:

fgetcsv($handle); // read and ignore header
while ($date = fgetcsv($handle))  { // read actual data
   // work with $date
}