XML-RPC: wp.getUsersBlogs, enforce arg string requirements

[josephscott]

https://core.trac.wordpress.org/ticket/65600

AI assistance: Yes
Tool(s): Claude
Model(s): Opus 4.8
Used for: Writing the unit tests

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props josephscott, westonruter.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[westonruter]

🤖 AI-generated code review. Produced by Claude Code using the Claude Opus 5 (1M context) model (claude-opus-5[1m]), run by @westonruter. Findings were verified empirically against a local checkout (guard temporarily disabled, tests re-run) rather than by inspection alone; the verification notes are inline below. Please treat it as input to human review, not a substitute for it.

---

Overview

Adds a type guard to wp_xmlrpc_server::wp_getUsersBlogs() rejecting non-string username/password with IXR_Error( 400, … ), plus a new test file with a 4-case data provider.

Verification performed:

• The bug is real: with the guard removed, wp_getUsersBlogs( array( 'user', array() ) ) throws TypeError: trim(): Argument #1 ($string) must be of type string, array given from wp_authenticate() (src/wp-includes/pluggable.php:690). The guard fixes it.
• composer lint clean; PHPStan reports no errors on the changed lines; the new tests pass 4/4.

Issues

1. The fix is one layer too low — every other authenticated method still fatals

Confirmed empirically: wp_getPosts( array( 1, 'user', array() ) ) throws the identical TypeError. So do the other authenticated methods, because they all funnel through wp_xmlrpc_server::login(), which passes straight to wp_authenticate().

Guarding inside login() fixes all of them in one place, and needs no new translatable string:

if ( ! is_string( $username ) || ! is_string( $password ) ) {
	$this->error       = new IXR_Error( 403, __( 'Incorrect username or password.' ) );
	$this->auth_failed = true;
	return false;
}

If the narrow scope is deliberate for this ticket, it's worth stating that on Trac so the broader case isn't lost.

2. BC break: integer credentials that authenticate today start returning 400

Confirmed: with the guard removed, a user whose password is 12345 authenticates successfully via wp_getUsersBlogs( array( 'numuser', 12345 ) ). With the guard it returns a 400. IXR_Message parses <int> / <i4> / <double> / <boolean> into PHP scalars, so any client that types a numeric password as <int> regresses.

Only arrays and objects cause the fatal — scalars coerce fine (trim( 12345 ) → "12345"). is_scalar() would fix the crash with no behavior change for existing working clients:

if ( ! is_scalar( $args[0] ) || ! is_scalar( $args[1] ) ) {

The test explicitly pins the int rejection ('an integer as password'), so this reads as a deliberate choice — it should be called out on the ticket rather than left implicit.

3. Objects are the untested fatal trigger, and they're reachable from the wire

class-IXR-message.php:194 turns <dateTime.iso8601> into an IXR_Date object, so an object is not a synthetic case — a client can put one in the password slot. trim( $object ) is the same TypeError. The data provider covers arrays and an int but no object; adding a new stdClass (or IXR_Date) case would cover the other half of the actual crash surface.

4. Test docblock overstates the array-username case

Verified: array( array(), '12345' ) does not fatal. sanitize_user() → wp_strip_all_tags() already guards non-scalars, so it emits an E_USER_WARNING and falls through to a normal 403. Only the password fatals. The docblock — "instead of triggering a fatal error" — is accurate for the password only. Hardening the username is still correct; just worth rewording.

Style / conventions

Measured against the sibling tests/phpunit/tests/xmlrpc/demo/addTwoNumbers.php, which uses the same error-code and message pattern:

• Assert the message, not just the code. minimum_args() also returns 400 ('Insufficient arguments…'), so assertSame( 400, $result->code ) alone doesn't prove which branch fired. addTwoNumbers asserts $result->message too — same here.
• Missing @covers wp_xmlrpc_server::wp_getUsersBlogs class-level annotation (the sibling has it).
• Provider return type: the sibling uses public function data_valid_integers(): array. Consider public static function data_non_string_credentials(): array — static is PHPUnit 10-ready (core currently has ~196 static vs ~959 non-static providers, so either passes review today).
• Double space in '…method. Requires two strings.' — inconsistent with the existing '…method. Requires two integers.' in addTwoNumbers().
• make_user_by_role( 'subscriber' ) in the test is dead setup — the guard returns before authentication. Harmless, but it implies a dependency that isn't there.

Missing coverage

• No positive case confirming valid string credentials still delegate correctly to blogger_getUsersBlogs(). Worth one test so a future tightening of the guard can't silently break the happy path.
• No null, float, bool, or object cases.
• No multisite (@group ms-required) run. The guard precedes the is_multisite() branch so this is low risk, but the paths diverge right after it.

Security & performance

Net positive: converts a PHP fatal — trivially triggerable by an unauthenticated request, and capable of leaking a stack trace with display_errors on — into a controlled 400. Two is_string() calls, no measurable cost. The error message is a static translated string with no user input reflected.

Recommendation

Sound fix for the reported symptom. Before landing, I'd want the is_scalar vs is_string BC question settled on the ticket (issue 2), an object case in the provider (issue 3), and message assertions in the tests. Moving the guard into login() (issue 1) is the higher-value change if the ticket scope allows it.