Merge pull request #5968 from google/bug/#5110-fix-php81-warnings

Bug/#5110 fix php81 warnings
google · Dec 16, 2022 · 8beca04 · 8beca04
2 parents 5c2a7ac + 9197ff7
commit 8beca04
Show file tree

Hide file tree

Showing 10 changed files with 39 additions and 37 deletions.
diff --git a/includes/Core/Admin/Screens.php b/includes/Core/Admin/Screens.php
@@ -367,7 +367,7 @@ private function get_screens() {
 					'render_callback'  => function( Context $context ) {
 						$is_view_only = ! $this->authentication->is_authenticated();
 
-						$setup_slug = $context->input()->filter( INPUT_GET, 'slug', FILTER_SANITIZE_STRING );
+						$setup_slug = htmlspecialchars( $context->input()->filter( INPUT_GET, 'slug' ) ?: '' );
 						$reauth = $context->input()->filter( INPUT_GET, 'reAuth', FILTER_VALIDATE_BOOLEAN );
 						if ( $context->input()->filter( INPUT_GET, 'permaLink' ) ) {
 							?>

diff --git a/includes/Core/Admin/Standalone.php b/includes/Core/Admin/Standalone.php
@@ -92,7 +92,7 @@ function() {
 	public function is_standalone() {
 		global $pagenow;
 
-		$page       = $this->context->input()->filter( INPUT_GET, 'page', FILTER_SANITIZE_STRING );
+		$page       = htmlspecialchars( $this->context->input()->filter( INPUT_GET, 'page' ) ?: '' );
 		$standalone = $this->context->input()->filter( INPUT_GET, 'googlesitekit-standalone', FILTER_VALIDATE_BOOLEAN );
 
 		return ( 'admin.php' === $pagenow && false !== strpos( $page, 'googlesitekit' ) && $standalone );

diff --git a/includes/Core/Assets/Assets.php b/includes/Core/Assets/Assets.php
@@ -911,18 +911,14 @@ private function get_inline_tracking_data() {
 	 * @return array The inline data to be output.
 	 */
 	private function get_inline_data() {
-		$current_user = wp_get_current_user();
-		$site_url     = $this->context->get_reference_site_url();
-		$input        = $this->context->input();
-		$page         = $input->filter( INPUT_GET, 'page', FILTER_SANITIZE_STRING );
+		$site_url = $this->context->get_reference_site_url();
+		$input    = $this->context->input();
 
 		$admin_data = array(
 			'siteURL'      => esc_url_raw( $site_url ),
 			'resetSession' => $input->filter( INPUT_GET, 'googlesitekit_reset_session', FILTER_VALIDATE_BOOLEAN ),
 		);
 
-		$current_entity = $this->context->get_reference_entity();
-
 		return array(
 
 			/**

diff --git a/includes/Core/Authentication/Authentication.php b/includes/Core/Authentication/Authentication.php
@@ -304,7 +304,7 @@ public function register() {
 			'admin_init',
 			function() {
 				if (
-					'googlesitekit-dashboard' === $this->context->input()->filter( INPUT_GET, 'page', FILTER_SANITIZE_STRING )
+					'googlesitekit-dashboard' === htmlspecialchars( $this->context->input()->filter( INPUT_GET, 'page' ) ?: '' )
 					&& User_Input_State::VALUE_REQUIRED === $this->user_input_state->get()
 				) {
 					wp_safe_redirect( $this->context->admin_url( 'user-input' ) );

diff --git a/includes/Core/Authentication/Clients/OAuth_Client.php b/includes/Core/Authentication/Clients/OAuth_Client.php
@@ -399,8 +399,8 @@ function ( $scope ) {
 	 * @since 1.49.0 Uses the new `Google_Proxy::setup_url_v2` method when the `serviceSetupV2` feature flag is enabled.
 	 */
 	public function authorize_user() {
-		$code       = $this->context->input()->filter( INPUT_GET, 'code', FILTER_SANITIZE_STRING );
-		$error_code = $this->context->input()->filter( INPUT_GET, 'error', FILTER_SANITIZE_STRING );
+		$code       = htmlspecialchars( $this->context->input()->filter( INPUT_GET, 'code' ) );
+		$error_code = htmlspecialchars( $this->context->input()->filter( INPUT_GET, 'error' ) );
 		// If the OAuth redirects with an error code, handle it.
 		if ( ! empty( $error_code ) ) {
 			$this->user_options->set( self::OPTION_ERROR_CODE, $error_code );
@@ -450,7 +450,7 @@ public function authorize_user() {
 		if ( isset( $token_response['scope'] ) ) {
 			$scopes = explode( ' ', sanitize_text_field( $token_response['scope'] ) );
 		} elseif ( $this->context->input()->filter( INPUT_GET, 'scope' ) ) {
-			$scope  = $this->context->input()->filter( INPUT_GET, 'scope', FILTER_SANITIZE_STRING );
+			$scope  = htmlspecialchars( $this->context->input()->filter( INPUT_GET, 'scope' ) );
 			$scopes = explode( ' ', $scope );
 		} else {
 			$scopes = $this->get_required_scopes();

diff --git a/includes/Core/Authentication/Setup.php b/includes/Core/Authentication/Setup.php
@@ -127,7 +127,7 @@ private function get_oauth_proxy_failed_help_link() {
 	 * @since 1.48.0
 	 */
 	public function handle_action_setup_start() {
-		$nonce        = $this->context->input()->filter( INPUT_GET, 'nonce', FILTER_SANITIZE_STRING );
+		$nonce        = htmlspecialchars( $this->context->input()->filter( INPUT_GET, 'nonce' ) );
 		$redirect_url = $this->context->input()->filter( INPUT_GET, 'redirect', FILTER_SANITIZE_URL );
 
 		$this->verify_nonce( $nonce, Google_Proxy::ACTION_SETUP_START );
@@ -207,12 +207,12 @@ public function handle_action_setup_start() {
 	 */
 	public function handle_action_verify() {
 		$input               = $this->context->input();
-		$step                = $input->filter( INPUT_GET, 'step', FILTER_SANITIZE_STRING );
-		$nonce               = $input->filter( INPUT_GET, 'nonce', FILTER_SANITIZE_STRING );
-		$code                = $input->filter( INPUT_GET, 'googlesitekit_code', FILTER_SANITIZE_STRING );
-		$site_code           = $input->filter( INPUT_GET, 'googlesitekit_site_code', FILTER_SANITIZE_STRING );
-		$verification_token  = $input->filter( INPUT_GET, 'googlesitekit_verification_token', FILTER_SANITIZE_STRING );
-		$verification_method = $input->filter( INPUT_GET, 'googlesitekit_verification_token_type', FILTER_SANITIZE_STRING );
+		$step                = htmlspecialchars( $input->filter( INPUT_GET, 'step' ) );
+		$nonce               = htmlspecialchars( $input->filter( INPUT_GET, 'nonce' ) );
+		$code                = htmlspecialchars( $input->filter( INPUT_GET, 'googlesitekit_code' ) );
+		$site_code           = htmlspecialchars( $input->filter( INPUT_GET, 'googlesitekit_site_code' ) );
+		$verification_token  = htmlspecialchars( $input->filter( INPUT_GET, 'googlesitekit_verification_token' ) );
+		$verification_method = htmlspecialchars( $input->filter( INPUT_GET, 'googlesitekit_verification_token_type' ) );
 
 		$this->verify_nonce( $nonce );
 
@@ -266,10 +266,10 @@ public function handle_action_verify() {
 	 */
 	public function handle_action_exchange_site_code() {
 		$input     = $this->context->input();
-		$step      = $input->filter( INPUT_GET, 'step', FILTER_SANITIZE_STRING );
-		$nonce     = $input->filter( INPUT_GET, 'nonce', FILTER_SANITIZE_STRING );
-		$code      = $input->filter( INPUT_GET, 'googlesitekit_code', FILTER_SANITIZE_STRING );
-		$site_code = $input->filter( INPUT_GET, 'googlesitekit_site_code', FILTER_SANITIZE_STRING );
+		$step      = htmlspecialchars( $input->filter( INPUT_GET, 'step' ) );
+		$nonce     = htmlspecialchars( $input->filter( INPUT_GET, 'nonce' ) );
+		$code      = htmlspecialchars( $input->filter( INPUT_GET, 'googlesitekit_code' ) );
+		$site_code = htmlspecialchars( $input->filter( INPUT_GET, 'googlesitekit_site_code' ) );
 
 		$this->verify_nonce( $nonce );
 

diff --git a/includes/Core/REST_API/Data_Request.php b/includes/Core/REST_API/Data_Request.php
@@ -121,7 +121,9 @@ public function __isset( $name ) {
 	 *
 	 * @return bool
 	 */
-	public function offsetExists( $key ) {
+	// phpcs:ignore Squiz.Commenting.InlineComment.WrongStyle,Squiz.PHP.CommentedOutCode.Found
+	#[\ReturnTypeWillChange]
+	public function offsetExists( $key ) { // phpcs:ignore Squiz.Commenting.FunctionComment
 		return array_key_exists( $key, $this->data );
 	}
 
@@ -132,7 +134,9 @@ public function offsetExists( $key ) {
 	 *
 	 * @return mixed
 	 */
-	public function offsetGet( $key ) {
+	// phpcs:ignore Squiz.Commenting.InlineComment.WrongStyle,Squiz.PHP.CommentedOutCode.Found
+	#[\ReturnTypeWillChange]
+	public function offsetGet( $key ) { // phpcs:ignore Squiz.Commenting.FunctionComment
 		if ( $this->offsetExists( $key ) ) {
 			return $this->data[ $key ];
 		}
@@ -146,7 +150,9 @@ public function offsetGet( $key ) {
 	 * @param string|int $key Key to set the value for.
 	 * @param mixed      $value New value for the given key.
 	 */
-	public function offsetSet( $key, $value ) {
+	// phpcs:ignore Squiz.Commenting.InlineComment.WrongStyle,Squiz.PHP.CommentedOutCode.Found
+	#[\ReturnTypeWillChange]
+	public function offsetSet( $key, $value ) { // phpcs:ignore Squiz.Commenting.FunctionComment
 		// Data is immutable.
 	}
 
@@ -155,7 +161,9 @@ public function offsetSet( $key, $value ) {
 	 *
 	 * @param string|int $key Key to unset.
 	 */
-	public function offsetUnset( $key ) {
+	// phpcs:ignore Squiz.Commenting.InlineComment.WrongStyle,Squiz.PHP.CommentedOutCode.Found
+	#[\ReturnTypeWillChange]
+	public function offsetUnset( $key ) { // phpcs:ignore Squiz.Commenting.FunctionComment
 		// Data is immutable.
 	}
 }
diff --git a/includes/Core/Util/Input.php b/includes/Core/Util/Input.php
@@ -45,6 +45,7 @@ public function __construct() {
 	 * Gets a specific external variable by name and optionally filters it.
 	 *
 	 * @since 1.1.2
+	 * @since n.e.x.t Changed default value of $options parameter to 0.
 	 *
 	 * @link https://php.net/manual/en/function.filter-input.php
 	 *
@@ -60,7 +61,7 @@ public function __construct() {
 	 *                                      If the flag FILTER_NULL_ON_FAILURE is used, it returns FALSE if the variable is not set
 	 *                                      and NULL if the filter fails.
 	 */
-	public function filter( $type, $variable_name, $filter = FILTER_DEFAULT, $options = null ) {
+	public function filter( $type, $variable_name, $filter = FILTER_DEFAULT, $options = 0 ) {
 		$value = filter_input( $type, $variable_name, $filter, $options );
 
 		// Fallback for environments where filter_input may not work with specific types.

diff --git a/includes/Modules/Analytics.php b/includes/Modules/Analytics.php
@@ -266,7 +266,7 @@ protected function handle_provisioning_callback() {
 		}
 
 		// The handler should check the received Account Ticket id parameter against the id stored in the provisioning step.
-		$account_ticket_id        = $input->filter( INPUT_GET, 'accountTicketId', FILTER_SANITIZE_STRING );
+		$account_ticket_id        = htmlspecialchars( $input->filter( INPUT_GET, 'accountTicketId' ) );
 		$stored_account_ticket_id = get_transient( self::PROVISION_ACCOUNT_TICKET_ID . '::' . get_current_user_id() );
 		delete_transient( self::PROVISION_ACCOUNT_TICKET_ID . '::' . get_current_user_id() );
 
@@ -278,17 +278,17 @@ protected function handle_provisioning_callback() {
 		}
 
 		// Check for a returned error.
-		$error = $input->filter( INPUT_GET, 'error', FILTER_SANITIZE_STRING );
+		$error = $input->filter( INPUT_GET, 'error' );
 		if ( ! empty( $error ) ) {
 			wp_safe_redirect(
-				$this->context->admin_url( 'module-analytics', array( 'error_code' => $error ) )
+				$this->context->admin_url( 'module-analytics', array( 'error_code' => htmlspecialchars( $error ) ) )
 			);
 			exit;
 		}
 
-		$account_id      = $input->filter( INPUT_GET, 'accountId', FILTER_SANITIZE_STRING );
-		$web_property_id = $input->filter( INPUT_GET, 'webPropertyId', FILTER_SANITIZE_STRING );
-		$profile_id      = $input->filter( INPUT_GET, 'profileId', FILTER_SANITIZE_STRING );
+		$account_id      = htmlspecialchars( $input->filter( INPUT_GET, 'accountId' ) );
+		$web_property_id = htmlspecialchars( $input->filter( INPUT_GET, 'webPropertyId' ) );
+		$profile_id      = htmlspecialchars( $input->filter( INPUT_GET, 'profileId' ) );
 
 		if ( empty( $account_id ) || empty( $web_property_id ) || empty( $profile_id ) ) {
 			wp_safe_redirect(

diff --git a/tests/phpunit/integration/ContextTest.php b/tests/phpunit/integration/ContextTest.php
@@ -69,9 +69,6 @@ public function test_filter_input() {
 		$_GET['foo'] = true;
 
 		$this->assertTrue( $context->input()->filter( INPUT_GET, 'foo', FILTER_VALIDATE_BOOLEAN ) );
-
-		$_GET['dirty'] = '<script>dirt</script>';
-		$this->assertEquals( 'dirt', $context->input()->filter( INPUT_GET, 'dirty', FILTER_SANITIZE_STRING ) );
 	}
 
 	public function test_admin_url() {