How to process multiple html inserts?

Question

I'm trying to create multiple HTML inserts in the same form so I can quickly insert multiple lines into my database to save time. However I'm not really sure how to process this.

<form action="admin1.php" method="post">
<?php
 function multiform($x){
    for ($x = 0; $x < 3; $x++){
        echo 'Episode: <input type="number" name="Episode[]">
        Date: <input type="date" name="Date[]">
        Guest: <input type="text" name="Guest[]">
        Type: <input type="text" name="Type[]">
        Youtube:<input type="text" name="Youtube[]"> MP3: <input type="text" name="MP3[]"> iTunes:<input type="text" name="Itunes[]"><br/><br/>';
    }
}

multiform(0);

?>
<input type="submit" value="Submit Form" name="submitForm">
</form>

This is what I tried to use:

$con = mysqli_connect("server","root","","database");

function multiformpost($x) {    
    for ($x = 0; $x < 3; $x++)  {

        $Episode = $_POST['Episode'][$x];
        $Date = $_POST['Date'][$x]; 
        $Guest = $_POST['Guest'][$x];
        $Type = $_POST['Type'][$x]; 
        $Youtube = $_POST['Youtube'][$x];
        $MP3 = $_POST['MP3'][$x];
        $Itunes = $_POST['Itunes'][$x];  

        $sql = "INSERT INTO podcasts(Episode, Date, Guest, Type, Youtube, MP3, Itunes) VALUES ('{$Episode}', '{$Date}', '{$Guest}', '{$Type}', '{$Youtube}', '{$MP3}', '{$Itunes}')";
    }
    if (mysqli_connect_errno()) {
    echo "Failed to connect to MySQL: " . mysqli_connect_error();

    if (!mysqli_query($con, $sql)) {
    die ('Error: ' . mysqli_error($con));
    }
    echo "Added to database";
    }
}

multiformpost(0);

mysqli_close($con);

Which simply returns a blank screen.. I know it's wrong but I'm not entirely sure why.

Lovely SQL injection attack vulnerabilities. Plus why are you passing `0` into your `multiform()` function, only to ignore/overwrite it in your `for()` loop anyways? — Marc B, Mar 10 '14 at 16:57
You're overwriting `$sql` every time through the loop, so when you call `mysqli_query` you just have the last one. You need to do the queries inside the loop. — Barmar, Mar 10 '14 at 17:00
Why are you calling `mysqli_connect_errno()` when you haven't called `mysqli_connect()`? — Barmar, Mar 10 '14 at 17:01
It has been called, my mistake for leaving it out (edited first post). — SkillSet12345, Mar 10 '14 at 17:08
The queries are now inside the loop but I still get a blank page. — SkillSet12345, Mar 10 '14 at 17:09
I'll worry about SQL injection after I can it to work in the first place :(. I've passed 0 in my function because when i leave it empty I get a 'missing argument' error. — SkillSet12345, Mar 10 '14 at 17:10
@FynnAB This is a scary comment - "I'll worry about SQL injection after I can it to work in the first place" The security of your application shoule be thought about up front. It should become second nature for you to code in a secure manner. Not doing so, we lead to lots of rework, as you may find you need to refactor your code to make it secure. — Mike Brant, Mar 10 '14 at 17:24
I meant I'm just coding stuff to learn, not planning to release this publicly. I understand what you mean and take it on board. — SkillSet12345, Mar 10 '14 at 17:31

Mike Brant · Accepted Answer · 2014-03-10T17:23:08.137

You need to be building up the VALUES section of your SQL in a loop and then executing a single query. So something like this:

$con = mysqli_connect("","","","");
if (mysqli_connect_errno()) {
    echo "Failed to connect to MySQL: " . mysqli_connect_error();
}
multiformpost($con);
mysqli_close($con);

function multiformpost($db) {
    if(empty($db) {
        throw new Exception('You need to pass a valid mysqli connection to this method');
    }

    $sql = "INSERT INTO podcasts(Episode, Date, Guest, Type, Youtube, MP3, Itunes) VALUES ";
    $size = count($_POST['Episode']);
    for ($x = 0; $x < $size; $x++)  {

        $Episode = mysqli_real_escape_string($db,$_POST['Episode'][$x]);
        $Date = mysqli_real_escape_string($db,$_POST['Date'][$x]); 
        $Guest = mysqli_real_escape_string($db,$_POST['Guest'][$x]);
        $Type = mysqli_real_escape_string($db,$_POST['Type'][$x]); 
        $Youtube = mysqli_real_escape_string($db,$_POST['Youtube'][$x]);
        $MP3 = mysqli_real_escape_string($db,$_POST['MP3'][$x]);
        $Itunes = mysqli_real_escape_string($db,$_POST['Itunes'][$x]);  

        $sql .= "('{$Episode}', '{$Date}', '{$Guest}', '{$Type}', '{$Youtube}', '{$MP3}', '{$Itunes}'),";
    }
    $sql = rtrim($sql,',');

    if (!mysqli_query($db, $sql)) {
        die ('Error: ' . mysqli_error($db));
    } 
    echo "Added to database";
}

Note that I also made the following changes which I also suggest:

I pass in DB connection to the function. I have no idea what your original parameter was being used for, since you can detect the array size of the POST arrays directly in the function. You would be even better served moving to object-oriented mysqli usage (as you could then verify an instantiate mysqli object was passed to the function), but I didn't make that change here.
I differentiated the use of $con (for global scope) and $db (for local sope in function) so that you do not confuse the two. Previously, your code referenced $con inside function scope without declaring global so that variable would not have even been available. This dependency injection approach is highly recommended as opposed to using global.
I moved DB connection error checking outside the function
I added string escaping to mitigate against SQL injection.
I moved all your global script elements together, as functions typically should not be inserted in the middle of procedural code like you have done, as that make the code more difficult to follow.

How to process multiple html inserts?

1 Answers1