From b7f11b320b7c33ce881b58df0a55c1521d11bc00 Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Sat, 28 Nov 2020 14:06:53 -0600 Subject: [PATCH 1/2] Adds is_array check. Props @xknown. --- src/wp-admin/includes/privacy-tools.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wp-admin/includes/privacy-tools.php b/src/wp-admin/includes/privacy-tools.php index 14914dcb9c4b5..a12bd4681bffe 100644 --- a/src/wp-admin/includes/privacy-tools.php +++ b/src/wp-admin/includes/privacy-tools.php @@ -769,7 +769,7 @@ function wp_privacy_process_personal_data_export_page( $response, $exporter_inde } else { $accumulated_data = get_post_meta( $request_id, '_export_data_raw', true ); - if ( $accumulated_data ) { + if ( $accumulated_data && is_array( $accumulated_data ) ) { $export_data = $accumulated_data; } } From 7c168d2290844d8c7ab3ebc99acde86d911409a3 Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Sat, 28 Nov 2020 19:18:59 -0600 Subject: [PATCH 2/2] Sends json error when '_export_data_raw' not array. Retains the behavior of the meta being passed into the array_merge(), i.e. making it easier for developers to debug. --- src/wp-admin/includes/privacy-tools.php | 13 +- ...wpPrivacyProcessPersonalDataExportPage.php | 138 ++++++++++++++++-- 2 files changed, 136 insertions(+), 15 deletions(-) diff --git a/src/wp-admin/includes/privacy-tools.php b/src/wp-admin/includes/privacy-tools.php index a12bd4681bffe..4ca1eeee1db4d 100644 --- a/src/wp-admin/includes/privacy-tools.php +++ b/src/wp-admin/includes/privacy-tools.php @@ -769,8 +769,17 @@ function wp_privacy_process_personal_data_export_page( $response, $exporter_inde } else { $accumulated_data = get_post_meta( $request_id, '_export_data_raw', true ); - if ( $accumulated_data && is_array( $accumulated_data ) ) { - $export_data = $accumulated_data; + if ( $accumulated_data ) { + if ( is_array( $accumulated_data ) ) { + $export_data = $accumulated_data; + } else { + wp_send_json_error( + sprintf( + 'Warning: array_merge(): Expected parameter 1 to be an array, %1$s given.', + gettype( $accumulated_data ) + ) + ); + } } } diff --git a/tests/phpunit/tests/privacy/wpPrivacyProcessPersonalDataExportPage.php b/tests/phpunit/tests/privacy/wpPrivacyProcessPersonalDataExportPage.php index fd20c51de4317..e7fa06485ee1f 100644 --- a/tests/phpunit/tests/privacy/wpPrivacyProcessPersonalDataExportPage.php +++ b/tests/phpunit/tests/privacy/wpPrivacyProcessPersonalDataExportPage.php @@ -160,7 +160,21 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { self::$exporter_key_first = 'custom-exporter-first'; self::$exporter_key_last = 'custom-exporter-last'; - $data = array( + $data = self::get_response_data(); + + self::$response_first_page = array( + 'done' => false, + 'data' => $data, + ); + + self::$response_last_page = array( + 'done' => true, + 'data' => $data, + ); + } + + private static function get_response_data() { + return array( array( 'group_id' => 'custom-exporter-group-id', 'group_label' => 'Custom Exporter Group Label', @@ -169,21 +183,11 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { 'data' => array( array( 'name' => 'Email', - 'value' => self::$requester_email, + 'value' => 'requester@example.com', ), ), ), ); - - self::$response_first_page = array( - 'done' => false, - 'data' => $data, - ); - - self::$response_last_page = array( - 'done' => true, - 'data' => $data, - ); } /** @@ -412,6 +416,115 @@ public function test_send_error_when_invalid_request_action_name( $send_as_email ); } + /** + * The function should store export raw data. + * + * @ticket 51423 + * + * @dataProvider data_stores_raw_export_data + * + * @param string $response The response from the personal data exporter for the given page. + * @param int $exporter_index The index of the personal data exporter. Begins at 1. + * @param int $page The page of personal data for this exporter. Begins at 1. + * @param array $expected Expected '_export_data_raw' meta. + */ + public function test_should_stores_raw_export_data( $response, $exporter_index, $page, $expected, $export_data = null ) { + if ( null === $export_data ) { + $this->assertEmpty( get_post_meta( self::$request_id, '_export_data_raw', true ) ); + } else { + update_post_meta( self::$request_id, '_export_data_raw', $export_data ); + } + + // Set to bail out to avoid post meta from being deleted. + $response['done'] = false; + $expect_error = ( null !== $export_data && ! is_array( $export_data ) ); + + if ( $expect_error ) { + $this->_setup_expected_failure( '{"success":false,"data":"Warning: array_merge(): Expected parameter 1 to be an array, ' . gettype( $export_data ) . ' given."}' ); + } else { + $this->expectOutputString( '' ); + } + + wp_privacy_process_personal_data_export_page( + $response, + $exporter_index, + self::$requester_email, + $page, + self::$request_id, + true, + self::$exporter_key_first + ); + + $actual = get_post_meta( self::$request_id, '_export_data_raw', true ); + $this->assertSame( $expected, $actual ); + } + + public function data_stores_raw_export_data() { + $data = self::get_response_data(); + $export_data = array( 'test' => 'data' ); + $merged_export_data = array_merge( $export_data, $data ); + + return array( + '1st exporter & page 1 with no export data stores response[data]' => array( + 'response' => array( + 'done' => false, + 'data' => $data, + ), + 'exporter_index' => 1, + 'page' => 1, + 'expected' => $data, + ), + '1st exporter & page 1 with (array) export data stores response[data]' => array( + 'response' => array( + 'done' => false, + 'data' => $data, + ), + 'exporter_index' => 1, + 'page' => 1, + 'expected' => $data, + 'export_data' => $export_data, + ), + 'not 1st exporter with no export data stores response[data]' => array( + 'response' => array( + 'done' => false, + 'data' => $data, + ), + 'exporter_index' => 2, + 'page' => 1, + 'expected' => $data, + ), + 'not 1st exporter with (array) export data stores export data merged with response[data]' => array( + 'response' => array( + 'done' => false, + 'data' => $data, + ), + 'exporter_index' => 2, + 'page' => 1, + 'expected' => $merged_export_data, + 'export_data' => $export_data, + ), + '1st exporter, page > 1, with (string) export data updates export data merged with response[data]' => array( + 'response' => array( + 'done' => false, + 'data' => $data, + ), + 'exporter_index' => 1, + 'page' => 2, + 'expected' => $data, + 'export_data' => 'not an array', + ), + 'not 1st exporter, page 1, no export data updates to response[data]' => array( + 'response' => array( + 'done' => false, + 'data' => $data, + ), + 'exporter_index' => 2, + 'page' => 1, + 'expected' => $data, + ), + ); + } + /** * The function should store export raw data until the export finishes. Then the meta key should be deleted. * @@ -420,7 +533,6 @@ public function test_send_error_when_invalid_request_action_name( $send_as_email * @dataProvider data_send_as_email_options * * @param bool Whether the final results of the export should be emailed to the user. - * */ public function test_raw_data_post_meta( $send_as_email ) { $this->assertEmpty( get_post_meta( self::$request_id, '_export_data_raw', true ) );