Element: Avoid double-escaping valid character references (#6620)

packages/element/src/serialize.js

@@ -28,7 +28,7 @@
 /**
  * External dependencies
  */
-import { isEmpty, castArray, omit, kebabCase } from 'lodash';
+import { flowRight, isEmpty, castArray, omit, kebabCase } from 'lodash';
 
 /**
  * WordPress dependencies
@@ -224,6 +224,46 @@ const CSS_PROPERTIES_SUPPORTS_UNITLESS = new Set( [
 	'zoom',
 ] );
 
+/**
+ * Returns a string with ampersands escaped. Note that this is an imperfect
+ * implementation, where only ampersands which do not appear as a pattern of
+ * named, decimal, or hexadecimal character references are escaped. Invalid
+ * named references (i.e. ambiguous ampersand) are are still permitted.
+ *
+ * @link https://w3c.github.io/html/syntax.html#character-references
+ * @link https://w3c.github.io/html/syntax.html#ambiguous-ampersand
+ * @link https://w3c.github.io/html/syntax.html#named-character-references
+ *
+ * @param {string} value Original string.
+ *
+ * @return {string} Escaped string.
+ */
+export function escapeAmpersand( value ) {
+	return value.replace( /&(?!([a-z0-9]+|#[0-9]+|#x[a-f0-9]+);)/gi, '&' );
+}
+
+/**
+ * Returns a string with quotation marks replaced.
+ *
+ * @param {string} value Original string.
+ *
+ * @return {string} Escaped string.
+ */
+export function escapeQuotationMark( value ) {
+	return value.replace( /"/g, '"' );
+}
+
+/**
+ * Returns a string with less-than sign replaced.
+ *
+ * @param {string} value Original string.
+ *
+ * @return {string} Escaped string.
+ */
+export function escapeLessThan( value ) {
+	return value.replace( /</g, '&lt;' );
+}
+
 /**
  * Returns an escaped attribute value.
  *
@@ -236,15 +276,15 @@ const CSS_PROPERTIES_SUPPORTS_UNITLESS = new Set( [
  *
  * @return {string} Escaped attribute value.
  */
-function escapeAttribute( value ) {
-	return value.replace( /&/g, '&amp;' ).replace( /"/g, '&quot;' );
-}
+export const escapeAttribute = flowRight( [
+	escapeAmpersand,
+	escapeQuotationMark,
+] );
 
 /**
  * Returns an escaped HTML element value.
  *
  * @link https://w3c.github.io/html/syntax.html#writing-html-documents-elements
- * @link https://w3c.github.io/html/syntax.html#ambiguous-ampersand
  *
  * "the text must not contain the character U+003C LESS-THAN SIGN (<) or an
  * ambiguous ampersand."
@@ -253,9 +293,10 @@ function escapeAttribute( value ) {
  *
  * @return {string} Escaped HTML element value.
  */
-function escapeHTML( value ) {
-	return value.replace( /&/g, '&amp;' ).replace( /</g, '&lt;' );
-}
+export const escapeHTML = flowRight( [
+	escapeAmpersand,
+	escapeLessThan,
+] );
 
 /**
  * Returns true if the specified string is prefixed by one of an array of

packages/element/src/test/serialize.js

@@ -13,6 +13,11 @@ import {
 	RawHTML,
 } from '../';
 import serialize, {
+	escapeAmpersand,
+	escapeQuotationMark,
+	escapeLessThan,
+	escapeAttribute,
+	escapeHTML,
 	hasPrefix,
 	renderElement,
 	renderNativeComponent,
@@ -21,6 +26,52 @@ import serialize, {
 	renderStyle,
 } from '../serialize';
 
+function testEscapeAmpersand( implementation ) {
+	it( 'should escape ampersand', () => {
+		const result = implementation( 'foo & bar & & baz Σ &#bad; Σ Σ &#xevil;' );
+
+		expect( result ).toBe( 'foo & bar & & baz Σ &#bad; Σ Σ &#xevil;' );
+	} );
+}
+
+function testEscapeQuotationMark( implementation ) {
+	it( 'should escape quotation mark', () => {
+		const result = implementation( '"Be gone!"' );
+
+		expect( result ).toBe( '"Be gone!"' );
+	} );
+}
+
+function testEscapeLessThan( implementation ) {
+	it( 'should escape less than', () => {
+		const result = implementation( 'Chicken < Ribs' );
+
+		expect( result ).toBe( 'Chicken &lt; Ribs' );
+	} );
+}
+
+describe( 'escapeAmpersand', () => {
+	testEscapeAmpersand( escapeAmpersand );
+} );
+
+describe( 'escapeQuotationMark', () => {
+	testEscapeQuotationMark( escapeQuotationMark );
+} );
+
+describe( 'escapeLessThan', () => {
+	testEscapeLessThan( escapeLessThan );
+} );
+
+describe( 'escapeAttribute', () => {
+	testEscapeAmpersand( escapeAttribute );
+	testEscapeQuotationMark( escapeAttribute );
+} );
+
+describe( 'escapeHTML', () => {
+	testEscapeAmpersand( escapeHTML );
+	testEscapeLessThan( escapeHTML );
+} );
+
 describe( 'serialize()', () => {
 	it( 'should render with context', () => {
 		class Provider extends Component {
@@ -155,7 +206,7 @@ describe( 'renderElement()', () => {
 	it( 'renders escaped string element', () => {
 		const result = renderElement( 'hello & world &amp; friends <img/>' );
 
-		expect( result ).toBe( 'hello &amp; world &amp;amp; friends &lt;img/>' );
+		expect( result ).toBe( 'hello &amp; world &amp; friends &lt;img/>' );
 	} );
 
 	it( 'renders numeric element as string', () => {