Stats: remove unnecessary params passed to wpcom (#28935)

* [not verified] add filte_and_build_query_string to generally filter and build query * changelog * fix typo * add tests for filter_and_build_query_string * no need to check before unsetting * just to be safe so still use isset first * fix tests * added get_wp_error and tests * fix syntax for php5.6
Automattic · Feb 14, 2023 · 48b1358 · 48b1358
1 parent 2defe1d
commit 48b1358
Show file tree

Hide file tree

Showing 3 changed files with 145 additions and 11 deletions.
diff --git a/projects/packages/stats-admin/changelog/fix-remove-unnecessary-params-passed-to-wpcom b/projects/packages/stats-admin/changelog/fix-remove-unnecessary-params-passed-to-wpcom
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fixed
+
+Stats: remove unnecessary params that breaks wpcom API
diff --git a/projects/packages/stats-admin/src/class-rest-controller.php b/projects/packages/stats-admin/src/class-rest-controller.php
@@ -290,8 +290,9 @@ public function get_single_post_likes( $req ) {
 				Constants::get_constant( 'JETPACK__WPCOM_JSON_API_BASE' ),
 				Jetpack_Options::get_option( 'id' ),
 				$req->get_param( 'resource_id' ),
-				http_build_query(
-					$req->get_params()
+				$this->filter_and_build_query_string(
+					$req->get_params(),
+					array( 'resource_id' )
 				)
 			),
 			array( 'timeout' => 5 )
@@ -385,7 +386,7 @@ public function get_site_posts( $req ) {
 				Constants::get_constant( 'JETPACK__WPCOM_JSON_API_BASE' ),
 				Jetpack_Options::get_option( 'id' ),
 				$req->get_param( 'resource_id' ),
-				http_build_query( $params )
+				$this->filter_and_build_query_string( $params, array( 'resource_id' ) )
 			),
 			array( 'timeout' => 5 )
 		);
@@ -419,7 +420,7 @@ public function site_has_never_published_post( $req ) {
 			sprintf(
 				'/sites/%d/site-has-never-published-post?%s',
 				Jetpack_Options::get_option( 'id' ),
-				http_build_query(
+				$this->filter_and_build_query_string(
 					$req->get_params()
 				)
 			),
@@ -441,7 +442,7 @@ public function get_wordads_earnings( $req ) {
 			sprintf(
 				'/sites/%d/wordads/earnings?%s',
 				Jetpack_Options::get_option( 'id' ),
-				http_build_query(
+				$this->filter_and_build_query_string(
 					$req->get_params()
 				)
 			),
@@ -461,7 +462,7 @@ public function get_wordads_stats( $req ) {
 			sprintf(
 				'/sites/%d/wordads/stats?%s',
 				Jetpack_Options::get_option( 'id' ),
-				http_build_query(
+				$this->filter_and_build_query_string(
 					$req->get_params()
 				)
 			),
@@ -518,11 +519,7 @@ protected function request_as_blog_cached( $path, $version = '1.1', $args = arra
 		}
 
 		if ( 200 !== $response_code ) {
-			return new WP_Error(
-				isset( $response_body['error'] ) ? 'remote-error-' . $response_body['error'] : 'remote-error',
-				isset( $response_body['message'] ) ? $response_body['message'] : 'unknown remote error',
-				array( 'status' => $response_code )
-			);
+			return $this->get_wp_error( $response_body, $response_code );
 		}
 
 		// Cache the successful JSON response for 5 minutes.
@@ -541,4 +538,50 @@ protected function get_forbidden_error() {
 
 		return new WP_Error( 'rest_forbidden', $error_msg, array( 'status' => rest_authorization_required_code() ) );
 	}
+
+	/**
+	 * Filter and build query string from all the requested params.
+	 *
+	 * @param array $params The params to filter.
+	 * @param array $keys_to_unset The keys to unset from the params array.
+	 * @return string The filtered and built query string.
+	 */
+	protected function filter_and_build_query_string( $params, $keys_to_unset = array() ) {
+		if ( isset( $params['rest_route'] ) ) {
+			unset( $params['rest_route'] );
+		}
+		if ( ! empty( $keys_to_unset ) && is_array( $keys_to_unset ) ) {
+			foreach ( $keys_to_unset as $key ) {
+				if ( isset( $params[ $key ] ) ) {
+					unset( $params[ $key ] );
+				}
+			}
+		}
+		return http_build_query( $params );
+	}
+
+	/**
+	 * Build error object from remote response body and status code.
+	 *
+	 * @param array $response_body Remote response body.
+	 * @param int   $response_code Http response code.
+	 * @return WP_Error
+	 */
+	protected function get_wp_error( $response_body, $response_code = 500 ) {
+		$error_code = 'remote-error';
+		foreach ( array( 'code', 'error' ) as $error_code_key ) {
+			if ( isset( $response_body[ $error_code_key ] ) ) {
+				$error_code = $response_body[ $error_code_key ];
+				break;
+			}
+		}
+
+		$error_message = isset( $response_body['message'] ) ? $response_body['message'] : 'unknown remote error';
+
+		return new WP_Error(
+			$error_code,
+			$error_message,
+			array( 'status' => $response_code )
+		);
+	}
 }
diff --git a/projects/packages/stats-admin/tests/php/test-rest-controller.php b/projects/packages/stats-admin/tests/php/test-rest-controller.php
@@ -180,4 +180,91 @@ public function test_stats_notices_illegal_params() {
 
 		$this->assertEquals( 400, $response->get_status() );
 	}
+
+	/**
+	 * Test filter_and_build_query_string.
+	 */
+	public function test_filter_and_build_query_string() {
+		$filter_and_build_query_string = new \ReflectionMethod( $this->rest_controller, 'filter_and_build_query_string' );
+		$filter_and_build_query_string->setAccessible( true );
+
+		$this->assertEquals(
+			'c=d&e=f',
+			$filter_and_build_query_string->invoke(
+				$this->rest_controller,
+				array(
+					'a'          => 'b',
+					'c'          => 'd',
+					'rest_route' => '/jetpack/v4/test-route',
+					'e'          => 'f',
+				),
+				array( 'a' )
+			)
+		);
+		$this->assertEquals(
+			'a=b&c=d&e=f',
+			$filter_and_build_query_string->invoke(
+				$this->rest_controller,
+				array(
+					'a'          => 'b',
+					'c'          => 'd',
+					'rest_route' => '/jetpack/v4/test-route',
+					'e'          => 'f',
+				)
+			)
+		);
+		$this->assertEquals(
+			'',
+			$filter_and_build_query_string->invoke(
+				$this->rest_controller,
+				array()
+			)
+		);
+	}
+
+	/**
+	 * Test filter_and_build_query_string.
+	 */
+	public function test_get_wp_error() {
+		$get_wp_error = new \ReflectionMethod( $this->rest_controller, 'get_wp_error' );
+		$get_wp_error->setAccessible( true );
+
+		$error = $get_wp_error->invoke(
+			$this->rest_controller,
+			array(
+				'error'   => 'err1',
+				'message' => 'msg1',
+			),
+			500
+		);
+		$this->assertEquals(
+			'err1',
+			$error->get_error_code()
+		);
+
+		$error = $get_wp_error->invoke(
+			$this->rest_controller,
+			array(
+				'code'    => 'err2',
+				'message' => 'msg1',
+			),
+			500
+		);
+		$this->assertEquals(
+			'err2',
+			$error->get_error_code()
+		);
+
+		$error = $get_wp_error->invoke(
+			$this->rest_controller,
+			array(
+				'code' => 'err2',
+			),
+			500
+		);
+		$this->assertEquals(
+			'unknown remote error',
+			$error->get_error_message()
+		);
+	}
 }