Remove redundant isset checks when using Wakka::GetSafeVar

[GeorgePetsagourakis]

Many times through the code the developer witnesses:

if (isset($_POST['action']) && ($this->GetSafeVar('action', 'post') == 'updatepass'))

All those should turn into:

if ($this->GetSafeVar('action', 'post') == 'updatepass')

Since GetSafeVar is checking for the isset.

Related tickets

• #995 Remove redundant isset checks in Wakka::GetSafeVar()

[BrianKoontz]

(In [1672]). Replaced raw references to $_POST/$_GET with GetSafeVar() calls where applicable. Refs. #989.

I've done a cursory check on each affected action/handler, but this ticket could use some extra vetting...

[KrzysztofTrybowski]

What is more, if GetSafeVar would accept a third parameter — a default value (instead of NULL), then we could also simplify things like:

$sort = DEFAULT_SORT_ROUTINE;
		
if (isset($_GET['sort']))
{
	$sort = $this->GetSafeVar('sort', 'get');
}

and we could replace it with:

$sort = $this->GetSafeVar('sort', 'get', DEFAULT_SORT_ROUTINE);

How's that?

[BrianKoontz]

This interface change would be best in a new enhancement ticket for milestone 1.4...

[GeorgePetsagourakis]

Replying to BrianKoontz:

This interface change would be best in a new enhancement ticket for milestone 1.4...

In fact the implementation of the function is easy:

function GetSafeVar($varname, $gpc='get', $default=NULL)
{
    if($gpc == 'get' || $gpc == 'post' || $gpc=='cookie')
    {
        $gpc = strtoupper('_'.$gpc);
        global $$gpc;
        return (isset(${$gpc}[$varname])) ? $this->htmlspecialchars_ent(${$gpc}[$varname]) : $default;
    }
    return $default;
}

[BrianKoontz]

George, I'm aware of how to make the change. There are probably a thousand "easy" fixes that can be made to the codebase for the 1.3 release. The problem is that each "easy" fix adds up timewise, and also increases the potential for breaking something. In addition, I never view interface changes to the Wakka.lib.php functions as "easy" or "trivial," because complacency in this regard can lead to unforeseen problems down the road.

Thanks for the code...I can assure you that if it's put into a 1.4 ticket, it will be reviewed again at that time. If it remains in this ticket, there are no guarantees I will remember that it's here.

[BrianKoontz]

(In [1686]) The body of the page cannot be read using GetSafeVar() (otherwise, important markup characters will be converted to HTML entities and not be processed by the formatter). This is OK, as the formatter will sanitize the output after processing the markup. Refs #989.

[DotMG]

(In [1710]) refs #989 : little correction for [1672]

[DotMG]

(In [1711]) refs #989 : little enhancement for [1672]

[BrianKoontz]

Changed milestone to 1.3.1