Taking advantage of the falsey and ineffective "" empty string

Question

The following code snippets are academic. I was required to make a PHP generated HTML interface for a MySQL database of a movie rental store, and the requirement was filtering the displayed movies by category.

The approach for having the filter persist across all form actions (without using a session) was to have the hidden form input inserted into all the other forms on the page. And my first approach to this was testing if $_POST['category'] is set, and if so echoing the HTML tag for the hidden input to carry the category variable to the next page.

This was still the method, but, I tinkered with it and decided that instead of testing if a category was selected at every form I could test once, and if not leave the string variable for the HTML tag empty and echo it regardless.

This worked nicely but I'm curious if if in a larger/co-operative project if the possible confusion for the outputting of a variable that is sometimes empty (i.e. ineffectual) and sometimes not will be more confusing than the lines of code and processing time saved.

<?php
$categorySelected = '';
$hiddenCategory = '';
////....................
//conect to databes
//create mysqli object
if( isset($_POST['category']) && $_POST['category'] != $Allcats ){
 //<check that catagory still has entries>
 //if there are entries for category filter, set local variable, and html tag for form input
 //hidden input tag is inserted to all forms to carry category selection to next page reload
 if(<videos of that category exist>){
 $categorySelected = $_POST['category'];
 $hiddenCategory = "<input type='hidden' name='category' value='$categorySelected'>";
 } //else //<display message that there are no videos of that category>
}
//database manipulations completed
//html header
//in all forms in html $hiddenCategory is echod
//to add category k/v pair to post
//or '' is echoed with no consequence if category not set
?>
<form id='addvideo' action = <?php echo $SELF; ?> method ='post'>
 <?php echo $hiddenCategory; ?>
 <input type='hidden' name='ACTION' value='addvideo'>
 <input type="submit" value="ADD VIDEO">
 Name:<input type="text" name="name">
 <!-- more fields.... -->
</form>
<?php
//prepare statement for all entries of category filtered
if(!$categorySelected){
 $vidStmt = $mysqli->prepare("SELECT * FROM records");
}else{
 $vidStmt = $mysqli->prepare("SELECT * FROM records WHERE category=?");
 $vidStmt->bind_param("s",$categorySelected);
}
//iterate over all videos in database query, creating <tr> for each
while($vidStmt->fetch()){
echo "<tr>
<td> $name <td> $category <td> $length <td> 
<form class='rent' action = $SELF method ='post'>
 $hiddenCategory
 <input type='hidden' name='ACTION' value='rent'>
 .... more fields ..
</form>
";
}
$vidStmt->close();
?>

Answer 1

I wouldn't do it this way, because it does seem confusing. And not just because you are using truthiness (which I really wouldn't do, because it's hard to understand and just screams for bugs). It takes a while to see what happens when there is no POST value, what happens if a categories doesn't exist, etc.

You don't need multiple checks either though, just move all the code that belongs to category-doesnt-exists and category-exists into one block. If you also extract some code to functions, your code might look something like this:

if(!isset($_POST['category']) || $_POST['category'] == $Allcats ){
 displayAddForm("", $SELF);
 displayRentForm("", getAll());
} else if (<videos of that category doesnt exist>) {
 // message that video doesn't exist
 displayAddForm("", $SELF);
 displayRentForm("", getAll());
} else {
 $categorySelected = $_POST['category'];
 $hiddenCategory = "<input type='hidden' name='category' value='htmlspecialchars($categorySelected, ENT_QUOTES, 'UTF-8') '>";
 displayAddForm($hiddenCategory, $SELF);
 displayRentForm($hiddenCategory, getByCategoryId($categorySelected));
}
function getAll() {
 $mysqli->prepare("SELECT * FROM records");
 // return array of results
}
function getByCategoryId($id) {
 $vidStmt = $mysqli->prepare("SELECT * FROM records WHERE category=?");
 $vidStmt->bind_param("s",$categorySelected);
 // return array of results
}
function displayRentForm($hiddenCategory, $categories) {
 foreach($categories as $category){
 echo "<tr>
 <td> htmlspecialchars($name, ENT_QUOTES, 'UTF-8') $name <td> htmlspecialchars($category, ENT_QUOTES, 'UTF-8') <td> htmlspecialchars($length, ENT_QUOTES, 'UTF-8') <td> 
 <form class='rent' action = htmlspecialchars($SELF, ENT_QUOTES, 'UTF-8') method ='post'>
 $hiddenCategory
 <input type='hidden' name='ACTION' value='rent'>
 .... more fields ..
 </form>
 ";
 }
}
function displayAddForm($hiddenCategory, $SELF) {
 echo "<form id='addvideo' action = <?php echo htmlspecialchars($SELF, ENT_QUOTES, 'UTF-8') ; ?> method ='post'>" .
 $hiddenCategory .
 "<input type='hidden' name='ACTION' value='addvideo'>
 <input type="submit" value="ADD VIDEO">
 Name:<input type="text" name="name">
 <!-- more fields.... -->
 </form>";
}

It's not perfect, but I you get the idea.

XSS

You shouldn't write vulnerable code, even for academic work. It's bad practice, and it normalizes unsecure code; you get used to writing it, and other people get used to reading it.

Always protect against XSS when echoing variable data, eg:

$hiddenCategory = "<input type='hidden' name='category' value='" . htmlspecialchars($categorySelected, ENT_QUOTES, 'UTF-8') . "'>";

And

<form id='addvideo' action = '<?php echo htmlspecialchars($SELF, ENT_QUOTES, 'UTF-8'); ?>' method ='post'>

Misc

• Don't mix " and ' so much. Something like <input type='hidden' name='ACTION' value='addvideo'><input type="submit" value="ADD VIDEO"> is hard to read, and harder to change than uniformly quoted code (you can see this in my code example above, where I didn't bother to correct it).
• Your indentation is off, making your code harder to read.

• Brandon Swanson: Thank you so much for the feedback! and indeed I want to protect against XSS, especially while its still academic, all of the SQL interactions (which aren't seen here) were done with prepared statements, but I had forgot to sanitize my post variables, Thanks to you, I will try not to make that same mistake in the future.