REST API: Fix incorrect calls to `rest_sanitize_value_from_schema`

[jnylen0]

Two instances in WP_REST_Users_Controller where we are passing $request instead of $args. This allows some minor weirdness:

POST /wp/v2/users?username=abcd&password=efgh&type=array

A user will be created with the username and password set to Array because $request['array'] is interpreted as a type argument.

One more incorrect call in rest_sanitize_request_arg where we are calling the function with a nonexistent third argument.

[joehoyle]

@jnylen0 looking at this, I think the intention was to use rest_sanitize_request_arg rather than rest_sanitize_value_from_schema. I think maybe it would be better to use rest_sanitize_request_arg incase these params get other schema / options in the future.

[jnylen0]

Yep, I wrote this code in #38739.

If we add additional schema / options here, it will presumably be for a reason that will have unit tests and will probably be addressed in the code for these check_user* functions. Until then I prefer the clarity and explicitness of strval - apparently it's easy to misuse these functions.

[rachelbaker]

@jnylen0 for the username we can't simply use any string value. There are invalid characters that need to be stripped. I *think* we would want to use the sanitize_user() function for this.

[jnylen0]

Replying to rachelbaker:

@jnylen0 for the username we can't simply use any string value. There are invalid characters that need to be stripped.

This is already handled (see validate_username call in the check_username function, and associated tests)

[rachelbaker]

Replying to jnylen0:

Replying to rachelbaker:

@jnylen0 for the username we can't simply use any string value. There are invalid characters that need to be stripped.

This is already handled (see validate_username call in the check_username function, and associated tests)

Ah. I had to scroll down :). Now I remember why I don't review patches on mobile.

Why the change to strval() from the (string) typecasting? Is there a reason to change that? In the RC phase of a release I would prefer avoid non-bug code tweaks.

[jnylen0]

Should be fine either way. I only changed it because rest_sanitize_value_from_schema uses strval (ref). (When this code was originally written, that conversion to string wasn't present.)

[rachelbaker]

Changes strval() back to (string) typecasting

[rachelbaker]

+1 on 38984.2.diff​ which just removes the added strval() function in check_user_password() and check_username().

Still needs another permanent committer to review.

[rachelbaker]

In 39400:

REST API: Fix incorrect uses of rest_sanitize_value_from_schema().

In the check_username() and check_password() callbacks in the Users controller cast the provided request value to a string. The rest_sanitize_value_from_schema() function was being used incorrectly which was causing unintended request parsing.
In rest_sanitize_request_arg() do not pass nonexistent third parameter for the rest_sanitize_value_from_schema() function.

Props jnylen0, joehoyle, rachelbaker, ocean90.
Fixes #38984.

[rachelbaker]

In 39401:

REST API: Fix incorrect uses of rest_sanitize_value_from_schema().

In the check_username() and check_password() callbacks in the Users controller cast the provided request value to a string. The rest_sanitize_value_from_schema() function was being used incorrectly which was causing unintended request parsing.
In rest_sanitize_request_arg() do not pass nonexistent third parameter for the rest_sanitize_value_from_schema() function.

Props jnylen0, joehoyle, rachelbaker, ocean90.

Merges [39400] to the 4.7 branch.
Fixes #38984 for 4.7.