Skip to content

Commit 3e55270

Browse files
committed
Builder: refuse a layout whose rows, cells or widgets are not lists
The loader now checks the same top-level shape the server accepts before building rows, so a stored value with a non-list member locks the builder instead of presenting an empty one that an edit could save over the stored layout. The nested widget test now proves sanitize_all() runs on an array layout and does not run on the preserving paths.
1 parent 06c0920 commit 3e55270

3 files changed

Lines changed: 46 additions & 2 deletions

File tree

js/siteorigin-panels/model/builder.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,25 @@ module.exports = Backbone.Model.extend({
9797
// what is stored.
9898
var isEmptyContainer = ! _.isNull( data ) && typeof data === 'object' && _.isEmpty( data );
9999

100-
if ( data !== false && ! _.isUndefined( data ) && ! isEmptyContainer ) {
100+
if ( data !== false && ! isEmptyContainer ) {
101101
throw new Error( 'Layout data has no grid cells' );
102102
}
103103

104104
this.finishLoad();
105105
return;
106106
}
107107

108+
// The same shape the server accepts: rows and cells are lists, and so
109+
// are the widgets when present. Anything else is not a layout, and an
110+
// unlocked empty builder would let an edit replace what is stored.
111+
if (
112+
! _.isArray( data.grid_cells ) ||
113+
! _.isArray( data.grids ) ||
114+
( ! _.isUndefined( data.widgets ) && ! _.isArray( data.widgets ) )
115+
) {
116+
throw new Error( 'Layout data is not a layout' );
117+
}
118+
108119
var gi;
109120
for ( var ci = 0; ci < data.grid_cells.length; ci ++ ) {
110121
gi = parseInt( data.grid_cells[ci].grid );

tests/js/builder-load-failure.test.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,26 @@ test( 'a scalar, a non-empty list or an unrelated object is a failed load', func
260260
} );
261261
} );
262262

263+
test( 'a layout-shaped object with a non-list member is a failed load', function () {
264+
[
265+
{ grids: 'x', grid_cells: [] },
266+
{ grids: [], grid_cells: 1 },
267+
{ widgets: '', grids: [], grid_cells: [] },
268+
{ grids: [ { cells: 1, style: {} } ], grid_cells: { 0: { grid: 0, index: 0, weight: 1 } } }
269+
].forEach( function ( bad ) {
270+
const model = builder();
271+
const events = countEvents( model );
272+
273+
model.loadPanelsData( bad );
274+
275+
assert.equal( model.loadFailed, true, JSON.stringify( bad ) );
276+
assert.equal( events.load_panels_data, 0, JSON.stringify( bad ) );
277+
278+
model.refreshPanelsData();
279+
assert.equal( events[ 'change:data' ], 0, JSON.stringify( bad ) );
280+
} );
281+
} );
282+
263283
test( 'an empty object or list is a successful empty load', function () {
264284
[ {}, [] ].forEach( function ( empty ) {
265285
const model = builder();

tests/save-post/SavePostMalformedPayloadTest.php

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,12 @@ private function require_admin_class() {
5757
eval(
5858
'class SiteOrigin_Panels_Styles_Admin {'
5959
. ' private static $instance;'
60+
. ' public static $sanitize_calls = 0;'
6061
. ' public static function single() {'
6162
. ' if ( empty( self::$instance ) ) { self::$instance = new self(); }'
6263
. ' return self::$instance;'
6364
. ' }'
64-
. ' public function sanitize_all( $panels_data ) { return $panels_data; }'
65+
. ' public function sanitize_all( $panels_data ) { self::$sanitize_calls++; return $panels_data; }'
6566
. '}'
6667
);
6768
}
@@ -353,11 +354,23 @@ public function test_layout_widget_returns_old_instance_on_malformed_string() {
353354
public function test_layout_widget_still_sanitizes_an_array_layout() {
354355
$old = array( 'builder_id' => 'old', 'panels_data' => array() );
355356
$new = array( 'panels_data' => $this->valid_layout() );
357+
$calls_before = \SiteOrigin_Panels_Styles_Admin::$sanitize_calls;
356358

357359
$updated = $this->widget()->update( $new, $old );
358360

359361
$this->assertSame( $this->valid_layout(), $updated['panels_data'] );
360362
$this->assertNotSame( 'old', $updated['builder_id'] );
363+
$this->assertSame( $calls_before + 1, \SiteOrigin_Panels_Styles_Admin::$sanitize_calls, 'sanitize_all() ran on the array layout' );
364+
}
365+
366+
public function test_layout_widget_does_not_sanitize_when_the_field_is_absent_or_refused() {
367+
$old = array( 'builder_id' => 'old', 'panels_data' => $this->valid_layout() );
368+
$calls_before = \SiteOrigin_Panels_Styles_Admin::$sanitize_calls;
369+
370+
$this->widget()->update( array( 'title' => 'x' ), $old );
371+
$this->widget()->update( array( 'panels_data' => '{bad' ), $old );
372+
373+
$this->assertSame( $calls_before, \SiteOrigin_Panels_Styles_Admin::$sanitize_calls, 'nothing to sanitize on the preserving paths' );
361374
}
362375

363376
public function test_layout_widget_decodes_a_valid_string_layout() {

0 commit comments

Comments
 (0)