Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate and protect corrupted block content #1929

Merged
merged 5 commits into from
Jul 26, 2017
Merged
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
48 changes: 45 additions & 3 deletions blocks/api/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { pickBy } from 'lodash';
import { parse as grammarParse } from './post.pegjs';
import { getBlockType, getUnknownTypeHandler } from './registration';
import { createBlock } from './factory';
import { getBeautifulContent, getSaveContent } from './serializer';

/**
* Returns the block attributes parsed from raw content.
Expand Down Expand Up @@ -81,18 +82,59 @@ export function createBlockWithFallback( name, rawContent, attributes ) {

// Include in set only if type were determined.
// TODO do we ever expect there to not be an unknown type handler?
if ( blockType && ( rawContent.trim() || name !== fallbackBlock ) ) {
if ( blockType && ( rawContent || name !== fallbackBlock ) ) {
// TODO allow blocks to opt-in to receiving a tree instead of a string.
// Gradually convert all blocks to this new format, then remove the
// string serialization.
const block = createBlock(
name,
getBlockAttributes( blockType, rawContent.trim(), attributes )
getBlockAttributes( blockType, rawContent, attributes )
);

// Validate that the parsed block is valid, meaning that if we were to
// reserialize it given the assumed attributes, the markup matches the
// original value. Otherwise, preserve original to avoid destruction.
block.isValid = isValidBlock( rawContent, blockType, block.attributes );
if ( ! block.isValid ) {
block.originalContent = rawContent;
}

return block;
}
}

/**
* Returns true if the parsed block is valid given the input content. A block
* is considered valid if, when serialized with assumed attributes, the content
* matches the original value.
*
* Logs to console in development environments when invalid.
*
* @param {String} rawContent Original block content
* @param {String} blockType Block type
* @param {Object} attributes Parsed block attributes
* @return {Boolean} Whether block is valid
*/
export function isValidBlock( rawContent, blockType, attributes ) {
const [ actual, expected ] = [
rawContent,
getSaveContent( blockType, attributes ),
].map( getBeautifulContent );

const isValid = ( actual === expected );

if ( ! isValid && 'development' === process.env.NODE_ENV ) {
// eslint-disable-next-line no-console
console.error(
'Invalid block parse\n' +
'\tExpected: ' + expected + '\n' +
'\tActual: ' + actual
);
}

return isValid;
}

/**
* Parses the post content with a PegJS grammar and returns a list of blocks.
*
Expand All @@ -102,7 +144,7 @@ export function createBlockWithFallback( name, rawContent, attributes ) {
export function parseWithGrammar( content ) {
return grammarParse( content ).reduce( ( memo, blockNode ) => {
const { blockName, rawContent, attrs } = blockNode;
const block = createBlockWithFallback( blockName, rawContent, attrs );
const block = createBlockWithFallback( blockName, rawContent.trim(), attrs );
if ( block ) {
memo.push( block );
}
Expand Down
33 changes: 25 additions & 8 deletions blocks/api/serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,33 @@ export function serializeAttributes( attrs ) {
.replace( /&/g, '\\u0026' ); // ibid
}

/**
* Returns HTML markup processed by a markup beautifier configured for use in
* block serialization.
*
* @param {String} content Original HTML
* @return {String} Beautiful HTML
*/
export function getBeautifulContent( content ) {
return beautifyHtml( content, {
indent_inner_html: true,
wrap_line_length: 0,
} );
}

