Optimization Detective's XPath expressions in HTML can be erroneously interpreted as JS/CSS comments

[westonruter]

Bug Description

I just discovered that certain plugins (example) may implement HTML compression that collapses whitespace and removes JS/CSS comments by means of regular expressions. For example (do not do this):

$buffer = preg_replace( '@\/\*(.*?)\*\/@s', ' ', $buffer );

This is dangerous and will often result in corrupted HTML markup.

This has the effect of turning this:

<p>This is an XPath: <code>/HTML/BODY/*[1][self::DIV]</code></p>

<p>This is a Script:</p>

<pre class="wp-block-code"><code>&lt;script>
/* example script */
&lt;/script></code></pre>

Into:

<p>This is an XPath: <code>/HTML/BODY &lt;/script&gt;</code></pre>

This is because the regular expression starts matching the supposed comment start in the first paragraph which mentions /HTML/BODY/* and then it terminates the "comment" in the code sample in the third block.

This will break pages that contain data-od-xpath attributes which Optimization Detective adds when detection is needed.

Granted, such regular expression logic will invariably cause many more problems than this, but we might want to consider hardening the data-od-xpath attribute to prevent this from happening, namely by doing something like base64-encoding it in the attribute via base64_encode() in PHP. It could then be converted back to the non-encoded form in JavaScript via atob().

Just something to consider.

[nani-samireddy]

Hi, I'm a bit new to the performance plugin but I would like to work on this.

[sarthak-19]

Hi, @nani-samireddy welcome to WP-Performance community, feel free to reach out if stuck while solving this issue. 😄

[nani-samireddy]

Thank you @sarthak-19

I have spent few hours on this and here are my findings:

Here are setting the xpath attribute in the PHP. So we can change the following

performance/plugins/optimization-detective/optimization.php

Lines 346 to 348 in a3b0d7c

	if ( $tracked_in_url_metrics && $needs_detection ) {
	$processor->set_meta_attribute( 'xpath', $processor->get_xpath() );
	}

To

if ( $tracked_in_url_metrics && $needs_detection ) {
	$processor->set_meta_attribute( 'xpath', base64_encode( $processor->get_xpath() ) );
}

And in the Javascript we are retrieving here:

performance/plugins/optimization-detective/detect.js

Lines 587 to 595 in a3b0d7c

	const breadcrumbedElementsMap = new Map(
	[ ...breadcrumbedElements ].map(
	/**
	* @param {HTMLElement} element
	* @return {[HTMLElement, string]} Tuple of element and its XPath.
	*/
	( element ) => [ element, element.dataset.odXpath ]
	)
	);

To

const breadcrumbedElementsMap = new Map(
	[ ...breadcrumbedElements ].map(
		/**
		 * @param {HTMLElement} element
		 * @return {[HTMLElement, string]} Tuple of element and its XPath.
		 */
		( element ) => [ element, atob( element.dataset.odXpath ) ]
	)
);

I tested this on release/3.4.1 and it is working fine.

But as I'm new to performance plugins cloud anyone help me to resolve this error on trunk

Because of this error I tested this on release/3.4.1 😅

also please let me know your thoughts on the above solution.

cc: @westonruter

[westonruter]

@nani-samireddy Hello! Welcome. And your research looks correct.

FYI: The latest release is release/2025-03-17 but testing on trunk is good here.

Nevertheless, I'm not sure yet whether we should implement this change. It's something I opened to discuss. Given it addresses a specific compatibility problem observed in another plugin, I'm wanting to find out if there are more examples of this where we should go ahead and implement this workaround.

In regards to the error you're seeing, this is expected when using wp-env, because it still doesn't support loopback requests. Nevertheless, the plugin specifically excludes this from blocking pages from being optimized by checking if it is a local env:

performance/plugins/optimization-detective/optimization.php

Lines 88 to 91 in a3b0d7c

	array(
	'test' => ! od_is_rest_api_unavailable() || ( wp_get_environment_type() === 'local' && ! function_exists( 'tests_add_filter' ) ),
	'reason' => __( 'Page is not optimized because the REST API for storing URL Metrics is not available.', 'optimization-detective' ),
	),

[nani-samireddy]

@westonruter Thank you for the warm welcome. As you mentioned, let's wait for more insights and other opinions on this and move forward accordingly.