Return type/value of get_site is not being checked in wp_insert_site

[david.binda]

The get_site function may return null in case the site is not found ( see https://core.trac.wordpress.org/browser/trunk/src/wp-includes/ms-site.php?rev=44727#L293 ), otherwise returns a WP_Site object.

The function is being called from inside the wp_insert_site (see https://core.trac.wordpress.org/browser/trunk/src/wp-includes/ms-site.php?rev=44727#L70 ) and the WP_Site object is implicitly expected, as the follow-up code treats the $new_site variable as an object (eg.: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/ms-site.php?rev=44727#L114).

I haven't run into any issues, but it might be worth checking the return type and return WP_Error out of the wp_insert_site instead of 0 in case the call to get_site fails. (the 0 would be an result of null->id, producing null casted to int).

[pento]

Good find, thanks @davidbinda! Putting this in 5.1.1, as it's mostly about better error case handling. If get_site() has failed immediately after the $wpdb->insert() succeeds, there are probably bigger problems. 🙂

[david.binda]

Totally agree! Thank you.

[vanyukov]

return WP_Error on null

[pbiron]

The patch looks good to me, except for the typo in the error message (/retrieves/retrieve/).

[lukecarbis]

Fix typo in error message.

[SergeyBiryukov]

In 44799:

Networks and Sites: Check the result of get_site() in wp_insert_site().

Props davidbinda, vanyukov, lukecarbis.
Fixes #46300.

[SergeyBiryukov]

In 44800:

Networks and Sites: Check the result of get_site() in wp_insert_site().

Props davidbinda, vanyukov, lukecarbis.
Merges [44799] to the 5.1 branch.
Fixes #46300.