Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refactor filter_input usage throughout Stream #257

Closed
frankiejarrett opened this issue Feb 19, 2014 · 8 comments · Fixed by #264
Closed

Refactor filter_input usage throughout Stream #257

frankiejarrett opened this issue Feb 19, 2014 · 8 comments · Fixed by #264
Assignees
Labels

Comments

@frankiejarrett
Copy link
Contributor

Recently there was a discussion around the use of PHP filter_* functions after a user reported having an issue in the WP.org forum.

@westonruter then informed us that for reasons of reliability Nacin doesn't allow these to be used in core. I think we should follow suite in Stream to avoid potential issues and stay in line with core coding standards.

See #254 (comment)

@frankiejarrett
Copy link
Contributor Author

It was suggested by @powelski that perhaps we add a helper function to Stream that does the isset check for us and returns false otherwise. Maybe this would be better than doing it longhand every time.

Could just be something like:

stream_input( $type, $var ) {
    if ( ! is_string( $type ) || ! is_string( $var ) ) {
        return false;
    }

    $result = ( 'INPUT_GET' === $type && isset( $_GET[$var] ) ) ? $_GET[$var] : false;
    $result = ( 'INPUT_POST' === $type && isset( $_POST[$var] ) ) ? $_POST[$var] : false;
    $result = ( 'INPUT_SERVER' === $type && isset( $_SERVER[$var] ) ) ? $_SERVER[$var] : false;

    return $result;
}

@powelski
Copy link

@fjarrett How about making it array-friendly by providing any array as the first argument? So we would call it like:

stream_input( $_GET, 'key', array( 'default_param' => 'default_value' ) );

Where the third argument is optional.

@shadyvb
Copy link
Contributor

shadyvb commented Feb 20, 2014

@powelski I'd suggest we use the same syntax as filter_input family so we minimize the effort needed to change what is written already, and to maintain compatibility in case they come back in new suits in new PHP versions.

@frankiejarrett
Copy link
Contributor Author

+1

@shadyvb shadyvb self-assigned this Feb 20, 2014
@jonathanbardo
Copy link
Contributor

@shadyvb I agree. Should stay with API parity.

@shadyvb
Copy link
Contributor

shadyvb commented Feb 20, 2014

Noticed couple of filter_var's in plugin code , will try to accommodate.

@frankiejarrett
Copy link
Contributor Author

@shadyvb Please create a new issue for the filter_var usage. Sorry I missed your comment and already merged and released the PR.

@shadyvb
Copy link
Contributor

shadyvb commented Feb 25, 2014

@fjarrett I believe filter_var functionality were merged to the master branch already, let me know if something still needs to be done.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants