Automattic / woocommerce-subscriptions-core

Subscriptions core package for WooCommerce
Other
81 stars 29 forks source link

PHP 8.1 error when saving edit subscription schedule metabox #485

Open james-allan opened 11 months ago

james-allan commented 11 months ago

Describe the bug

I haven't been able to replicate this myself however creating a subscription via admin on PHP 8.1+ can lead to the following error:

CRITICAL Uncaught TypeError: gmdate(): Argument #2 ($timestamp) must be of type ?int, string given in /public_html/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/admin/meta-boxes/class-wcs-meta-box-schedule.php:94
Stack trace:
#0 /public_html/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/admin/meta-boxes/class-wcs-meta-box-schedule.php(94): gmdate()
#1 /public_html/wp-includes/class-wp-hook.php(308): WCS_Meta_Box_Schedule::save()
#2 /public_html/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters()
#3 /public_html/wp-includes/plugin.php(517): WP_Hook->do_action()
#4 /public_html/wp-content/plugins/woocommerce/includes/admin/class-wc-admin-meta-boxes.php(258): do_action()
#5 /public_html/wp-includes/class-wp-hook.php(310): WC_Admin_Meta_Boxes->save_meta_boxes()
#6 /public_html/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters()
#7 /public_html/wp-includes/plugin.php(517): WP_Hook->do_action()
#8 /public_html/wp-includes/post.php(4715): do_action()
#9 /public_html/wp-includes/post.php(4817): wp_insert_post()
#10 /public_html/wp-admin/includes/post.php(439): wp_update_post()
#11 /public_html/wp-admin/post.php(227): edit_post()
#12 {main}
  thrown in /public_html/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/admin/meta-boxes/class-wcs-meta-box-schedule.php on line 94

In my testing, the $datetime value is always a string because it is pulled out of $POST but it only throws an error if the value is an empty string ''. '0' or '1690353856' doesn't appear to throw an error. 🤷‍♂️

In any case, it makes sense that we protect against this error by making sure the value we pass to gmdate() is an int.

To Reproduce

I have been able to reproduce this error natively yet.

Expected behavior

gmdate() expects an int as its second param and it will throw errors on the latest PHP version.

Actual behavior

Under some condition it's possible to get an empty string passed to this function.