You are using prepared SQL statements, but not using parameterised queries in them, so you're still wide open to SQL injections. This is how you should be doing it:
$gid = $_GET['gid'];
$queryr = $pdo->prepare('SELECT test3.name FROM test1, test2, test3 WHERE test1.id = test2.groupid AND test3.id = test2.peopleid AND test1.id = :gid ORDER BY test3.name ASC');
$queryr->bindParam(':gid', $gid, PDO::PARAM_STR);
$queryr->execute();
Assuming that your $gid
is supposed to be numeric, you should probably extract it this way too:
if(is_numeric($_GET['gid'])) {
$gid = (int)$_GET['gid'];
// ...
$queryr->bindParam(':gid', $gid, PDO::PARAM_INT);
The same applies to your later queries that are being generated from $_POST
.
This segment also seems to be missing a set of braces. If it works, then it's likely only a coincidence. if
without braces is hard enough to read in just PHP, but if you drop out of PHP, I honestly have no idea how PHP will react.
<?php foreach ($queryl as $i => $rowl) { ?>
<li>
<?php if ($i)?> <!-- **** BRACES MISSING HERE? **** -->
<input name="checkbox_del[]" id="test_<?php echo $i ?>" type="checkbox" value="<? echo $rowl['id']; ?>"/>
<label for="test_<?php echo $i ?>">
<a href="test1.php?gid=<?php echo $rowl['id']; ?>"><?php echo $rowl['name']; ?></a>
</label>
</li>
<?php } ?>
There is a larger problem with this script in general. Mixing logic with the output can cause design problems. Ideally you would load all your data at the top, then generate your page all at once. I don't know how big a system this is, but in general it's much easier to manage the system as a whole if you structure your pages this way. What happens if you want to conditionally show something at the top of the page, but the condition isn't generated till later when you are processing data?