Need to get values on drop-down to sort by two different criteria

[Skormy]

On my site I have a drop-down menu that allows visitors to sort a widget by a code, let's say color.  And that has been working fine.  From the same drop-down menu, I would like visitors to be able additionally be able to sort the same widget by the material it is made from.  I have been able to get each to work independently, but when I combine them, I believe it defaults to the "else" condition which just shows everything.  I have tried to use a specific code for each as well as a generic code for both.  The results are the same, displaying all the widgets, not a subset of them.  Everything I've tried so far hasn't given me the results I'm looking for.  Would love some guidance on how to solve this issue.  Thanks.

<FORM NAME="sort" method="POST" action="<?php echo $_SERVER['PHP_SELF']; ?>"> 
<SELECT NAME="productcode" onchange="this.form.submit();">


<?php 
    $sortcode1 = $_POST['sortcode1'];
 ?>
<OPTION VALUE="ALL"<?php if ($sortcode1=="ALL") { print "selected =\"selected\""; }?>>Display All</option> 
<OPTION VALUE="E"<?php if ($sortcode1=="E") { print "selected =\"selected\"";} ?>>Display Only Yellow Widgets</option> 
<OPTION VALUE="P"<?php if ($sortcode1=="P") { print "selected =\"selected\""; }?>>Display Only Purple Widgets</option> 
<?php 
      $sortcode2 = $_POST['sortcode2'];
 ?>
<OPTION VALUE="CM"<?php if ($sortcode2=="CM") { print "selected =\"selected\""; }?>>Display Only Metal Widgets</option>
<OPTION VALUE="FG"<?php if ($sortcode2=="FG") { print "selected =\"selected\""; }?>>Display Only Plastic Widgets</option>

</SELECT>

<noscript>
<input type="submit" name="productcode" value="Display Selected Widgets">
</noscript>


</FORM>

<?php

// database connect stuff

$sortcode1 = $_POST['sortcode1'];

if ($sortcode1 =="ALL")
{
$query = "SELECT stuff1,stuff2,stuff3
	   	 FROM $db_table WHERE status = 'A'
		 order by stuff1 DESC ";
} 

elseif ($sortcode1 == 'E')
    
{ 
 $query = "SELECT stuff1,stuff2,stuff3
	   	 FROM $db_table WHERE status = 'A'
		 AND sortcode1 = '$sortcode1'
		 order by stuff1 DESC " ;
		 
}		  

elseif ($sortcode1 == 'P')
    
{ 
 $query = "SELECT stuff1,stuff2,stuff3
	   	 FROM $db_table WHERE status = 'A'
		 AND sortcode1 = '$sortcode1'
		 order by stuff1 DESC " ;
}		

$sortcode2 = $_POST['sortcode2'];

elseif ($sortcode2 == 'CM')
    
{ 
 $query = "SELECT stuff1,stuff2,stuff3
	   	 FROM $db_table WHERE status = 'A'
		 AND sortcode2 = '$sortcode2'
		 order by stuff1 DESC " ;
}		

elseif ($sortcode2 == 'FG') 
    
{ 
 $query = "SELECT stuff1,stuff2,stuff3
	   	 FROM $db_table WHERE status = 'A'
		 AND sortcode2 = '$sortcode2'
		 order by stuff1 DESC " ; 
}		

else
{
	  		
        $query = "SELECT stuff1,stuff2,stuff3
	   	 FROM $db_table WHERE status = 'A'
		 order by stuff1 DESC ";
}

[ginerjm]

Way too difficult to follow. You need to learn how to write your html code separate from the business logic (the php) so that one can make sense of it.

 

Sort your data before trying to build the html. Then it becomes simply a matter of creating the option tags without worrying about if/else code.

[benanamen]

Your code is all sorts of wrong. I will just point out a couple things.

 

$_SERVER['PHP_SELF'] is vulnerable to an XSS Attack. You need to use $_SERVER['SCRIPT_NAME']

 

It appears you are sending user supplied data directly to the database. That is a huge no-no.

 

You have quite a bit of redundant SQL

[Jacques1]

$_SERVER['PHP_SELF'] is vulnerable to an XSS Attack. You need to use $_SERVER['SCRIPT_NAME']

 

I disagree. The solution to XSS attacks is escaping, not switching to some other parameter which you hope is safe. In this specific case, you can actually omit the action attribute altogether. The default action is always the current resource, so there isn't any need for an explicit URL.

 

Another problem is that the physical script name (as provided by $_SERVER['SCRIPT_NAME']) isn't guaranteed to match the public URL under which the site is accessible. As soon as you implement URL rewriting, physical names become meaningless and will likely break the frontend.

 

So just leave out the action:

<form method="post">
    ...
</form>

[benanamen]

I disagree.

 

To be clear, you do agree $_SERVER['PHP_SELF'] is vulnerable to an XSS Attack right?

 

not switching to some other parameter which you hope is safe.

 

In your experience, have you ever known $_SERVER['SCRIPT_NAME'] to be unsafe in any cases, edge or otherwise?

 

In this specific case

 

That would seem key. What about in the case where an index.php includes all the pages from $_GET?

i.e. index.php?p=contact (Code reference: https://forums.phpfreaks.com/topic/302370-router-any-issues-comments/)

 

Would you advocate always hard coding the index.php name instead of the following:

<form action="<?= $_SERVER['SCRIPT_NAME'] ?>?p=<?= $_GET['p'] ?>" method="post">

 

As soon as you implement URL rewriting, physical names become meaningless and will likely break the frontend.

 

 You specifically mention the frontend. I pretty much only do backends with the previously noted code link so I don't do URL rewriting. Is there any case with the previous style AND Url rewriting that would be a problem?

Edited November 9, 2016 by benanamen

[Jacques1]

To be clear, you do agree $_SERVER['PHP_SELF'] is vulnerable to an XSS Attack right?

 

Yes, potentially.

 

 

 

In your experience, have you ever known $_SERVER['SCRIPT_NAME'] to be unsafe in any cases, edge or otherwise?

 

It doesn't matter. I would never even try to distinguish between “safe” and “unsafe” values:

• Even “safe” values can screw up the application (it's not just about security).
• Just because we cannot think of an exploit doesn't mean the input is in fact safe. I've seen many examples where experienced programmers were absolutely sure that an exploit is impossible, and then they turned out to be wrong (servers are sometimes misconfigured, PHP sometimes has bugs, and we're fallible too).
• Constantly having to decide whether or not you should escape a value is extremely error-prone by itself.
• Code changes all the time. Maybe the input is safe now, but that may no longer be true after the next revision. And I'm pretty sure you don't scan your entire application whenever you make a small change, just to be sure that your assumptions still hold.

I think the only valid approach is to escape every value except the ones that are supposed to be HTML fragments. This is classical whitelisting. What you've proposed is essentially blacklisting.

 

 

 

That would seem key. What about in the case where an index.php includes all the pages from $_GET?

i.e. index.php?p=contact (Code reference: https://forums.phpfreaks.com/topic/302370-router-any-issues-comments/)

 

Would you advocate always hard coding the index.php name instead of the following:

<form action="<?= $_SERVER['SCRIPT_NAME'] ?>?p=<?= $_GET['p'] ?>" method="post">

 

No. I would use relative URLs:

?p=contact

As in

<form action="?p=contact" method="post">

Even better would be properly rewritten URLs where you don't have to add parameters all the time, e. g.

/contact -> index.php?p=contact