Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion classes/Visualizer/Module/Admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ public function __construct( Visualizer_Plugin $plugin ) {
$this->_addFilter( 'media_view_strings', 'setupMediaViewStrings' );
$this->_addFilter( 'plugin_action_links', 'getPluginActionLinks', 10, 2 );
$this->_addFilter( 'plugin_row_meta', 'getPluginMetaLinks', 10, 2 );
$this->_addFilter( 'visualizer_logger_data', 'getLoggerData' );
$this->_addFilter( 'visualizer_feedback_review_trigger', 'feedbackReviewTrigger' );
$this->_addFilter( 'themeisle_sdk_blackfriday_data', 'add_black_friday_data' );

Expand Down
6 changes: 3 additions & 3 deletions classes/Visualizer/Module/Setup.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public function getUsage( $data, $meta_keys = array() ) {
$lib = get_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_LIBRARY, true );
$charts['library'][ $lib ] = isset( $charts['library'][ $lib ] ) ? $charts['library'][ $lib ] + 1 : 1;
$settings = get_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, true );
if ( array_key_exists( 'manual', $settings ) && ! empty( $settings['manual'] ) ) {
if ( is_array( $settings ) && ! empty( $settings['manual'] ) ) {
$charts['manual_config'] = $charts['manual_config'] + 1;
}

Expand All @@ -124,15 +124,15 @@ public function getUsage( $data, $meta_keys = array() ) {

if ( Visualizer_Module::is_pro() ) {
$permissions = get_post_meta( $chart_id, Visualizer_Pro::CF_PERMISSIONS, true );
if ( empty( $permissions ) ) {
if ( ! is_array( $permissions ) || empty( $permissions['permissions'] ) ) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in a4de841 — the scenario reproduces exactly as described (TypeError at the count() call). Guarded the per-key value with is_array() and added a regression test that forces the pro branch via the visualizer_is_pro filter with a stubbed Visualizer_Pro class.

continue;
}
$permissions = $permissions['permissions'];
$customized = false;
foreach ( $default_perms as $key => $val ) {
if ( ! is_array( $val ) && ! is_null( $val ) && isset( $permissions[ $key ] ) && $permissions[ $key ] !== $val ) {
$customized = true;
} elseif ( is_array( $val ) && ! is_null( $val ) && isset( $permissions[ $key ] ) && count( $permissions[ $key ] ) !== count( $val ) ) {
} elseif ( is_array( $val ) && ! is_null( $val ) && isset( $permissions[ $key ] ) && is_array( $permissions[ $key ] ) && count( $permissions[ $key ] ) !== count( $val ) ) {
$customized = true;
}
}
Expand Down
16 changes: 16 additions & 0 deletions tests/e2e/config/mu-plugins/plant-chart-settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,21 @@ function () {
},
)
);

// Runs the SDK usage logger on demand, so specs can verify it
// tolerates whatever chart meta they planted (issue #1359).
register_rest_route(
'visualizer-e2e/v1',
'/usage',
array(
'methods' => 'GET',
'permission_callback' => function () {
return current_user_can( 'manage_options' );
},
'callback' => function () {
return apply_filters( 'visualizer_logger_data', array() );
},
)
);
}
);
69 changes: 69 additions & 0 deletions tests/e2e/specs/usage-logger.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/**
* WordPress dependencies
*/
const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' );

/**
* Internal dependencies
*/
const { deleteAllCharts } = require( '../utils/common' );

