Sanitize boolean types in rest_sanitize_request_arg() #2633

Activity

1. websupporter commented on Aug 2, 2016

   

   websupporter

   on Aug 2, 2016

   Member

   More actions

   Hi @rachelbaker,
   since boolval( 'false' ); would be true I think we would need here also a function, this time to sanitize. This function could then be placed in the rest_sanitize_request_arg(), which is executed after the validation.

   Although we know already the value will be a boolean, I thought, it would be useful to also return a WP_Error in case its not, so the function could also be used in cases, where you can't be sure. So I smashed the function I've suggested in #2616 with your function rest_is_boolean().

   I searched a bit and finally I found, yes, to use strict mode for in_array() can be done for php 5.2, since it was introduced in 4.x (http://php.net/ChangeLog-4.php).

       if ( ! function_exists( 'rest_sanitize_boolean' ) ) {
           function rest_sanitize_boolean( $maybe_bool ) {
               if ( ! rest_is_boolean( $maybe_bool ) ) {
                   return new WP_Error( 'no-bool', __( 'The parameter was no boolean.' ) );
               }
   
               if ( is_string( $maybe_bool ) ) {
                   $maybe_bool = strtolower( $maybe_bool );
               }
   
               $true = array(
                   true,
                   '1',
                   'true',
               );
   
               if ( in_array( $maybe_bool, $true, true ) ) {
                   return true;
               }
   
               return false;           
           }
       }
   

   Sorry for more or less reintroducing my suggestion, but let me quickly do some advertisement for it :)

   For validation, you expect true or false to know if the value is valid. To sanitize a boolean, true or false are the values, if sanitized. But what to return if you do not enter a valid boolean? It can't be false and I do not think it should be nothing. So, I think WP_Error is a good solution.

   What do you think?
2. BE-Webdesign commented on Aug 8, 2016

   

   BE-Webdesign

   on Aug 8, 2016

   Last edited by BE-Webdesign

   Member

   More actions

   I would make the PR without the WP_Error for now, as we will have to wait a while until the check for sanitizing the request args is in core. Then we can open up an issue to keep track of wanting to put that in. Alternatively you could leave it in but commented out with a note saying enable once core can handle this.

   Otherwise, I think that function is good and would need to be called within rest_sanitize_request_arg() in the PR.
3. websupporter commented on Aug 17, 2016

   

   websupporter

   on Aug 17, 2016

   Member

   More actions

   Hi @BE-Webdesign, I think it makes sense right now to wait a bit to be clear on #2630 (e.g. now the (int)1|0). Once we now what is valid, we know what the sanitize function can expect.
4. BE-Webdesign

   added

   Bug

   on Sep 8, 2016
5. BE-Webdesign commented on Sep 17, 2016

   

   BE-Webdesign

   on Sep 17, 2016

   Member

   More actions

   Added in #2704.
6. BE-Webdesign

   closed this as completedon Sep 17, 2016