WordPress.org

Make WordPress Core

Opened 4 weeks ago

Last modified 4 weeks ago

#44215 new defect (bug)

Personal data exports - Empty values are included in the report

Reported by: bor0 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch dev-feedback has-screenshots
Focuses: Cc:

Description

Empty values should not be included in the report. Otherwise, the report will be cluttered with empty values.

Use the following code in mu-plugins to reproduce:

<?php
function exporter_1( $email_address, $page = 1 ) {
	return array(
		'data' => array( array(
			'group_id'    => 'group-1',
			'group_label' => __( 'Orders', 'woocommerce-services' ),
			'item_id'     => 'item-1',
			'data'	=> array(
				array(
					'name'  => __( 'Exporter 1' ),
					'value' => 1234,
				),
				array(
					'name'  => __( 'Exporter 2' ),
					'value' => '',
				),
				array(
					'name'  => __( 'Exporter 3' ),
					'value' => '0',
				),
				array(
					'name'  => __( 'Exporter 4' ),
					'value' => 0,
				),
			),
		) ),
		'done' => true,
	);
}

function register_exporter_1( $exporters ) {
	$exporters['exporter_1'] = array(
		'exporter_friendly_name' => __( 'Exporter 1' ),
		'callback' => 'exporter_1',
	);
	return $exporters;
}

add_filter(
	'wp_privacy_personal_data_exporters',
	'register_exporter_1'
);

Results:

Before proposed patch: https://cloudup.com/iPJ8ocUnyiz

After proposed patch: https://cloudup.com/i7MqWkMZjrj

Attachments (1)

44215.patch (726 bytes) - added by bor0 4 weeks ago.

Download all attachments as: .zip

Change History (7)

@bor0
4 weeks ago

#1 @bor0
4 weeks ago

Pinging @allendav for any input. Thanks!

#2 @allendav
4 weeks ago

I don't know - I would hope plugins could avoid inserting meaningless values into the report. That said, a completely empty string - sure - we could discard that. Beyond that however, I would push back beyond having core pass judgement on what is passed to it by an exporter.

This ticket was mentioned in Slack in #gdpr-compliance by desrosj. View the logs.


4 weeks ago

#4 @bor0
4 weeks ago

Empty strings would be neat and sufficient :)

#5 @desrosj
4 weeks ago

  • Version changed from trunk to 4.9.6

I am wondering if leaving empty values in the report is important. The site is technically storing data on the user, the string is just empty. This could be significant in certain contexts.

In your example there are 0s. A 0 (which is falsey or can be considered empty) could indicate that a user has no orders or no published posts. An empty string could mean that they did not provide a URL on their profile. A user meta field could be stored for speaks_english and be set to false or 0.

So in certain contexts, these empty values may be important.

Last edited 4 weeks ago by desrosj (previous) (diff)

#6 @birgire
4 weeks ago

It's a good question, but I agree with @desrosj, that we should not filter out "empty" values, like false and 0 as they could have a specific meaning.

Also anonymous functions, like in 44215.patch, were first supported in PHP 5.3.0

https://secure.php.net/manual/en/functions.anonymous.php

but WordPress still supports PHP 5.2.4

https://wordpress.org/about/requirements/

Note: See TracTickets for help on using tickets.