2
\$\begingroup\$
public function get($item) {
    if (isset($_POST[$item]) && $_POST[$item] != '') {
        $_POST[$item] = filter_var($_POST[$item],FILTER_SANITIZE_SPECIAL_CHARS);
        if(is_numeric($_POST[$item])) {
            return (int)$_POST[$item];
        }
        return $_POST[$item];
    }else if (isset($_GET[$item]) && $_GET[$item] != '') {
              $_GET[$item] = filter_var($_GET[$item],FILTER_SANITIZE_SPECIAL_CHARS);
        if(is_numeric($_GET[$item])) {
            return (int)$_GET[$item];
        }
        return $_GET[$item];
    }
    return '';
}

Is there a way I can make this more efficient?

\$\endgroup\$
2
\$\begingroup\$

If you don't mind that it checks more things than just POST and GET, you can use the $_REQUEST superglobal instead:

public function get($item) {
    if ( isset($_REQUEST[$item]) && $_REQUEST[$item] != '' ) {
        $_REQUEST[$item] = filter_var($_REQUEST[$item], FILTER_SANITIZE_SPECIAL_CHARS);
        if ( is_numeric($_REQUEST[$item]) ) {
            return (int)$_REQUEST[$item];
        }
        return $_REQUEST[$item];
    }

    return '';
}

Note that this won't update the $_POST or $_GET superglobals.

Further note that as written, your code will escape $_REQUEST[$item] each time this function is called with $item as the input. I would think that this would be an undesirable behavior. A&B would turn into A&B then A&B, etc.

\$\endgroup\$
1
\$\begingroup\$

I don't know about more 'efficient', but you can make the code cleaner. Here is a small refactor that simplifies the logic so it is more DRY (Don't repeat yourself).

public function get($item) {
    $newItem = '';
    if(isset($_POST[$item]) && $_POST[$item] != '') {
        $newItem = $_POST[$item];
    } else if (isset($_GET[$item] && $_GET[$item] != '') {
        $newItem = $_GET[$item];
    }

    $newItem = sanitizeInput($newItem);
    if(is_numeric($newItem)) {
        return (int)$newItem;
    } else {
        return $newItem;
    }

    return '';

}

function sanitizeInput($input) {
    $cleanInput = '';
    if($input != '') {
        $cleanInput = filter_var($input, FILTER_SANITIZE_SPECIAL_CHARS);
    }

    return $cleanInput;
}

Also, I removed the pieces of your code that were modifying the actual POST or GET variable. Your method is called get, so it should do just that and not produce any side effects. By moving the sanitization into another function, you make it easier to add additional sanitation steps in the future. Additionally the encapsulation provided by the additional method allows you to call it from other places, which is a likely eventuality when validating user input (think cookies, file upload, http headers).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.