News:

SMF 2.1.4 has been released! Take it for a spin! Read more.

Main Menu

Can I do This with empty Check

Started by mickjav, March 25, 2023, 12:49:56 PM

Previous topic - Next topic

mickjav

The code below is the original
        if (!empty($_REQUEST['cat']))
            $cat = (int) $_REQUEST['cat'];
        else
            $cat = 0;

        if (empty($cat))
            fatal_error($txt['smflinks_nocatselected'], false);

Can somebody let me know if there is a reason why I can't do the below with the statment.

        if (!empty($_REQUEST['cat']))
            $cat = (int) $_REQUEST['cat'];
        else
            fatal_error($txt['smflinks_nocatselected'], false);

All the best mick
Site Under redesign not expected to be complete before January 2026

Aleksi "Lex" Kilpinen

Really not sure about this, but it looks like the original code tries to make sure that $cat is defined, if empty, maybe to avoid an undefined index error later?
Slava
Ukraini!
"Before you allow people access to your forum, especially in an administrative position, you must be aware that that person can seriously damage your forum. Therefore, you should only allow people that you trust, implicitly, to have such access." -Douglas

How you can help SMF

mickjav

Sorry didn't think I needed to add it but there is also A isset

if (isset($_REQUEST['cat']))
{
if (!empty($_REQUEST['cat']))
$cat = (int) $_REQUEST['cat'];
else
$cat = 0;

if (empty($cat))
fatal_error($txt['smflinks_nocatselected'], false);
Site Under redesign not expected to be complete before January 2026

Arantor

The reason? What happens if someone tries index.php?cat=sometexthere

In the first piece of code, $_REQUEST['cat'] is defined, $cat is set to (int) of that string, which is zero. So when you hit the secondary check, you have weeded out not only that cat is not zero, but that it is also not a non-integer.

In the second piece of code, if fed some text, you will get $cat containing 0 but still passing your test, which will then fail further down the line in a way that you don't want.

Trying to shortcut code is usually a bad idea unless you fully understand what every possible branch can do and whether any possible branch will be triggered by conditions you don't expect.

There is also no actual benefit to keeping the code as lean as you're trying to there, it is not a performance benefit.

mickjav

Thanks @Arantor Just trying to understand how things work

I have tested the updated code below

With changing the Address to Cat=0 and cat=test Both give me the message

if (isset($_REQUEST['cat']))
{
if (!empty($_REQUEST['cat']))
$cat = (int) $_REQUEST['cat'];
else
fatal_error($txt['smflinks_nocatselected'], false);

But will return it to the original

Many thanks for your help mick
Site Under redesign not expected to be complete before January 2026

Arantor

Then it's something else catching it - that code as given will not filter out $cat=test, but I suspect when it goes to look up $cat later on against the relevant table, 0 won't match any rows. But it's still getting further than it should.

mickjav

cool thanks marking solved

Edit Found second check after query, which is why I'm getting same message

if (empty($row['ID_CAT']))
fatal_error($txt['smflinks_nocatselected'], false);
Site Under redesign not expected to be complete before January 2026

Arantor

Yes, you get the same overall outcome - but the extra check before the query means you save the query, which is definitely a performance saving.

Advertisement: