4

I know there are multiple questions here on SO regarding this same issue already and I've looked into them but didn't quite get a satisfying answer. So here goes my question,

I have a form which consists of a few textboxes and checkboxes. It looks like this,

enter image description here

The user can select multiple checkboxes. I'm trying to insert the values(not the displaying text string) of those checkboxes into a MySQL table. It should look like this,

enter image description here

One Service ID(SID) can have multiple Locations(Loc_Code). Those location codes (CO, GQ) are the values of the checkboxes.

I've written this following code so far.

<html>
<head>
</head>
<body>
<?php
require_once("db_handler.php");

$conn = iniCon();
$db = selectDB($conn);

/* Generating the new ServiceID */
$query = "SELECT SID FROM taxi_services ORDER BY SID DESC LIMIT 1";
$result = mysql_query($query, $conn);
$row = mysql_fetch_array($result);
$last_id = $row["SID"];

$id_letter = substr($last_id, 0, 1);
$id_num = substr($last_id, 1) + 1;
$id_num = str_pad($id_num, 3, "0", STR_PAD_LEFT);
$new_id = $id_letter . $id_num;

//Selecting locations
$query = "SELECT Loc_Code, Name FROM districts";
$result = mysql_query($query, $conn);

$count = mysql_num_rows($result);
?>

<?php
if(isset($_POST["savebtn"]))
{
    //inserting the new service information
    $id = $_POST["sid"];
    $name = $_POST["name"];
    $cost = $_POST["cost"];
    if($_POST["active"] == "on") $active = 1; else $active = 0;

    $query = "INSERT INTO taxi_services(SID, Name, Cost, Active) VALUES('$id', '$name', '$cost', '$active')";
    $result = mysql_query($query, $conn);

    //inserting the location details
    for($j = 0; $j < $count; $j++)
    {
        $loc_id = $_POST["checkbox2"][$j];
        $query = "INSERT INTO service_locations(SID, Loc_Code) VALUES('$id', '$loc_id')";
        $result5 = mysql_query($query, $conn);
    }

    if (!$result || !$result5)
    {
        die("Error " . mysql_error());
    }
    else
    {
?>
        <script type="text/javascript">
            alert("Record added successfully!");
        </script>
<?php
    }   
    mysql_close($conn); 
}
?>

<div id="serv">
<b>Enter a new taxi service</b>
<br/><br/>
    <form name="servForm" action="<?php $PHP_SELF; ?>" method="post" >
        <table width="300" border="0">
          <tr>
            <td>Service ID</td>
            <td><input type="text" name="sid" readonly="readonly" value="<?php echo $new_id; ?>" style="text-align:right" /></td>
          </tr>
          <tr>
            <td>Name</td>
            <td><input type="text" name="name" style="text-align:right" /></td>
          </tr>
          <tr>
            <td>Cost</td>
            <td><input type="text" name="cost" style="text-align:right" onkeypress="return isNumberKey(event)" /></td>
          </tr>
          <tr>
            <td>Active</td>
            <td><input type="checkbox" name="active" /></td>
          </tr>
        </table>
</div>

<div id="choseLoc">
Locations <br/><br/>
    <table border="0">
        <?php
        $a = 0;
        while($row = mysql_fetch_array($result))
            {
            if($a++ %5 == 0) echo "<tr>";
            ?>
            <td align="center"><input type="checkbox" name="checkbox2[]" value="<?php echo $row['Loc_Code']; ?>" /></td>
            <td style="text-align:left"><?php echo $row["Name"]; ?></td>
        <?php
        if($a %5 == 0) echo "</tr>";
            }
        ?>
    </table>
</div>
<br/>
<div id="buttons">
    <input type="reset" value="Clear" /> <input type="submit" value="Save" name="savebtn" />
    </form>
</div>

</body>
</html>

It inserts the Service details correctly. But when it inserts location data, a problem like this occurs,

enter image description here

I selected 4 checkboxes and saved. The 4 location codes gets saved along with the service ID. But as you can see from the screenshot above, a bunch of empty rows gets inserted too.

