Make WordPress Core

Opened 4 years ago

Last modified 4 months ago

#56025 new defect (bug)

wp_validate_boolean() not doing what it describes, causes issues with [video] shortcode

Reported by: arnodeleeuw's profile arnodeleeuw Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.0
Component: General Keywords: has-patch dev-feedback
Focuses: Cc:

Description

The function in question:

/**
* Filter/validate a variable as a boolean.
*
* Alternative to `filter_var( $var, FILTER_VALIDATE_BOOLEAN )`.
*
* @since 4.0.0
*
* @param mixed $var Boolean value to validate.
* @return bool Whether the value is validated.
*/
function wp_validate_boolean( $var ) {
    if ( is_bool( $var ) ) {
        return $var;
    }
 
    if ( is_string( $var ) && 'false' === strtolower( $var ) ) {
        return false;
    }
 
    return (bool) $var;
}

Steps to recreate the issue:

Add the following shortcodes to a page:

[video src="YOUR-SOURCE-HERE"]
[video src="YOUR-SOURCE-HERE" loop="off"]
[video src="YOUR-SOURCE-HERE" loop="0"]
[video src="YOUR-SOURCE-HERE" loop="false"]
  • The first shortcode works as intended, rendering a video on the frontend without the loop attribute.
  • The second shortcode's <video> element will have the attribute loop="1" despite the loop attribute being set to off.
  • The third and fourth shortcode's <video> elements will not have the loop attribute set.

Description:

wp_validate_boolean() says it 's an alternative to filter_var( $var, FILTER_VALIDATE_BOOLEAN ) , however
filter_var( $var, FILTER_VALIDATE_BOOLEAN ) will return FALSE when you pass the string "off" whereas wp_validate_boolean() will return TRUE in this case.
(See this for the documentation for filter_validate_boolean: https://www.w3schools.com/php/filter_validate_boolean.asp).

Currently, wp_validate_boolean() only returns FALSE if the $var is a string and if it is === "false", or if the $var passed is the int 0.

This causes unexpected behaviour in the [video] shortcode. The documentation of the video shortcode (https://wordpress.org/support/article/video-shortcode/) mentions that the loop attribute needs to be either "on"/"off". Because they are strings wp_validate_boolean() will return TRUE, causing the loop attribute to always be added with a value of "1" unless you change the value of the attribute to be specifically the string "false", the int 0, or leave it completely empty.

The parsing of the attributes of the [video] shortcode happens here: (https://github.com/WordPress/wordpress-develop/blob/6.0/src/wp-includes/media.php#L3322-L3344).
As you can see on line 3340 it uses wp_validate_boolean() to get filter value of the loop attribute. Passing "off" here returns true, causing the loop attribute to always be added to the eventual HTML output later down the line with a value of "1".

Potential fix:

  • Change the working of wp_validate_boolean() to do as it says in its description. Make it smarter, and thus also return FALSE when the strings "off","OFF","no","NO", etc. are passed just like filter_var( $var, FILTER_VALIDATE_BOOLEAN ) does (See this documentation link: https://www.php.net/manual/en/function.filter-var.php#121263).
  • Change how the parsing of attributes happens in the [video] shortcode so that it also works as the documentation says: not adding certain attributes when they are set to "off". However there may be other instances where wp_validate_boolean() is used that I'm currently not aware of, so this would be more like a band-aid fix and similar issues may arise in the future.

Attachments (1)

56025.diff (493 bytes) - added by karpstrucking 4 years ago.
update wp_validate_boolean() to validate false-y strings

Download all attachments as: .zip

Change History (5)

@karpstrucking
4 years ago

update wp_validate_boolean() to validate false-y strings

#1 @karpstrucking
4 years ago

  • Component changed from Shortcodes to General
  • Keywords has-patch dev-feedback added

hey @arnodeleeuw !

thanks for opening this ticket.

I've uploaded a simple patch that addresses this for "off" and "no".

devs: any value if making this list of false-y strings filterable?

This ticket was mentioned in PR #9670 on WordPress/wordpress-develop by @callumbw95.


5 months ago
#2

Update the wp_validate_boolean() function to work with truthy values.

Trac ticket: #56025

#3 @callumbw95
5 months ago

Hi @karpstrucking,

I have just come across this ticket, and after reviewing the initial "falsey list" approach, I'd like to propose we consider flipping the logic to use a "truthy list" instead. My main concern with the original approach is that it defaults to true for any unrecognised string, which could lead to unexpected behaviour.

Here’s a quick summary of why I think a "truthy list" is a safer and more robust approach:

  • It's Safer by Default: This approach works on an "allow list" principle. We explicitly define what is true. Anything else—including typos, empty values, or unexpected inputs—will safely default to false. This prevents a feature from being accidentally enabled.
  • It's More Explicit: The code becomes self-documenting about what values are considered active or "on". There's no ambiguity.
  • It Avoids Unintended Consequences: With a "falsey list," an input like 'truu' or 'ok' would evaluate to true. The proposed method correctly identifies these as false.

I have put in a PR in with this change now, so please let me know your thoughts. 😃

@mindctrl commented on PR #9670:


4 months ago
#4

@CallumBW95 this PR causes existing unit tests to fail.

1) Tests_Functions_wpValidateBoolean::test_wp_validate_boolean with data set #4 ('false', false)
Failed asserting that true is identical to false.

/var/www/tests/phpunit/tests/functions/wpValidateBoolean.php:24

2) Tests_Functions_wpValidateBoolean::test_wp_validate_boolean with data set #5 ('FalSE', false)
Failed asserting that true is identical to false.

/var/www/tests/phpunit/tests/functions/wpValidateBoolean.php:24

3) Tests_Functions_wpValidateBoolean::test_wp_validate_boolean with data set #6 ('FALSE', false)
Failed asserting that true is identical to false.

/var/www/tests/phpunit/tests/functions/wpValidateBoolean.php:24
Note: See TracTickets for help on using tickets.