WP_Date_Query - broken hour-validation

[ChriCo]

It seems, there is still an error in the implementation of #25834:

$min_max_checks['hour'] = array( 
 	'min' => 1, 
 	'max' => 23 
); 

This means, that 0:00 o'clock (midnight) is invalid. Shouldn't the 'min' for 'hour'-validation 0?

[ChriCo]

SergeyBiryukov
You're right. Go ahead and do so. :-)

[tyxla]

Updating test_validate_date_values_hour() unit test to support 0 as a valid hour value. Removing the string from invalid value checks, as a string will evaluate as 0, which is now actually a valid value.

[boonebgorges]

ChriCo - Thanks for the report. And tyxla - thanks for the test update, though in this case we can't just remove the string check; the fact that it's failing suggests a further bug. We can work around the 0/string issue by checking is_numeric() separately. Fix coming up.

[boonebgorges]

In 31251:

WP_Date_Query date validation should not fail for hour = 0.

Props ChriCo, tyxla.
Fixes #31067 for trunk.

[boonebgorges]

Reopening for 4.1.1.

[ChriCo]

Why adding an is_numeric-Check here, when already one is there ?

Maybe we should also consider, if its a good way to move some checks out of the class and to the formattings.php. Something like this:

/**
 * Verifies that the given value is a valid hour.
 *
 * @since 4.1.1
 *
 * @param int|string $value
 * @return bool true|false
*/
function is_hour( $value ) {
	
	if ( ! is_numeric( $value ) ) {
		return false;
	}

	if ( $value < 0 || $value > 23 ) {
		return false;
	}

	return true;
}

same for: year, day, dayofweek, dayofyear, month, minute, second and week.

Week and dayofyear needs a second param "year" for a strict check.

[boonebgorges]

Why adding an is_numeric-Check here, when already one is there ?

The one you linked to only checks the 'year'. Perhaps the 'year' one is no longer needed, but this needs testing.

Maybe we should also consider, if its a good way to move some checks out of the class and to the formattings.php.

It seems a little bit verbose to have separate functions for all of this - it's not easy to see how they'd be used anywhere else in WordPress itself - but it might be worth considering having a single date validation function. If you think this is worth pursuing, please feel free to open an enhancement ticket.

[ChriCo]

The one you linked to only checks the 'year'. Perhaps the 'year' one is no longer needed, but this needs testing.

Yep you're right, so the year can be removed in my second link.

It seems a little bit verbose to have separate functions for all of this - it's not easy to see how they'd be used anywhere else in WordPress itself - but it might be worth considering having a single date validation function. If you think this is worth pursuing, please feel free to open an enhancement ticket.

Yes and no. This "function" was a simple example for extracting the validation to a reusable component which can be tested with Unit Tests. The best thing would be a Validation-/Sanitizing-API which can be used in other situations too.

The hour, day, month, ... is nothing else than a "is_between"-check:

function is_between( $value, $args ) {
	
	$default_args = array(
		'min'		=> 0,
		'max'		=> PHP_INT_MAX,
		'inclusive	=> TRUE
	);	
	
	$args = wp_parse_args( $args, $default_args );
	
	if ( (bool) $args[ 'inclusive' ] ) {
		if ( $args[ 'min' ] > $value || $value > $args[ 'max' ] ) {
			return false;
		}
	} else {
		if ( $args[ 'min' ] >= $value || $value >= $args[ 'max' ] ) {
			return false;
		}
	}

	return true;
}

The current "validation"-implementation in WP_Date_Query is way to complex and are just special validations for date-queries.

[dd32]

In 31377:

WP_Date_Query date validation should not fail for hour = 0.

Props ChriCo, tyxla.
Merges [31251] to the 4.1 branch.
Fixes #31067.