My question is how can I stop this from happening? How can I insert the data from the checkboxes only I select?

Thank you.

Isuru
  • 30,617
  • 60
  • 187
  • 303
  • 5
    **Your code is vulnerable to SQL injection.** You *really* should be using [prepared statements](http://stackoverflow.com/a/60496/623041), into which you pass your variables as parameters that do not get evaluated for SQL. If you don't know what I'm talking about, or how to fix it, read the story of [Bobby Tables](http://stackoverflow.com/questions/332365/xkcd-sql-injection-please-explain). – eggyal Aug 25 '12 at 12:59
  • 5
    Also, as stated in [the introduction](http://www.php.net/manual/en/intro.mysql.php) to the PHP manual chapter on the `mysql_*` functions: *This extension is not recommended for writing new code. Instead, either the [mysqli](http://www.php.net/manual/en/book.mysqli.php) or [PDO_MySQL](http://www.php.net/manual/en/ref.pdo-mysql.php) extension should be used. See also the [MySQL API Overview](http://www.php.net/manual/en/mysqlinfo.api.choosing.php) for further help while choosing a MySQL API.* – eggyal Aug 25 '12 at 13:00
  • Have you tried checking the `$_POST` variable to check what gets submitted? – Madara's Ghost Aug 25 '12 at 13:02
  • @eggyal: Ah yes, I'm aware of SQL injection. But this is just a small front end for a database which's only user is me. Its not online. But thanks for the heads up :) – Isuru Aug 25 '12 at 13:03
  • @Truth: you mean the `$_POST` at this line? `$loc_id = $_POST["checkbox2"][$j];` – Isuru Aug 25 '12 at 13:04
  • @nK0de: I mean the entire $_POST array. `var_dump($_POST)` will help you see what gets submitted. – Madara's Ghost Aug 25 '12 at 13:05

3 Answers3

10

One way would be to only loop over the checkboxes that were submitted:

//inserting the location details
foreach($_POST["checkbox2"] as $loc_id)
{
  $query = "INSERT INTO service_locations(SID, Loc_Code) VALUES('$id', '$loc_id')";
  $result5 = mysql_query($query, $conn);
}

I reiterate here the SQL injection warning given above: you would be much better off preparing an INSERT statement and then executing it with parameters. Using PDO, it would look something like:

//inserting the location details
$stmt = $dbh->prepare('
  INSERT INTO service_locations(SID, Loc_Code) VALUES(:id, :loc)
');
$stmt->bindValue(':id', $id);
$stmt->bindParam(':loc', $loc_id);
foreach($_POST["checkbox2"] as $loc_id) $stmt->execute();
eggyal
  • 122,705
  • 18
  • 212
  • 237
  • Works beautifully. :) Can you please explain this one line? `foreach($_POST["checkbox2"] as $loc_id)` How `$loc_id` gets the value of the checkbox? | I'm currently studying PDO :) When I get the hang of it, I will be sure to use it when I'm working with PHP and MySQL. Thank you. – Isuru Aug 25 '12 at 13:20
  • @nK0de: Read about [`foreach`](http://php.net/manual/en/control-structures.foreach.php). – eggyal Aug 25 '12 at 13:27
0

from these sentence:

for($j = 0; $j < $count; $j++)
    {
        $loc_id = $_POST["checkbox2"][$j];
        $query = "INSERT INTO service_locations(SID, Loc_Code) VALUES('$id', '$loc_id')";
        $result5 = mysql_query($query, $conn);
    }

i find the problem is that the value of loc_code must be the last loction you selected. because in this loop, the value of loc_code will replaced everytime. if you want to insert all the location, you should put it on the one sentence, like INSERT INTO service_locations(SID, Loc_Code) VALUES('$id', '$loc_id'), the value of $loc_id should be CO,GQ,GL.

leoyfm
  • 241
  • 4
  • 16
-5

This is happening because the checkboxes that weren't ticked still get posted, they just have empty values. Before you do your insert to service_locations just check if $loc_id is empty or not, only do the insert if it isn't.

Tieran
  • 976
  • 6
  • 9