export function serializeBlock( block ) {
const blockName = block.name;
const blockType = getBlockType( blockName );
const saveContent = getSaveContent( blockType, block.attributes );

let saveContent;
if ( block.isValid ) {
saveContent = getSaveContent( blockType, block.attributes );
} else {
// If block was parsed as invalid, skip serialization behavior and opt
// to use original content instead so we don't destroy user content.
saveContent = block.originalContent;
}

const saveAttributes = getCommentAttributes( block.attributes, parseBlockAttributes( saveContent, blockType.attributes ) );

if ( 'wp:core/more' === blockName ) {
Expand All @@ -131,13 +154,7 @@ export function serializeBlock( block ) {

return (
`<!-- wp:${ blockName } ${ serializedAttributes }-->\n` +

/** make more readable - @see https://github.com/WordPress/gutenberg/pull/663 */
beautifyHtml( saveContent, {
indent_inner_html: true,
wrap_line_length: 0,
} ) +

getBeautifulContent( saveContent ) +
`\n<!-- /wp:${ blockName } -->`
);
}
Expand Down
28 changes: 27 additions & 1 deletion blocks/api/test/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,22 @@ import { text } from '../query';
import {
getBlockAttributes,
parseBlockAttributes,
isValidBlock,
createBlockWithFallback,
default as parse,
} from '../parser';
import {
registerBlockType,
unregisterBlockType,
getBlockTypes,
getBlockType,
setUnknownTypeHandler,
} from '../registration';

describe( 'block parser', () => {
const defaultBlockSettings = { save: noop };
const defaultBlockSettings = {
save: ( { attributes } ) => attributes.fruit,
};

afterEach( () => {
setUnknownTypeHandler( undefined );
Expand Down Expand Up @@ -89,6 +93,28 @@ describe( 'block parser', () => {
} );
} );

describe( 'isValidBlock()', () => {
it( 'returns false is block is not valid', () => {
registerBlockType( 'core/test-block', defaultBlockSettings );

expect( isValidBlock(
'Apples',
getBlockType( 'core/test-block' ),
{ fruit: 'Bananas' }
) ).toBe( false );
} );

it( 'returns true is block is valid', () => {
registerBlockType( 'core/test-block', defaultBlockSettings );

expect( isValidBlock(
'Bananas',
getBlockType( 'core/test-block' ),
{ fruit: 'Bananas' }
) ).toBe( true );
} );
} );

describe( 'createBlockWithFallback', () => {
it( 'should create the requested block if it exists', () => {
registerBlockType( 'core/test-block', defaultBlockSettings );
Expand Down
16 changes: 15 additions & 1 deletion blocks/api/test/serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ import { createElement, Component } from 'element';
/**
* Internal dependencies
*/
import serialize, { getCommentAttributes, getSaveContent, serializeAttributes } from '../serializer';
import serialize, {
getCommentAttributes,
getBeautifulContent,
getSaveContent,
serializeAttributes,
} from '../serializer';
import { getBlockTypes, registerBlockType, unregisterBlockType } from '../registration';

describe( 'block serializer', () => {
Expand All @@ -16,6 +21,14 @@ describe( 'block serializer', () => {
} );
} );

describe( 'getBeautifulContent()', () => {
it( 'returns beautiful content', () => {
const content = getBeautifulContent( '<div><div>Beautiful</div></div>' );

expect( content ).toBe( '<div>\n <div>Beautiful</div>\n</div>' );
} );
} );

describe( 'getSaveContent()', () => {
describe( 'function save', () => {
it( 'should return string verbatim', () => {
Expand Down Expand Up @@ -185,6 +198,7 @@ describe( 'block serializer', () => {
content: 'Ribs & Chicken',
stuff: 'left & right -- but <not>',
},
isValid: true,
},
];
const expectedPostContent = '<!-- wp:core/test-block {"stuff":"left \\u0026 right \\u002d\\u002d but \\u003cnot\\u003e"} -->\n<p class="wp-block-test-block">Ribs & Chicken</p>\n<!-- /wp:core/test-block -->';
Expand Down
3 changes: 2 additions & 1 deletion blocks/test/fixtures/core-embed__animoto.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"caption": [
"Embedded content from animoto"
]
}
},
"isValid": true
}
]
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core-embed__animoto.serialized.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
https://animoto.com/
<figcaption>Embedded content from animoto</figcaption>
</figure>
<!-- /wp:core-embed/animoto -->
<!-- /wp:core-embed/animoto -->
3 changes: 2 additions & 1 deletion blocks/test/fixtures/core-embed__cloudup.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"caption": [
"Embedded content from cloudup"
]
}
},
"isValid": true
}
]
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core-embed__cloudup.serialized.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
https://cloudup.com/
<figcaption>Embedded content from cloudup</figcaption>
</figure>
<!-- /wp:core-embed/cloudup -->
<!-- /wp:core-embed/cloudup -->
3 changes: 2 additions & 1 deletion blocks/test/fixtures/core-embed__collegehumor.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"caption": [
"Embedded content from collegehumor"
]
}
},
"isValid": true
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
https://collegehumor.com/
<figcaption>Embedded content from collegehumor</figcaption>
</figure>
<!-- /wp:core-embed/collegehumor -->
<!-- /wp:core-embed/collegehumor -->
3 changes: 2 additions & 1 deletion blocks/test/fixtures/core-embed__dailymotion.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"caption": [
"Embedded content from dailymotion"
]
}
},
"isValid": true
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
https://dailymotion.com/
<figcaption>Embedded content from dailymotion</figcaption>
</figure>
<!-- /wp:core-embed/dailymotion -->
<!-- /wp:core-embed/dailymotion -->
3 changes: 2 additions & 1 deletion blocks/test/fixtures/core-embed__facebook.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"caption": [
"Embedded content from facebook"
]
}
},
"isValid": true
}
]
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core-embed__facebook.serialized.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
https://facebook.com/
<figcaption>Embedded content from facebook</figcaption>
</figure>
<!-- /wp:core-embed/facebook -->
<!-- /wp:core-embed/facebook -->
3 changes: 2 additions & 1 deletion blocks/test/fixtures/core-embed__flickr.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"caption": [
"Embedded content from flickr"
]
}
},
"isValid": true
}
]
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core-embed__flickr.serialized.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
https://flickr.com/
<figcaption>Embedded content from flickr</figcaption>
</figure>
<!-- /wp:core-embed/flickr -->
<!-- /wp:core-embed/flickr -->
3 changes: 2 additions & 1 deletion blocks/test/fixtures/core-embed__funnyordie.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"caption": [
"Embedded content from funnyordie"
]
}
},
"isValid": true
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
https://funnyordie.com/
<figcaption>Embedded content from funnyordie</figcaption>
</figure>
<!-- /wp:core-embed/funnyordie -->
<!-- /wp:core-embed/funnyordie -->
3 changes: 2 additions & 1 deletion blocks/test/fixtures/core-embed__hulu.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"caption": [
"Embedded content from hulu"
]
}
},
"isValid": true
}
]
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core-embed__hulu.serialized.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
https://hulu.com/
<figcaption>Embedded content from hulu</figcaption>
</figure>
<!-- /wp:core-embed/hulu -->
<!-- /wp:core-embed/hulu -->
3 changes: 2 additions & 1 deletion blocks/test/fixtures/core-embed__imgur.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"caption": [
"Embedded content from imgur"
]
}
},
"isValid": true
}
]
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core-embed__imgur.serialized.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
https://imgur.com/
<figcaption>Embedded content from imgur</figcaption>
</figure>
<!-- /wp:core-embed/imgur -->
<!-- /wp:core-embed/imgur -->
3 changes: 2 additions & 1 deletion blocks/test/fixtures/core-embed__instagram.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"caption": [
"Embedded content from instagram"
]
}
},
"isValid": true
}
]
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core-embed__instagram.serialized.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
https://instagram.com/
<figcaption>Embedded content from instagram</figcaption>
</figure>
<!-- /wp:core-embed/instagram -->
<!-- /wp:core-embed/instagram -->
3 changes: 2 additions & 1 deletion blocks/test/fixtures/core-embed__issuu.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"caption": [
"Embedded content from issuu"
]
}
},
"isValid": true
}
]
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core-embed__issuu.serialized.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
https://issuu.com/
<figcaption>Embedded content from issuu</figcaption>
</figure>
<!-- /wp:core-embed/issuu -->
<!-- /wp:core-embed/issuu -->
Loading