WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#30238 closed enhancement (fixed)

Treat 'FALSE' as false in wp_validate_boolean()

Reported by: TobiasBg Owned by: boonebgorges
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.0
Component: General Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

In [28542] for #28170, we introduced wp_validate_boolean() to allow converting the string 'false' into a boolean false.

Let's also allow uppercase ('FALSE') and mixed-case ('False') versions of the string, which could for example be coming from a Shortcode parameter, like [foo bar=FALSE /].

Attachments (3)

30238.diff (618 bytes) - added by TobiasBg 4 years ago.
30238.2.diff (639 bytes) - added by TobiasBg 4 years ago.
30238.2-tests.diff (2.0 KB) - added by TobiasBg 4 years ago.

Download all attachments as: .zip

Change History (9)

@TobiasBg
4 years ago

#1 @kitchin
4 years ago

From the patch: if ( 'false' === strtolower( $var ) ) {

But strtolower() always returns a string so === is redundant. A non-scalar $var (like an object) might cause an error or warning. So I'd suggest: if ( is_string( $var ) && 'false' == strtolower( $var ) ) {

@TobiasBg
4 years ago

#2 @TobiasBg
4 years ago

Nice catch, kitchin! Thanks!

30238.2.diff is a new patch with an is_string() check. I'd stick with the === check though, as it's cleaner and shows that we know that we are dealing with strings at this point.

#3 @boonebgorges
4 years ago

  • Keywords needs-unit-tests added

Oof. This whole function needs unit tests :)

#4 @TobiasBg
4 years ago

30238.2-tests.diff adds unit tests as well.

#5 @boonebgorges
4 years ago

In 30206:

Add unit tests for wp_validate_boolean().

Props TobiasBg.
See #30238.

#6 @boonebgorges
4 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 30207:

Ignore case when checking string 'false' in wp_validate_boolean().

Props TobiasBg, kitchin.
Fixes #30238.

Note: See TracTickets for help on using tickets.