Code Quality: Correct nullable WP_Http_Cookie property types.

A cookie created without a name, value, domain, expiration or host-only flag, or from incomplete data, leaves those properties `null`, and an unparsable expiration date is stored as `false`, but their documented types said otherwise. PHPStan therefore treated `isset()` checks on them as always true, so correcting the types retires the `booleanOr.alwaysFalse` baseline entirely along with several `isset.property` entries.

The corrected types surfaced real errors in callers, which are fixed rather than baselined:
* In `WP_Http::normalize_cookies()`, a `WP_Http_Cookie` value was passed to `WpOrg\Requests\Cookie` as is, which throws an `InvalidArgument` exception for anything but a string, failing the request. A cookie without a value or with a non-scalar value is now skipped, as `WP_Http_Cookie::getHeaderValue()` already does, and other scalar values are cast to strings.
* `wp_remote_retrieve_cookie_value()` now returns an empty string instead of `null` for a cookie without a value.

The handling of empty or missing cookie names is unchanged, as it is being addressed separately in #66305.

Developed in #14283.
Follow-up to r38164, r45135, r57501, r63023.

Props huzaifaalmesbah, westonruter.
See #65817, #66305.


git-svn-id: https://develop.svn.wordpress.org/trunk@64350 602fd350-edb4-49c9-b593-d223f7449a82

Author: westonruter

phpstan.neon.dist

@@ -31,7 +31,6 @@ includes:
 	- tests/phpstan/baselines/booleanAnd.rightAlwaysTrue.neon
 	- tests/phpstan/baselines/booleanNot.alwaysFalse.neon
 	- tests/phpstan/baselines/booleanNot.alwaysTrue.neon
-	- tests/phpstan/baselines/booleanOr.alwaysFalse.neon
 	- tests/phpstan/baselines/class.notFound.neon
 	- tests/phpstan/baselines/deadCode.unreachable.neon
 	- tests/phpstan/baselines/empty.offset.neon

src/wp-includes/class-wp-http-cookie.php

