Skip to content

Commit

Permalink
Element: Improve validation for attribute names in serializer (#8775)
Browse files Browse the repository at this point in the history
  • Loading branch information
gziolo committed Aug 9, 2018
1 parent 80d2d91 commit f5617ac
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 0 deletions.
28 changes: 28 additions & 0 deletions packages/element/src/serialize.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,19 @@ const CSS_PROPERTIES_SUPPORTS_UNITLESS = new Set( [
'zoom',
] );

/**
* Regular expression matching invalid attribute names.
*
* "Attribute names must consist of one or more characters other than controls,
* U+0020 SPACE, U+0022 ("), U+0027 ('), U+003E (>), U+002F (/), U+003D (=),
* and noncharacters."
*
* @link https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
*
* @type {RegExp}
*/
const REGEXP_INVALID_ATTRIBUTE_NAME = /[\u007F-\u009F "'>/="\uFDD0-\uFDEF]/;

/**
* Returns a string with ampersands escaped. Note that this is an imperfect
* implementation, where only ampersands which do not appear as a pattern of
Expand Down Expand Up @@ -301,6 +314,17 @@ export const escapeHTML = flowRight( [
escapeLessThan,
] );

/**
* Returns true if the given attribute name is valid, or false otherwise.
*
* @param {string} name Attribute name to test.
*
* @return {boolean} Whether attribute is valid.
*/
export function isValidAttributeName( name ) {
return ! REGEXP_INVALID_ATTRIBUTE_NAME.test( name );
}

/**
* Returns true if the specified string is prefixed by one of an array of
* possible prefixes.
Expand Down Expand Up @@ -556,6 +580,10 @@ export function renderAttributes( props ) {

for ( const key in props ) {
const attribute = getNormalAttributeName( key );
if ( ! isValidAttributeName( attribute ) ) {
continue;
}

let value = getNormalAttributeValue( key, props[ key ] );

// If value is not of serializeable type, skip.
Expand Down
44 changes: 44 additions & 0 deletions packages/element/src/test/serialize.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { noop } from 'lodash';
*/
import {
Component,
createElement,
Fragment,
RawHTML,
} from '../';
Expand All @@ -18,6 +19,7 @@ import serialize, {
escapeAttribute,
escapeHTML,
hasPrefix,
isValidAttributeName,
renderElement,
renderNativeComponent,
renderComponent,
Expand Down Expand Up @@ -71,7 +73,49 @@ describe( 'escapeHTML', () => {
testEscapeLessThan( escapeHTML );
} );

describe( 'isValidAttributeName', () => {
it( 'should return false for attribute with controls', () => {
const result = isValidAttributeName( 'bad\u007F' );

expect( result ).toBe( false );
} );

it( 'should return false for attribute with non-permitted characters', () => {
const result = isValidAttributeName( 'bad"' );

expect( result ).toBe( false );
} );

it( 'should return false for attribute with noncharacters', () => {
const result = isValidAttributeName( 'bad\uFDD0' );

expect( result ).toBe( false );
} );

it( 'should return true for valid attribute name', () => {
const result = isValidAttributeName( 'good' );

expect( result ).toBe( true );
} );
} );

describe( 'serialize()', () => {
it( 'should allow only valid attribute names', () => {
const element = createElement(
'div',
{
'notok\u007F': 'bad',
'notok"': 'bad',
ok: 'good',
'notok\uFDD0': 'bad',
},
);

const result = serialize( element );

expect( result ).toBe( '<div ok="good"></div>' );
} );

it( 'should render with context', () => {
class Provider extends Component {
getChildContext() {
Expand Down

0 comments on commit f5617ac

Please sign in to comment.