WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 3 years ago

#28992 new defect (bug)

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

Reported by: pbearne Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.8
Component: Date/Time Keywords: needs-testing dev-feedback has-patch has-unit-tests
Focuses: Cc:

Description

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

Attachments (3)

mysql2date.patch (4.0 KB) - added by pbearne 4 years ago.
mysql2date.patch with UnitTest and function patch
mysql2date.2.patch (3.0 KB) - added by pbearne 4 years ago.
removed commented code and added doc block with ticket no
28992.patch (9.5 KB) - added by pbearne 3 years ago.
complete patch for mysql2date() and Unit tests adding _doing_it_wrong call to warning about bad output

Download all attachments as: .zip

Change History (16)

@pbearne
4 years ago

mysql2date.patch with UnitTest and function patch

#1 @pbearne
4 years ago

  • Keywords has-patch needs-testing added

@pbearne
4 years ago

removed commented code and added doc block with ticket no

This ticket was mentioned in IRC in #wordpress-dev by pbearne. View the logs.


4 years ago

This ticket was mentioned in IRC in #wordpress-dev by pbearne. View the logs.


4 years ago

This ticket was mentioned in IRC in #wordpress-dev by pbearne. View the logs.


4 years ago

This ticket was mentioned in IRC in #wordpress-dev by pbearne. View the logs.


4 years ago

#6 @tellyworth
4 years ago

  • Version changed from trunk to 3.8

#7 @welcher
3 years ago

  • Keywords dev-feedback added

#8 @aaroncampbell
3 years ago

  • Keywords needs-refresh added; has-patch removed
  • Milestone changed from Awaiting Review to Future Release

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?

#9 @aaroncampbell
3 years ago

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).

#10 @pbearne
3 years ago

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
Last edited 3 years ago by SergeyBiryukov (previous) (diff)

#11 @pbearne
3 years ago

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
3 years ago

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

#12 @pbearne
3 years ago

  • Keywords has-patch has-unit-tests added; needs-refresh removed

#13 @pbearne
3 years ago

  • Summary changed from mysql2date() return empty string not false to mysql2date() return empty string not false sometimes added doing it wrong message
Note: See TracTickets for help on using tickets.