Skip to content

Commit

Permalink
Fixes #6953 LRC replace DOM output with regex injection (#6954)
Browse files Browse the repository at this point in the history
Co-authored-by: Gael Robin <[email protected]>
  • Loading branch information
MathieuLamiot and Miraeld authored Sep 11, 2024
1 parent 22cad0a commit 2edb082
Show file tree
Hide file tree
Showing 14 changed files with 1,320 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,24 @@ class Dom implements ProcessorInterface {

use HelperTrait;

/**
* Number of injects hashes.
*
* @since 3.17
*
* @var int
*/
private $count;

/**
* Maximum number of hashes to inject.
*
* @since 3.17
*
* @var int
*/
private $max_hashes;

/**
* Add hashes to the HTML elements
*
Expand Down Expand Up @@ -44,27 +62,35 @@ public function add_hashes( $html ) {
return $html;
}

$this->add_hash_to_element( $body, $this->get_depth() );
$this->max_hashes = $this->get_max_tags();
$this->count = 0;

return $dom->saveHTML();
return $this->add_hash_to_element( $body, $this->get_depth(), $html );
}

/**
* Add a hash to the element and its children.
*
* @param \DOMElement $element The element to add the hash to.
* @param int $depth The depth of the recursion.
* @param string $html The HTML content.
*
* @return string
*/
private function add_hash_to_element( $element, $depth ) {
private function add_hash_to_element( $element, $depth, $html ) {

Check warning on line 80 in inc/Engine/Optimization/LazyRenderContent/Frontend/Processor/Dom.php

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

inc/Engine/Optimization/LazyRenderContent/Frontend/Processor/Dom.php#L80

The method add_hash_to_element() has a Cyclomatic Complexity of 10. The configured cyclomatic complexity threshold is 10.
if ( $depth < 0 ) {
return;
return $html;
}

$processed_tags = $this->get_processed_tags();

static $count = 0;

foreach ( $element->childNodes as $child ) { // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase

if ( $this->count >= $this->max_hashes ) {
Logger::warning( 'Stopping LRC hash injection as max_hashes is reached.', [ 'LazyRenderContent' ] );
return $html;
}

if ( ! $child instanceof \DOMElement ) {
continue;
}
Expand All @@ -81,15 +107,32 @@ private function add_hash_to_element( $element, $depth ) {
$child_html = $child->ownerDocument->saveHTML( $child ); // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase
$opening_tag_html = strstr( $child_html, '>', true ) . '>';

$hash = md5( $opening_tag_html . $count );
$hash = md5( $opening_tag_html . $this->count );

++$count;
++$this->count;

// Add the data-rocket-location-hash attribute.
$child->setAttribute( 'data-rocket-location-hash', $hash );
// Inject the hash as an attribute in the opening tag.
$replace = preg_replace( '/' . $child->tagName . '/is', '$0 data-rocket-location-hash="' . $hash . '"', $opening_tag_html, 1 ); // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase
if ( is_null( $replace ) ) {
continue;
}
// Replace the opening tag in the HTML by the manipulated one
// If DOMDocument automatically modified the original element, we might not find it in the HTML.
// Known issue: if there is an element with the exact same opening tag before in the HTML that did not receive a hash, it will replaced instead of the correct element in the HTML.
$element_replacements = 0;
$modified_html = preg_replace( '/' . preg_quote( $opening_tag_html, '/' ) . '/', $replace, $html, 1, $element_replacements );
if ( $element_replacements < 1 ) {
Logger::warning( 'Opening tag from DOMDocument not found in original HTML.', [ 'LazyRenderContent' ] );
}
if ( is_null( $modified_html ) ) {
continue;
}
$html = $modified_html;

// Recursively process child elements.
$this->add_hash_to_element( $child, $depth - 1 );
$html = $this->add_hash_to_element( $child, $depth - 1, $html );
}

return $html;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ trait HelperTrait {
/**
* Get filtered elements depth.
*
* @since 3.17
*
* @return int
*/
protected function get_depth() {
Expand All @@ -20,6 +22,23 @@ protected function get_depth() {
return wpm_apply_filters_typed( 'integer', 'rocket_lrc_depth', 2 );
}

/**
* Get filtered element maximum count.
*
* @since 3.17
*
* @return int
*/
protected function get_max_tags() {
/**
* Filter the maximal number of processed tags.
* High values allow to process more elements but expose to a risk of performance issue because of the regex replacement process.
*
* @param int $depth Depth value.
*/
return wpm_apply_filters_typed( 'integer', 'rocket_lrc_max_hashes', 200 );
}

/**
* Get processed tags.
*
Expand All @@ -29,6 +48,8 @@ protected function get_processed_tags() {
/**
* Filter the processed element tags.
*
* @since 3.17
*
* @param array|string[] $tags Tags to be processed.
*/
$tags = wpm_apply_filters_typed(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@ class Regex implements ProcessorInterface {

use HelperTrait;

/**
* Number of injects hashes.
*
* @since 3.17
*
* @var int
*/
private $count;

/**
* Maximum number of hashes to inject.
*
* @since 3.17
*
* @var int
*/
private $max_hashes;

/**
* Add hashes to the HTML elements
*
Expand All @@ -25,6 +43,9 @@ public function add_hashes( $html ) {
return $html;
}

$this->max_hashes = $this->get_max_tags();
$this->count = 0;

return $this->add_hash_to_element( $html, $matches[0] );
}

Expand All @@ -47,15 +68,19 @@ private function add_hash_to_element( $html, $element ) {
return $html;
}

$count = 0;

foreach ( $matches as $child ) {

if ( $this->count >= $this->max_hashes ) {
Logger::warning( 'Stopping LRC hash injection as max_hashes is reached.', [ 'LazyRenderContent' ] );
return $html;
}

// Calculate the hash of the opening tag.
$opening_tag_html = strstr( $child[0], '>', true ) . '>';

$hash = md5( $opening_tag_html . $count );
$hash = md5( $opening_tag_html . $this->count );

++$count;
++$this->count;

// Add the data-rocket-location-hash attribute.
$replace = preg_replace( '/' . $child[1] . '/is', '$0 data-rocket-location-hash="' . $hash . '"', $child[0], 1 );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,24 @@ class SimpleHtmlDom implements ProcessorInterface {

use HelperTrait;

/**
* Number of injects hashes.
*
* @since 3.17
*
* @var int
*/
private $count;

/**
* Maximum number of hashes to inject.
*
* @since 3.17
*
* @var int
*/
private $max_hashes;

/**
* Add hashes to the HTML elements
*
Expand All @@ -30,6 +48,9 @@ public function add_hashes( $html ) {
return $html;
}

$this->max_hashes = $this->get_max_tags();
$this->count = 0;

$this->add_hash_to_element( $body, $this->get_depth() );

return $dom->save();
Expand All @@ -48,9 +69,13 @@ private function add_hash_to_element( $element, $depth ) {

$processed_tags = $this->get_processed_tags();

static $count = 0;

foreach ( $element->childNodes() as $child ) {

if ( $this->count >= $this->max_hashes ) {
Logger::warning( 'Stopping LRC hash injection as max_hashes is reached.', [ 'LazyRenderContent' ] );
return;
}

if ( ! in_array( strtoupper( $child->getTag() ), $processed_tags, true ) ) {
continue;
}
Expand All @@ -59,9 +84,9 @@ private function add_hash_to_element( $element, $depth ) {
$child_html = $child->html();
$opening_tag_html = strstr( $child_html, '>', true ) . '>';

$hash = md5( $opening_tag_html . $count );
$hash = md5( $opening_tag_html . $this->count );

++$count;
++$this->count;

// Add the data-rocket-location-hash attribute.
$child->setAttribute( 'data-rocket-location-hash', $hash );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<article data-rocket-location-hash="d1f41b6001aa95d1577259dd681a9b19">
<header data-rocket-location-hash="fbfcccd11db41b93d3d0676c9e14fdc8">
<h1>Original</h1>
<div></div>
<div attribute-added-to-bypass-dom-processor-known-issue="1"></div>
</header>
<section data-rocket-location-hash="75202dec0fcc2df72399263306a2e2e7">
<h2>Text</h2>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<article data-rocket-location-hash="d1f41b6001aa95d1577259dd681a9b19">
<header data-rocket-location-hash="fbfcccd11db41b93d3d0676c9e14fdc8">
<h1>Original</h1>
<div data-rocket-location-hash="7b16eca0652d4703f83ba63e304f2030"></div>
<div data-rocket-location-hash="d9527fe8d9073f13915c4ae1183acd6a" attribute-added-to-bypass-dom-processor-known-issue="1"></div>
</header>
<section data-rocket-location-hash="372b731e1cb055b2c1a710227f2f6cd6">
<h2>Text</h2>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<article>
<header>
<h1>Original</h1>
<div></div>
<div attribute-added-to-bypass-dom-processor-known-issue="1"></div>
</header>
<section>
<h2>Text</h2>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,39 @@
'expected' => [
'html' => file_get_contents( WP_ROCKET_TESTS_FIXTURES_DIR . '/inc/Engine/Optimization/LazyRenderContent/Frontend/Subscriber/html/expected.php' ),
]
],
'shouldStopAt200Hashes' => [
'config' => [
'row' => [
'url' => 'http://example.org/',
'is_mobile' => 0,
'below_the_fold' => json_encode(
[]
),
'status' => 'completed'
],
'html' => file_get_contents( WP_ROCKET_TESTS_FIXTURES_DIR . '/inc/Engine/Optimization/LazyRenderContent/Frontend/Subscriber/html/long_original.php' ),
],
'expected' => [
'html' => file_get_contents( WP_ROCKET_TESTS_FIXTURES_DIR . '/inc/Engine/Optimization/LazyRenderContent/Frontend/Subscriber/html/long_expected_200_hashes.php' ),
]
],
'shouldStopAtFilteredMaxHashes' => [
'config' => [
'row' => [
'url' => 'http://example.org/',
'is_mobile' => 0,
'below_the_fold' => json_encode(
[]
),
'status' => 'completed'
],
'html' => file_get_contents( WP_ROCKET_TESTS_FIXTURES_DIR . '/inc/Engine/Optimization/LazyRenderContent/Frontend/Subscriber/html/long_original.php' ),
'max_hashes' => 150,
],
'expected' => [
'html' => file_get_contents( WP_ROCKET_TESTS_FIXTURES_DIR . '/inc/Engine/Optimization/LazyRenderContent/Frontend/Subscriber/html/long_expected_150_hashes.php' ),
]
]
]
];

Large diffs are not rendered by default.

Loading

0 comments on commit 2edb082

Please sign in to comment.