Speculative Loading invalid key for a URL pattern error - exclude_paths filter

[brianleejackson]

Bug Description

Trying to simply exclude a URL pattern and running into what appears to be a bug with the plsr_speculation_rules_href_exclude_paths filter.

Getting this error:

Invalid key "0" for a URL pattern object found.

Steps to reproduce

1. Fresh WordPress Install with Speculative Loading plugin running.
2. Add a filter with the following exclusion and URL pattern. Using example from WordPress.org docs (source).

add_filter(
    'plsr_speculation_rules_href_exclude_paths',
    function ( array $exclude_paths ): array {
        $exclude_paths[] = '/go/*';
        return $exclude_paths;
    }
);

3. Log out of the site and test in Chrome DevTools. Error should flag.

Additional Context

• PHP Version: 8.2
• OS: macOS Sonoma 14.4.1
• Browser: Chrome Version 123.0.6312.124
• Plugin Version: 1.2.1

If I'm doing something wrong, feel free to clarify and close the issue. Thanks!

[westonruter]

Your code looks absolutely correct. I can reproduce the issue. It's resulting in the following JSON:

{
  "prerender": [
    {
      "source": "document",
      "where": {
        "and": [
          { "href_matches": "/*" },
          {
            "not": {
              "href_matches": {
                "0": "/wp-login.php",
                "1": "/wp-admin/*",
                "2": "/*\\?*(^|&)_wpnonce=*",
                "6": "/go/*"
              }
            }
          },
          { "not": { "selector_matches": "a[rel=nofollow]" } },
          { "not": { "selector_matches": ".no-prerender" } }
        ]
      },
      "eagerness": "moderate"
    }
  ]
}

For reference, the default JSON looks like this:

{
  "prerender": [
    {
      "source": "document",
      "where": {
        "and": [
          { "href_matches": "/*" },
          {
            "not": {
              "href_matches": [
                "/wp-login.php",
                "/wp-admin/*",
                "/*\\?*(^|&)_wpnonce=*"
              ]
            }
          },
          { "not": { "selector_matches": "a[rel=nofollow]" } },
          { "not": { "selector_matches": ".no-prerender" } }
        ]
      },
      "eagerness": "moderate"
    }
  ]
}

Notice the strange keys:

              "href_matches": {
                "0": "/wp-login.php",
                "1": "/wp-admin/*",
                "2": "/*\\?*(^|&)_wpnonce=*",
                "6": "/go/*"
              }

I'm not sure why the item pushed to the end of the array is getting the key of 6.

It look like this can be fixed simply with this patch, by putting the exclusion URLs through array_values() to ensure the array keys are sequential:

--- a/plugins/speculation-rules/helper.php
+++ b/plugins/speculation-rules/helper.php
@@ -63,12 +63,14 @@ function plsr_get_speculation_rules() {
 	$href_exclude_paths = (array) apply_filters( 'plsr_speculation_rules_href_exclude_paths', $href_exclude_paths, $mode );
 
 	// Ensure that there are no duplicates and that the base paths cannot be removed.
-	$href_exclude_paths = array_unique(
-		array_map(
-			array( $prefixer, 'prefix_path_pattern' ),
-			array_merge(
-				$base_href_exclude_paths,
-				$href_exclude_paths
+	$href_exclude_paths = array_values(
+		array_unique(
+			array_map(
+				array( $prefixer, 'prefix_path_pattern' ),
+				array_merge(
+					$base_href_exclude_paths,
+					$href_exclude_paths
+				)
 			)
 		)
 	);

It seems array_unique() is responsible for the introduction of these non-sequential keys, and I see why. The issue that $href_exclude_paths starts out the same as $base_href_exclude_paths and then it is filtered where your new entry is then added.

If no changes were made to the array, then the $href_exclude_paths and $base_href_exclude_paths are both:

array(
  0 => "/wp-login.php",
  1 => "/wp-admin/*",
  2 => "/*\\?*(^|&)_wpnonce=*",
)

When they get merged here:

performance/plugins/speculation-rules/helper.php

Lines 65 to 74 in 8f07477

	// Ensure that there are no duplicates and that the base paths cannot be removed.
	$href_exclude_paths = array_unique(
	array_map(
	array( $prefixer, 'prefix_path_pattern' ),
	array_merge(
	$base_href_exclude_paths,
	$href_exclude_paths
	)
	)
	);

They are:

array(
  0 => "/wp-login.php",
  1 => "/wp-admin/*",
  2 => "/*\\?*(^|&)_wpnonce=*",
  3 => "/wp-login.php",
  4 => "/wp-admin/*",
  5 => "/*\\?*(^|&)_wpnonce=*",
)

When array_unique() runs, it removes keys 3-5 since they are duplicates. However, when you push your excluded URL onto the list, the full merged array becomes:

array(
  0 => "/wp-login.php",
  1 => "/wp-admin/*",
  2 => "/*\\?*(^|&)_wpnonce=*",
  3 => "/wp-login.php",
  4 => "/wp-admin/*",
  5 => "/*\\?*(^|&)_wpnonce=*",
  6 => "/go/*",
)

And then array_unique() reduces this to:

array(
  0 => "/wp-login.php",
  1 => "/wp-admin/*",
  2 => "/*\\?*(^|&)_wpnonce=*",
  6 => "/go/*",
)

This results in the non-sequential keys.

Easy to fix, but I'm surprised it wasn't discovered until now. Fix incoming!

[westonruter]

Oh, I have a workaround for you. Since the base rules are merged with the filtered rules anyway, you can just skip pushing your URL and replace the rule entirely:

add_filter(
	'plsr_speculation_rules_href_exclude_paths',
	function ( array $exclude_paths ): array {
		return array( '/go/*' ); // TODO: This is a temporary hack!!
	}
);

This will prevent the duplicates from appearing in the first place.

The PR to fix the issue should (1) do array_values() and (2) avoid even passing the $base_href_exclude_paths into the filter to begin with since they get merged in the end anyway.

[brianleejackson]

Awesome, thanks for the super fast response and explanation.

I can confirm that the temporary hack works. 😅

Glad to hear this will get fixed in a future update!

Keep up the great work.

[westonruter]

@brianleejackson I've got a fix opened in #1164.

Would you mind testing this build to see if it fixes the issue? #1164 (comment)

[brianleejackson]

@westonruter Great! I tested the .zip version and can confirm it works.

I tested with the original filter again from the doc and no errors. Can also confirm a test link containing /go/ was properly excluded.

Really appreciate the fast responses! 👍

[westonruter]

@brianleejackson We found another bug which is also fixed in the same PR. I attached another build ZIP. If you have a chance, could you test that as well? I'm hoping to release the fix today.

[brianleejackson]

@westonruter Tested new .zip on my end, all good! 👍

[westonruter]

Thank you! v1.2.2 is now live on the plugin directory.