mysql2date() return empty string not false sometimes added doing it wrong message

[pbearne]

If you pass a empty date string or bad date to mysql2date() we get a empty string back

e.g.

mysql2date( 'Y-m-d', "" )

mysql2date( 'Y-m-d', 'i am not a date' )

see the unit tests for more examples

I have created patch to return false if it not going to return a date as this is safer i belive

[pbearne]

mysql2date.patch with UnitTest and function patch

[pbearne]

removed commented code and added doc block with ticket no

[aaroncampbell]

The inline documentation for mysql2date was updated in [29344] as part of #28310, and two unit tests were added.

My initial thought was that I didn't want to change the return in any case, but instead just document them. Having said that, I don't like how inconsistent this is:

mysql2date( 'Y-m-d', '' ); // false - expected

mysql2date( 'U', 'not a date' ); // false - expected
mysql2date( 'G', 'not a date' ); // false - expected
mysql2date( 'Y-m-d', 'not a date' ); // "2015-10-04" (current date)?!

mysql2date( 'U', '2012-12-30' ); // 1356825600 - expected
mysql2date( 'G', '2012-12-30' ); // 1356825600 - expected
mysql2date( 'Y-m-d', '2012-12-30' ); // "2012-12-30" - expected

mysql2date( 'U', '1' ); // false - expected
mysql2date( 'G', '1' ); // false - expected
mysql2date( 'Y-m-d', '1' ); // "2015-10-04" (current date)?!

I don't like how "1" and "not a date" return false if the format string is U or G, and current date for any other format string. Does anyone have any particularly compelling reasons for having dates that strtotime doesn't understand return false vs empty string or anything else?

[aaroncampbell]

Sorry, I left out an important bit:
An invalid date, with a format string other than U or G has returned the current date for a very long time with this function (pre WordPress 1.0).

[pbearne]

There were 13 failures when incorrect usage notice added

1) Tests_WP_Date_Query::test_validate_date_values_should_process_array_value_for_year
Unexpected incorrect usage notice for mysql2date

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:278
/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:84

2) Tests_WP_Date_Query::test_validate_date_values_should_process_array_value_for_day
Unexpected incorrect usage notice for mysql2date

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:278
/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:84

3) Tests_WP_Date_Query::test_validate_date_values_should_process_array_value_for_day_when_values_are_invalid
Unexpected incorrect usage notice for mysql2date

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:278
/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:84

4) Tests_Query_Conditionals::test_search_feed
Unexpected incorrect usage notice for mysql2date

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:278
/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:84

5) Tests_Query_Conditionals::test_category_feed
Unexpected incorrect usage notice for mysql2date

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:278
/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:84

6) Tests_Query_Conditionals::test_tag_feed
Unexpected incorrect usage notice for mysql2date

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:278
/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:84

7) Tests_Query_Conditionals::test_author_feed
Unexpected incorrect usage notice for mysql2date

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:278
/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:84

8) Tests_Query_VerbosePageRules::test_search_feed
Unexpected incorrect usage notice for mysql2date

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:278
/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:84

9) Tests_Query_VerbosePageRules::test_category_feed
Unexpected incorrect usage notice for mysql2date

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:278
/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:84

10) Tests_Query_VerbosePageRules::test_tag_feed
Unexpected incorrect usage notice for mysql2date

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:278
/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:84

11) Tests_Query_VerbosePageRules::test_author_feed
Unexpected incorrect usage notice for mysql2date

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:278
/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:84

12) Tests_XMLRPC_wp_newPost::test_invalid_post_date_does_not_fatal
Unexpected incorrect usage notice for mysql2date

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:278
/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:84

13) Tests_XMLRPC_wp_newPost::test_invalid_post_date_gmt_does_not_fatal
Unexpected incorrect usage notice for mysql2date

[pbearne]

Added a _doing_it_wrong() call to mysql2date so we can at a later time change it to a return false if a bad date is passed instead of the current state where an empty string can returned