@@ -26,7 +26,7 @@ class WP_Http_Cookie {
 	 *
 	 * @since 2.8.0
 	 *
-	 * @var string
+	 * @var string|null
 	 */
 	public $name;
 
@@ -35,16 +35,18 @@ class WP_Http_Cookie {
 	 *
 	 * @since 2.8.0
 	 *
-	 * @var string
+	 * @var string|null
 	 */
 	public $value;
 
 	/**
-	 * When the cookie expires. Unix timestamp or formatted date.
+	 * When the cookie expires, as a Unix timestamp.
+	 *
+	 * False if the expiration date could not be parsed, or null if not provided.
 	 *
 	 * @since 2.8.0
 	 *
-	 * @var string|int|null
+	 * @var int|false|null
 	 */
 	public $expires;
 
@@ -62,7 +64,7 @@ class WP_Http_Cookie {
 	 *
 	 * @since 2.8.0
 	 *
-	 * @var string
+	 * @var string|null
 	 */
 	public $domain;
 
@@ -76,11 +78,13 @@ class WP_Http_Cookie {
 	public $port;
 
 	/**
-	 * host-only flag.
+	 * Host-only flag.
+	 *
+	 * Null if not provided.
 	 *
 	 * @since 5.2.0
 	 *
-	 * @var bool
+	 * @var bool|null
 	 */
 	public $host_only;
 
@@ -100,9 +104,9 @@ class WP_Http_Cookie {
 	 *     @type mixed           $value     Value. Should NOT already be urlencoded.
 	 *     @type string|int|null $expires   Optional. Unix timestamp or formatted date. Default null.
 	 *     @type string|null     $path      Optional. Path. Default '/'.
-	 *     @type string          $domain    Optional. Domain. Default host of parsed $requested_url.
+	 *     @type string|null     $domain    Optional. Domain. Default host of parsed $requested_url.
 	 *     @type int|string|null $port      Optional. Port or comma-separated list of ports. Default null.
-	 *     @type bool            $host_only Optional. host-only storage flag. Default true.
+	 *     @type bool|null       $host_only Optional. Host-only storage flag. Default null.
 	 * }
 	 * @param string       $requested_url The URL which the cookie was set on, used for default $domain
 	 *                                    and $port values.
@@ -262,9 +266,9 @@ public function getFullHeader() { // phpcs:ignore WordPress.NamingConventions.Va
 	 * @return array {
 	 *     List of attributes.
 	 *
-	 *     @type string|int|null $expires When the cookie expires. Unix timestamp or formatted date.
-	 *     @type string          $path    Cookie URL path.
-	 *     @type string          $domain  Cookie domain.
+	 *     @type int|false|null $expires When the cookie expires, as a Unix timestamp.
+	 *     @type string         $path    Cookie URL path.
+	 *     @type string|null    $domain  Cookie domain.
 	 * }
 	 */
 	public function get_attributes() {

src/wp-includes/class-wp-http.php

@@ -474,24 +474,38 @@ public function request( $url, $args = array() ) {
 	 *
 	 * @since 4.6.0
 	 *
-	 * @param array $cookies Array of cookies to send with the request.
+	 * @param array<int|string, WP_Http_Cookie|scalar> $cookies Array of cookies to send with the request,
+	 *                                                          as WP_Http_Cookie objects or name/value pairs.
+	 *                                                          Cookies without a scalar value are ignored.
 	 * @return WpOrg\Requests\Cookie\Jar Cookie holder object.
 	 */
 	public static function normalize_cookies( $cookies ) {
 		$cookie_jar = new WpOrg\Requests\Cookie\Jar();
 
 		foreach ( $cookies as $name => $value ) {
 			if ( $value instanceof WP_Http_Cookie ) {
-				$attributes                 = array_filter(
+				$cookie_name  = (string) $value->name;
+				$cookie_value = $value->value;
+				$attributes   = array_filter(
 					$value->get_attributes(),
 					static function ( $attr ) {
 						return null !== $attr;
 					}
 				);
-				$cookie_jar[ $value->name ] = new WpOrg\Requests\Cookie( (string) $value->name, $value->value, $attributes, array( 'host-only' => $value->host_only ) );
-			} elseif ( is_scalar( $value ) ) {
-				$cookie_jar[ $name ] = new WpOrg\Requests\Cookie( (string) $name, (string) $value );
+				$flags        = array( 'host-only' => $value->host_only );
+			} else {
+				$cookie_name  = (string) $name;
+				$cookie_value = $value;
+				$attributes   = array();
+				$flags        = array();
 			}
+
+			// A cookie without a value is not sent by WP_Http_Cookie::getHeaderValue() either, and a non-scalar value cannot be represented.
+			if ( ! is_scalar( $cookie_value ) ) {
+				continue;
+			}
+
+			$cookie_jar[ $cookie_name ] = new WpOrg\Requests\Cookie( $cookie_name, (string) $cookie_value, $attributes, $flags );
 		}
 
 		return $cookie_jar;

src/wp-includes/http.php

@@ -381,7 +381,7 @@ function wp_remote_retrieve_cookie_value( $response, $name ) {
 		return '';
 	}
 
-	return $cookie->value;
+	return $cookie->value ?? '';
 }
 
 /**

tests/phpstan/baselines/booleanOr.alwaysFalse.neon

@@ -113,21 +113,6 @@ parameters:
 			identifier: isset.property
 			count: 1
 			path: ../../../src/wp-includes/class-wp-customize-widgets.php
-		-
-			message: '#^Property WP_Http_Cookie\:\:\$domain \(string\) in isset\(\) is not nullable\.$#'
-			identifier: isset.property
-			count: 1
-			path: ../../../src/wp-includes/class-wp-http-cookie.php
-		-
-			message: '#^Property WP_Http_Cookie\:\:\$name \(string\) in isset\(\) is not nullable\.$#'
-			identifier: isset.property
-			count: 1
-			path: ../../../src/wp-includes/class-wp-http-cookie.php
-		-
-			message: '#^Property WP_Http_Cookie\:\:\$value \(string\) in isset\(\) is not nullable\.$#'
-			identifier: isset.property
-			count: 1
-			path: ../../../src/wp-includes/class-wp-http-cookie.php
 		-
 			message: '#^Property WP_Post\:\:\$ID \(int\<0, max\>\) in isset\(\) is not nullable\.$#'
 			identifier: isset.property

tests/phpstan/baselines/isset.property.neon

@@ -21,5 +21,5 @@ parameters:
 		-
 			message: '#^WpOrg\\Requests\\Cookie\\Jar does not accept WpOrg\\Requests\\Cookie\.$#'
 			identifier: offsetAssign.valueType
-			count: 2
+			count: 1
 			path: ../../../src/wp-includes/class-wp-http.php

tests/phpstan/baselines/offsetAssign.valueType.neon

@@ -225,6 +225,20 @@ public function test_get_response_cookies() {
 		$this->assertSame( '', $no_cookie );
 	}
 
+	/**
+	 * Tests that {@see wp_remote_retrieve_cookie_value()} returns an empty string for a cookie without a value.
+	 *
+	 * @covers ::wp_remote_retrieve_cookie_value
+	 */
+	public function test_wp_remote_retrieve_cookie_value_for_cookie_without_value(): void {
+		$cookie = new WP_Http_Cookie( array( 'name' => 'flavor' ) );
+		$this->assertNull( $cookie->value, 'Expected the cookie value to be null.' );
+
+		$response = array( 'cookies' => array( $cookie ) );
+
+		$this->assertSame( '', wp_remote_retrieve_cookie_value( $response, 'flavor' ) );
+	}
+
 	/**
 	 * @ticket 37437
 	 *

tests/phpunit/tests/http/functions.php

@@ -696,4 +696,76 @@ public function test_normalize_cookies_casts_cookie_name_integer_to_string() {
 		$this->assertInstanceOf( 'WpOrg\Requests\Cookie\Jar', $cookie_jar );
 		$this->assertInstanceOf( 'WpOrg\Requests\Cookie', $cookie_jar['1'] );
 	}
+
+	/**
+	 * Tests that {@see WP_Http::normalize_cookies()} skips a cookie without a value, as {@see WP_Http_Cookie::getHeaderValue()} does.
+	 *
+	 * @covers WP_Http::normalize_cookies
+	 */
+	public function test_normalize_cookies_skips_cookie_without_value(): void {
+		$cookie = new WP_Http_Cookie( array( 'name' => 'flavor' ) );
+		$this->assertNull( $cookie->value, 'Expected the cookie value to be null.' );
+		$this->assertSame( '', $cookie->getHeaderValue(), 'Expected the cookie to be omitted from the header value.' );
+
+		$cookie_jar = WP_Http::normalize_cookies( array( $cookie ) );
+
+		$this->assertSame( array(), iterator_to_array( $cookie_jar ) );
+	}
+
+	/**
+	 * Tests that {@see WP_Http::normalize_cookies()} skips a cookie with a non-scalar value.
+	 *
+	 * @covers WP_Http::normalize_cookies
+	 */
+	public function test_normalize_cookies_skips_cookie_with_non_scalar_value(): void {
+		$cookie = new WP_Http_Cookie(
+			array(
+				'name'  => 'prefs',
+				'value' => array( 'a' => 1 ),
+			)
+		);
+
+		$cookie_jar = WP_Http::normalize_cookies( array( $cookie ) );
+
+		$this->assertSame( array(), iterator_to_array( $cookie_jar ) );
+	}
+
+	/**
+	 * Tests that {@see WP_Http::normalize_cookies()} sends a cookie with a scalar value as a string.
+	 *
+	 * @covers WP_Http::normalize_cookies
+	 */
+	public function test_normalize_cookies_casts_scalar_cookie_value_to_string(): void {
+		$cookie = new WP_Http_Cookie(
+			array(
+				'name'  => 'count',
+				'value' => 5,
+			)
+		);
+
+		$cookie_jar = WP_Http::normalize_cookies( array( $cookie ) );
+
+		$this->assertInstanceOf( 'WpOrg\Requests\Cookie', $cookie_jar['count'] );
+		$this->assertSame( '5', $cookie_jar['count']->value );
+	}
+
+	/**
+	 * Tests that {@see WP_Http::normalize_cookies()} sends a cookie with an empty value.
+	 *
+	 * @covers WP_Http::normalize_cookies
+	 */
+	public function test_normalize_cookies_sends_cookie_with_empty_value(): void {
+		$cookie = new WP_Http_Cookie(
+			array(
+				'name'  => 'flavor',
+				'value' => '',
+			)
+		);
+		$this->assertSame( 'flavor=', $cookie->getHeaderValue(), 'Expected the cookie to be in the header value.' );
+
+		$cookie_jar = WP_Http::normalize_cookies( array( $cookie ) );
+
+		$this->assertInstanceOf( 'WpOrg\Requests\Cookie', $cookie_jar['flavor'] );
+		$this->assertSame( 'flavor=', $cookie_jar['flavor']->format_for_header() );
+	}
 }