/**
* Regression tests for https://github.com/Codeinwp/visualizer/issues/1359
*
* A published chart whose `visualizer-settings` meta is a string (instead of
* the sanitized settings array) crashed `Visualizer_Module_Setup::getUsage()`
* with a PHP 8 TypeError, aborting the whole SDK usage collection request.
* The logger must tolerate such charts and still report the others.
*/
test.describe( 'Usage logger', () => {
let corruptedId;
let manualId;

test.beforeAll( async ( { requestUtils } ) => {
// The assertions below count charts, so start from a clean library.
await deleteAllCharts( requestUtils );

// A chart whose settings meta is a corrupted string value.
const corrupted = await requestUtils.rest( {
method: 'POST',
path: '/wp/v2/visualizer',
data: { title: 'Corrupted settings chart', status: 'publish' },
} );
corruptedId = corrupted.id;
await requestUtils.rest( {
method: 'POST',
path: `/visualizer-e2e/v1/chart-settings/${ corruptedId }`,
data: { settings: 'corrupted string settings' },
} );

// A healthy chart with a manual configuration, which must still be counted.
const manual = await requestUtils.rest( {
method: 'POST',
path: '/wp/v2/visualizer',
data: { title: 'Manual config chart', status: 'publish' },
} );
manualId = manual.id;
await requestUtils.rest( {
method: 'POST',
path: `/visualizer-e2e/v1/chart-settings/${ manualId }`,
data: { settings: { manual: '{"colors": ["#000"]}' } },
} );
} );

test.afterAll( async ( { requestUtils } ) => {
for ( const id of [ corruptedId, manualId ] ) {
if ( id ) {
await requestUtils.rest( { method: 'DELETE', path: `/wp/v2/visualizer/${ id }`, params: { force: true } } );
}
}
} );

test( 'survives a chart whose settings meta is a string', async ( { requestUtils } ) => {
// Before the fix this request died with a TypeError (HTTP 500).
const usage = await requestUtils.rest( { method: 'GET', path: '/visualizer-e2e/v1/usage' } );

expect( usage.manual_config ).toBe( 1 );
expect( Object.values( usage.types ).reduce( ( a, b ) => a + b, 0 ) ).toBe( 2 );
} );
} );
103 changes: 103 additions & 0 deletions tests/test-usage-logger.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
<?php
/**
* WordPress unit test plugin.
*
* @package visualizer
* @subpackage Tests
* @license http://opensource.org/licenses/gpl-2.0.php GNU Public License
*/

/**
* Test the usage logger fed to the Themeisle SDK.
*
* Regression tests for https://github.com/Codeinwp/visualizer/issues/1359 —
* a published chart whose `visualizer-settings` meta is a string crashed
* `Visualizer_Module_Setup::getUsage()` with a TypeError on PHP 8, aborting
* scheduled usage collection.
*/
class Test_Visualizer_Usage_Logger extends WP_UnitTestCase {

/**
* Create a published chart with the given settings meta value.
*
* @param mixed $settings The value stored in the visualizer-settings meta.
* @return int The chart id.
*/
private function create_chart( $settings ) {
$chart_id = self::factory()->post->create(
array(
'post_type' => Visualizer_Plugin::CPT_VISUALIZER,
'post_status' => 'publish',
'post_content' => wp_slash( serialize( array( array( 'Label' ), array( 'Value' ) ) ) ),
)
);
update_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_TYPE, 'line' );
update_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, $settings );
return $chart_id;
}

/**
* A chart whose settings meta is a string must not abort usage collection.
*/
public function test_string_settings_meta_does_not_crash_logger() {
$this->create_chart( 'corrupted string settings' );

$usage = apply_filters( 'visualizer_logger_data', array() );

$this->assertIsArray( $usage );
$this->assertSame( 0, $usage['manual_config'] );
}

/**
* A chart with no settings meta at all must not abort usage collection.
*/
public function test_missing_settings_meta_does_not_crash_logger() {
$chart_id = $this->create_chart( array() );
delete_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS );

$usage = apply_filters( 'visualizer_logger_data', array() );

$this->assertIsArray( $usage );
$this->assertSame( 0, $usage['manual_config'] );
}

/**
* On pro, a permission entry that should be an array but is a string
* must not abort usage collection with a count() TypeError.
*/
public function test_malformed_permissions_meta_does_not_crash_logger() {
// The stub stays defined for the rest of the PHPUnit process. That only
// affects code gating on class_exists( 'Visualizer_Pro' ) — the legacy
// license fallback in proFeaturesEnabled() — which no test exercises.
if ( ! class_exists( 'Visualizer_Pro' ) ) {
eval( 'class Visualizer_Pro { const CF_PERMISSIONS = "visualizer-permissions"; }' );
}

$chart_id = $this->create_chart( array() );
update_post_meta(
$chart_id,
Visualizer_Pro::CF_PERMISSIONS,
array( 'permissions' => array( 'edit-specific' => 'administrator' ) )
);

add_filter( 'visualizer_is_pro', '__return_true' );
$usage = apply_filters( 'visualizer_logger_data', array() );
remove_filter( 'visualizer_is_pro', '__return_true' );

$this->assertIsArray( $usage );
$this->assertSame( 0, $usage['permissions'] );
}

/**
* Valid array settings still count manual configurations.
*/
public function test_manual_config_still_counted_for_array_settings() {
$this->create_chart( array( 'manual' => '{"colors": ["#000"]}' ) );
$this->create_chart( 'corrupted string settings' );

$usage = apply_filters( 'visualizer_logger_data', array() );

$this->assertSame( 1, $usage['manual_config'] );
$this->assertSame( 2, $usage['types']['line'] );
}
}
Loading