Skip to content

Commit

Permalink
Merge pull request #1004 from Automattic/fix/style-attribute-processing
Browse files Browse the repository at this point in the history
Eliminate use of safecss_filter_attr() in processing style attributes since redundant and lossy
  • Loading branch information
westonruter authored Mar 9, 2018
2 parents d0f4f01 + da3a779 commit 8079fcd
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 20 deletions.
25 changes: 9 additions & 16 deletions includes/sanitizers/class-amp-style-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -483,27 +483,20 @@ private function collect_inline_styles( $element ) {
*
* @since 0.4
*
* @param string $string Style string.
* @param string $css Style string.
* @return array Style properties.
*/
private function process_style( $string ) {
/*
* Filter properties
*
* @todo Removed values are not reported.
*/
$string = safecss_filter_attr( esc_html( $string ) );
private function process_style( $css ) {

if ( ! $string ) {
return array();
}
// Normalize whitespace.
$css = str_replace( array( "\n", "\r", "\t" ), '', $css );

/*
* safecss returns a string but we want individual rules.
* Use preg_split to break up rules by `;` but only if the
* semi-colon is not inside parens (like a data-encoded image).
*/
$styles = array_map( 'trim', preg_split( '/;(?![^(]*\))/', $string ) );
$styles = preg_split( '/\s*;\s*(?![^(]*\))/', trim( $css, '; ' ) );
$styles = array_filter( $styles );

// Normalize the order of the styles.
sort( $styles );
Expand All @@ -512,12 +505,12 @@ private function process_style( $string ) {

// Normalize whitespace and filter rules.
foreach ( $styles as $index => $rule ) {
$arr2 = array_map( 'trim', explode( ':', $rule, 2 ) );
if ( 2 !== count( $arr2 ) ) {
$tuple = preg_split( '/\s*:\s*/', $rule, 2 );
if ( 2 !== count( $tuple ) ) {
continue;
}

list( $property, $value ) = $this->filter_style( $arr2[0], $arr2[1] );
list( $property, $value ) = $this->filter_style( $tuple[0], $tuple[1] );
if ( empty( $property ) || empty( $value ) ) {
continue;
}
Expand Down
10 changes: 6 additions & 4 deletions tests/test-amp-style-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,12 @@ public function get_body_style_attribute_data() {
),
),

'div_kses_banned_style' => array(
'<span style="overflow-x: hidden;">Specific overflow axis not allowed.</span>',
'<span>Specific overflow axis not allowed.</span>',
array(),
'span_display_none' => array(
'<span style="display: none;">Kses-banned properties are allowed since Kses will have already applied if user does not have unfiltered_html.</span>',
'<span class="amp-wp-inline-0f1bf07c72fdf1784fff2e164d9dca98">Kses-banned properties are allowed since Kses will have already applied if user does not have unfiltered_html.</span>',
array(
'.amp-wp-inline-0f1bf07c72fdf1784fff2e164d9dca98 { display:none; }',
),
),

'div_amp_banned_style' => array(
Expand Down

0 comments on commit 8079fcd

Please sign in to comment.