Added $this->setExpectedIncorrectUsage( 'mysql2date' ); to all the test cases that where failing after I added the do it wrong

Moved two already existing tests for mysql2date() from post to the new test class

This function should fixed in a later version

[pbearne]

complete patch for mysql2date() and Unit tests adding _doing_it_wrong call to warning about bad output

[Rarst]

Ok, added a patch with fresh look in unit tests and revamp for the function.

My thinking is this function is only ever meant to eat MySQL formatted times. As with many other date/time functions it will accept other stuff, but in this case you are definitely on your own if you are doing strange things with it.

For same reason I dropped doing it wrong, since I don't think this warrants special handling and messaging.

I focused unit tests accordingly and made false return for any input that date_create() couldn't deal with.

I also re-documented it, since documentation was really just making confusing claims about what it does. While G and U were supposed to be somehow different timestamp return, in practice code produced exactly same return for them.

[pbearne]

the Patch looks good just happy to see it moving

[SergeyBiryukov]

In 45908:

Date/Time: Revamp mysql2date() to use wp_date() and handle invalid input in a consistent manner.

Add unit tests, improve documentation.

Props Rarst, pbearne.
Fixes #28992.

[SergeyBiryukov]

2 failures in XML-RPC tests:

1) Tests_XMLRPC_wp_newPost::test_invalid_post_date_does_not_fatal
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'1970-01-01 00:00:00'
+'2019-08-29 05:13:46'
/var/www/tests/phpunit/tests/xmlrpc/wp/newPost.php:358
2) Tests_XMLRPC_wp_newPost::test_invalid_post_date_gmt_does_not_fatal
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'1970-01-01 00:00:00'
+'0000-00-00 00:00:00'
/var/www/tests/phpunit/tests/xmlrpc/wp/newPost.php:375

The first result is for invalid_date (with underscore), the second is for invalid date (with a space).

Are these expected?

[Rarst]

Ugh. I did change behavior on invalid input, the problem is function behaves differently on different invalid input.

I'll look at the test and probably restore the behavior (keeping the new implementation though, because it does solve other issues). I've been thinking I need to restore old behavior in localization as well.

[Rarst]

Ok, so the old behavior is compound failure really:

1. strtotime() fails and returns false.
2. date() receives false instead of timestamp, but it gets cast to 0

This causes XML RPC to create post with invalid date as 1970-01-01

However this isn't normal for creating WP posts, they would default to current date without date input.

So making mysql2date fail consistently makes XMLRPC behavior consistent with "normal" post creation.

Since the failing tests were originally meant to test for error, not specifically to codify zero time as correct outcome, I suggest we update the tests.

[SergeyBiryukov]

Replying to SergeyBiryukov:

The first result is for invalid_date (with underscore), the second is for invalid date (with a space).

Ah, looks like it's not that. It's because the first test is for post_date and the second one is for post_date_gmt, which is handled differently if an invalid date is passed.

The resulting post data before [45908]:

post_date: 1970-01-01 00:00:00
post_date_gmt: 1970-01-01 00:00:00

After [45908]:

post_date: 2019-08-29 05:13:46
post_date_gmt: 0000-00-00 00:00:00

There's some logic in convert_date_gmt()​ to deal with the latter case.

Still, as the tests were originally meant to check for the lack of a fatal error, not specifically mark a particular outcome as the correct one, I agree with updating the tests.

[SergeyBiryukov]

In 45911:

Date/Time: Update XML-RPC tests for invalid date input to account for a more consistent mysql2date() error handling in [45908].

The tests were meant to check for the lack of a fatal error in case of invalid date input, not specifically mark a particular outcome as the correct one.

Props Rarst.
Fixes #28992.

[johnbillion]

In 51950:

Date/Time: Improve the docblocks for various date and time related functions.

See #53399, #28992, #40653