WP_REST_Templates_Controller::get_wp_templates_author_text_field() doesn't always return value

[antonvlasenko]

The WP_REST_Templates_Controller::get_wp_templates_author_text_field() method sometimes fails to return a value. Its @return type is specified as a string, so it should consistently return a string.

Possible solutions include returning an empty string or throwing an exception if the $original_source does not equal one of the expected values.

[debarghyabanerjee]

Replying to antonvlasenko:

I have checked the core codebase, and if you check this function, you will find that it will always return a string, which will match atleast one of the expected values/cases.

WP_REST_Templates_Controller::get_wp_templates_original_source_field()

Trac ticket: https://core.trac.wordpress.org/ticket/61580

[antonvlasenko]

Replying to debarghyabanerjee:

I have checked the core codebase, and if you check this function, you will find that it will always return a string, which will be one of the expected values/cases.

WP_REST_Templates_Controller::get_wp_templates_original_source_field()

Thank you for your feedback. You are right. The :: get_wp_templates_author_text_field() method will always return a string if the ::get_wp_templates_original_source_field() method returns one of the expected values: theme, plugin, site, or user.

However, if ::get_wp_templates_original_source_field() method is changed to return unexpected values, it poses a risk.

I suggest that ::get_wp_templates_author_text_field() should return a default value instead of relying on ::get_wp_templates_original_source_field() to return predefined values.
I explored that option in ​https://github.com/WordPress/wordpress-develop/pull/7000.

[hellofromTonya]

Hmm, this is interesting.

WP_REST_Templates_Controller:: get_wp_templates_original_source_field() always returns a string with one of the values used within the switch() within WP_REST_Templates_Controller::get_wp_templates_author_text_field().

But as @antonvlasenko noted, what if either method gets changed? To make the code more future-proof to avoid changes in one method affecting the other, a default of an empty string could be the graceful fall-through, e.g. return '';' at the end of the method.

Moving this into 6.7 for visibility.

I'll also comment on the PR with a suggested change.

Hi @anton-vlasenko,

Did you check this method,

WP_REST_Templates_Controller::get_wp_templates_original_source_field()

This will always return a string which will match atleast one of the cases. Thanks :-)

Hi @anton-vlasenko,

Did you check this method,

WP_REST_Templates_Controller::get_wp_templates_original_source_field()

This will always return a string which will match atleast one of the cases. Thanks :-)

[hellofromTonya]

Patch: ​https://github.com/WordPress/wordpress-develop/pull/7000

Marking this ready for commit and self-assigning for the commit.

[hellofromTonya]

In 58705:

REST API: Ensure string returned in WP_REST_Templates_Controller::get_wp_templates_author_text_field().

Adds a fail-safe to return an empty string should the switch ever fall through without returning.

Currently, WP_REST_Templates_Controller::get_wp_templates_author_text_field() is tightly coupled to WP_REST_Templates_Controller::get_wp_templates_original_source_field(). However, if the $original_source values change in either method, but not both, it is possible a void or null will be returned, rather than a string.

Follow-up to [57366].

Props antonvlasenko, hellofromTonya, debarghyabanerjee.
Fixes #61580.

Committed via https://core.trac.wordpress.org/changeset/58705. Thanks everyone for your contributions

Committed via https://core.trac.wordpress.org/changeset/58705. Thanks everyone for your contributions

Thank you!