wp_parse_str() improperly casts boolean querystring values as strings

[r-a-y]

If you use a querystring in wp_parse_args(), wp_parse_str() is used.

Let's say we do something like this:

$r = wp_parse_args( 'this_should_be_bool_false=false&this_should_be_bool_true=true', array() );

$r['this_should_be_bool_false'] equals the string 'false' instead of boolean false.

Attached patch uses wp_validate_boolean() to properly cast string values of 'true' or 'false' to their boolean equivalents and includes a unit test.

This may be a wontfix since it's more of a PHP issue with parse_str(), but I thought I'd post this anyway.

[dd32]

Although the change makes sense on the surface, what you're really doing here is literally passing a string value of 'false', which is a completely acceptable string to be used here.

In many themes and even some places in core in the past, we've used bool_attribute=0 to designate false-y.

This change unfortunately has the downside that it could cause a bug in anything that expects the string "false" as a value.

[r-a-y]

Totally agree. Like I said, it's probably a wontfix, but I thought I'd post it and see what everyone's thoughts were.

[nacin]

Yeah, I don't think we should change this. But it's good to have documented here in a ticket.

I'd prefer to discourage query string usage. At the same time I'd also like to not break it. And this will surely break something that someone is doing somewhere. wp_parse_args() is an ubiquitous, low-level function.

Thanks for including a unit test! Perhaps we should instead add a unit test that expects this to produce the string 'false', and link to this ticket, in order to enforce the design decision here. That way it's documented in an even better way.

[voldemortensen]

Patch adds unit tests to make sure that 'true' or 'false' passed in query args are checked as strings.

[SergeyBiryukov]

In 31310:

Add a unit test that expects wp_parse_args() to treat 'true' and 'false' in a query string as strings.

props voldemortensen for initial patch.
see #30753.

[SergeyBiryukov]

Closing as wontfix, per comment:3.

[SergeyBiryukov]

In 31311:

Remove redundant parameter.

see #30753.