WP_Http::normalize_cookies does not convert cookie names to strings

[nosilver4u]

We ran into an issue where, if a cookie with an integer name is submitted via wp_remote_post(), it triggers a fatal error/exception in Requests\Cookie::construct(). Looking at the stack trace, the cookies go through WP_Http::normalize_cookies(), which already casts the values to strings. Unfortunately, it just sends the cookie names straight through rather than casting the names to strings as is required by Requests\Cookies::construct().

Partial stack trace here:

PHP Fatal error: Uncaught WpOrg\Requests\Exception\InvalidArgument: WpOrg\Requests\Cookie::__construct(): Argument #1 ($name) must be of type string, integer given in [...]public_html/wp-includes/Requests/src/Exception/InvalidArgument.php:29
Stack trace:
#0 [...]public_html/wp-includes/Requests/src/Cookie.php(83): WpOrg\Requests\Exception\InvalidArgument::create(1, '$name', 'string', 'integer')
#1 [...]public_html/wp-includes/class-wp-http.php(472): WpOrg\Requests\Cookie->__construct(135370, '')
#2 [...]public_html/wp-includes/class-wp-http.php(352): WP_Http::normalize_cookies(Array)
#3 [...]public_html/wp-includes/class-wp-http.php(616): WP_Http->request('https://allegor...', Array)
#4 [...]public_html/wp-includes/http.php(179): WP_Http->post('https://allegor...', Array)
#5 [...]public_html/wp-content/plugins/ewww-image-optimizer/common.php(11271): wp_remote_post('https://allegor...', Array)

[nosilver4u]

This bug also seems to be affecting the Site Health page in certain cases: ​https://wordpress.org/support/topic/php-fatal-error-class-wp-site-health-php/

[engahmeds3ed]

To add more context here, plz check this PHP core feature/bug :) here: ​https://github.com/php/php-src/issues/9029

So if we even try to cast the cookie key to be string, this won't happen because PHP will detect that this key is numeric and change it back to integer so the error will occur in all cases.

I used the following code:

<?php
add_filter( 'http_request_args', function( $args ){
    if ( empty( $args['cookies'] ) || ! is_array( $args['cookies'] ) ) {
        return $args;
    }
    
    foreach ( $args['cookies'] as $cookie_key => $cookie_value ) {
        if ( is_string( $cookie_key ) ) {
            continue;
        }

        unset( $args['cookies'][$cookie_key] );
    }

    return $args;
}, 1000 );

Just to remove the integer cookie keys not to fall in this WP fatal error trap :)

Can we simply remove the thrown exception from here:
​https://github.com/WordPress/wordpress-develop/blob/b6b6ded79a54535b9ea80ae9c395dfa90262a4f9/src/wp-includes/Requests/src/Cookie.php#L83-L85

What do u think?

[nosilver4u]

Ah, that's a good catch on the PHP "feature", I hadn't thought of that happening. So there's no sense in casting the array indices to string, since they won't be preserved.
Instead, the Cookie() constructor should accept string OR integer values for the $name parameter. I think it's good for to be validating the "input", so I don't think dropping the exception would be appropriate.

[barry.hughes]

Can we simply remove the thrown exception from here:

​https://github.com/WordPress/wordpress-develop/blob/b6b6ded79a54535b9ea80ae9c395dfa90262a4f9/src/wp-includes/Requests/src/Cookie.php#L83-L85

Or widen it to allow integers? That would still offer some protection (against something truly unexpected, like a bool or object) while still accommodating both valid key types.

if ( is_string( $name ) === false && is_integer( $name ) === false ) {
        throw InvalidArgument::create( 1, '$name', 'string|int', gettype( $name ) );
}

[schlessera]

The above should be fixed by adding (string) casts to the first argument of the Cookie constructor in lines 470 & 472 here:

​https://github.com/WordPress/wordpress-develop/blob/ac4f768222ee9583357d44059df6c61940f7b2a5/src/wp-includes/class-wp-http.php#L470-L472