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

Add greater than symbol to attribute escaping #9963

Merged
merged 7 commits into from
Mar 18, 2019
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
2 changes: 1 addition & 1 deletion packages/element/src/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe( 'element', () => {
}, '<"WordPress" & Friends>' ) );

expect( result ).toBe(
'<a href="/index.php?foo=bar&amp;qux=<&quot;scary&quot;>" style="background-color:red">' +
'<a href="/index.php?foo=bar&amp;qux=<&quot;scary&quot;&gt;" style="background-color:red">' +
'&lt;"WordPress" &amp; Friends>' +
'</a>'
);
Expand Down
2 changes: 1 addition & 1 deletion packages/element/src/test/serialize.js
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ describe( 'renderAttributes()', () => {
href: '/index.php?foo=bar&qux=<"scary">',
} );

expect( result ).toBe( ' style="background:url(&quot;foo.png&quot;)" href="/index.php?foo=bar&amp;qux=<&quot;scary&quot;>"' );
expect( result ).toBe( ' style="background:url(&quot;foo.png&quot;)" href="/index.php?foo=bar&amp;qux=<&quot;scary&quot;&gt;"' );
} );

it( 'should render numeric attributes', () => {
Expand Down
4 changes: 4 additions & 0 deletions packages/escape-html/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.1.1 (Unreleased)

- Add fix for WordPress wptexturize greater-than tokenize bug (see https://core.trac.wordpress.org/ticket/45387)

## 1.0.1 (2018-10-19)

## 1.0.0 (2018-10-18)
Expand Down
20 changes: 14 additions & 6 deletions packages/escape-html/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ _This package assumes that your code will run in an **ES2015+** environment. If

### escapeAmpersand

[src/index.js#L28-L30](src/index.js#L28-L30)
[src/index.js#L33-L35](src/index.js#L33-L35)

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 All @@ -41,7 +41,7 @@ named references (i.e. ambiguous ampersand) are are still permitted.

### escapeAttribute

[src/index.js#L66-L68](src/index.js#L66-L68)
[src/index.js#L79-L81](src/index.js#L79-L81)

Returns an escaped attribute value.

Expand All @@ -52,6 +52,14 @@ Returns an escaped attribute value.
"[...] the text cannot contain an ambiguous ampersand [...] must not contain
any literal U+0022 QUOTATION MARK characters (")"

Note we also escape the greater than symbol, as this is used by wptexturize to
split HTML strings. This is a WordPress specific fix

Note that if a resolution for Trac#45387 comes to fruition, it is no longer
necessary for `__unstableEscapeGreaterThan` to be used.

See: <https://core.trac.wordpress.org/ticket/45387>

**Parameters**

- **value** `string`: Attribute value.
Expand All @@ -62,7 +70,7 @@ any literal U+0022 QUOTATION MARK characters (")"

### escapeHTML

[src/index.js#L82-L84](src/index.js#L82-L84)
[src/index.js#L95-L97](src/index.js#L95-L97)

Returns an escaped HTML element value.

Expand All @@ -83,7 +91,7 @@ ambiguous ampersand."

### escapeLessThan

[src/index.js#L50-L52](src/index.js#L50-L52)
[src/index.js#L55-L57](src/index.js#L55-L57)

Returns a string with less-than sign replaced.

Expand All @@ -97,7 +105,7 @@ Returns a string with less-than sign replaced.

### escapeQuotationMark

[src/index.js#L39-L41](src/index.js#L39-L41)
[src/index.js#L44-L46](src/index.js#L44-L46)

Returns a string with quotation marks replaced.

Expand All @@ -111,7 +119,7 @@ Returns a string with quotation marks replaced.

### isValidAttributeName

[src/index.js#L93-L95](src/index.js#L93-L95)
[src/index.js#L106-L108](src/index.js#L106-L108)

Returns true if the given attribute name is valid, or false otherwise.

Expand Down
2 changes: 1 addition & 1 deletion packages/escape-html/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@wordpress/escape-html",
"version": "1.1.0",
"version": "1.1.1",
"description": "Escape HTML utils.",
"author": "The WordPress Contributors",
"license": "GPL-2.0-or-later",
Expand Down
15 changes: 15 additions & 0 deletions packages/escape-html/src/escape-greater.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Returns a string with greater-than sign replaced.
*
* Note that if a resolution for Trac#45387 comes to fruition, it is no longer
* necessary for `__unstableEscapeGreaterThan` to exist.
*
* See: https://core.trac.wordpress.org/ticket/45387
*
* @param {string} value Original string.
*
* @return {string} Escaped string.
*/
export default function __unstableEscapeGreaterThan( value ) {
return value.replace( />/g, '&gt;' );
}
15 changes: 14 additions & 1 deletion packages/escape-html/src/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* Internal dependencies
*/
import __unstableEscapeGreaterThan from './escape-greater';

/**
* Regular expression matching invalid attribute names.
*
Expand Down Expand Up @@ -59,12 +64,20 @@ export function escapeLessThan( value ) {
* "[...] the text cannot contain an ambiguous ampersand [...] must not contain
* any literal U+0022 QUOTATION MARK characters (")"
*
* Note we also escape the greater than symbol, as this is used by wptexturize to
* split HTML strings. This is a WordPress specific fix
*
johngodley marked this conversation as resolved.
Show resolved Hide resolved
* Note that if a resolution for Trac#45387 comes to fruition, it is no longer
* necessary for `__unstableEscapeGreaterThan` to be used.
*
* See: https://core.trac.wordpress.org/ticket/45387
*
* @param {string} value Attribute value.
*
* @return {string} Escaped attribute value.
*/
export function escapeAttribute( value ) {
return escapeQuotationMark( escapeAmpersand( value ) );
return __unstableEscapeGreaterThan( escapeQuotationMark( escapeAmpersand( value ) ) );
}

/**
Expand Down
13 changes: 13 additions & 0 deletions packages/escape-html/src/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ import {
escapeHTML,
isValidAttributeName,
} from '../';
import __unstableEscapeGreaterThan from '../escape-greater';

function testUnstableEscapeGreaterThan( implementation ) {
it( 'should escape greater than', () => {
const result = implementation( 'Chicken > Ribs' );
expect( result ).toBe( 'Chicken &gt; Ribs' );
} );
}

function testEscapeAmpersand( implementation ) {
it( 'should escape ampersand', () => {
Expand Down Expand Up @@ -46,9 +54,14 @@ describe( 'escapeLessThan', () => {
testEscapeLessThan( escapeLessThan );
} );

describe( 'escapeGreaterThan', () => {
testUnstableEscapeGreaterThan( __unstableEscapeGreaterThan );
} );

describe( 'escapeAttribute', () => {
testEscapeAmpersand( escapeAttribute );
testEscapeQuotationMark( escapeAttribute );
testUnstableEscapeGreaterThan( escapeAttribute );
} );

describe( 'escapeHTML', () => {
Expand Down