Fix composition of href exclude paths to account for JSON encoding and differing site/home URLs #1164
Conversation
westonruter
added
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Build for testing: speculation-rules.zip |
plugins/speculation-rules/helper.php
Outdated
| array_map( | ||
| array( $prefixer, 'prefix_path_pattern' ), | ||
| array_merge( | ||
| $base_href_exclude_paths, |
There was a problem hiding this comment.
Now that $base_href_exclude_paths are being run through $prefixer::prefix_path_pattern here, should we avoid doing so above?
There was a problem hiding this comment.
Oh! I think you identified another bug, specifically when the site_url and home_url are different. I've added a test case to show this: 74e52a2 & d77b46c. The problem is that the base exclude paths for wp-admin and wp-login are initially prefixed with the site URL, but then later after merging with the filter-supplied exclude paths, it is getting prefixed again and this time with the home URL. The result is the prefix from both the home URL and the site URL is getting applied to the base URLs. I've proposed a fix for this in 01d9153. Please check.
westonruter
changed the title
|
Updated build for testing: speculation-rules.zip |
Summary
When filtering
plsr_speculation_rules_href_exclude_pathsto add additional paths, make sure the resulting de-duplicated array has sequential keys starting from zero. If this is not the case, thenjson_encode()interprets the array as an object which causes a speculation rules failure in the browser.Additionally, we found that the base exclude hrefs were getting prefixed with the site URL path and then prefixed again with the home URL path with the merge with the additional filtered exclude href exclude paths. On sites with different site URL and home URL (e.g.
/wp/and/blog/) this can result in the base URLs getting double-prefixed in an erroneous prefix (e.g./blog/wp/). This is fixed by avoiding the redundant prefixing, so that only the href exclude paths provided by the filter are prefixed with the home URL path. This PR also ensures that the nonce/_wpnonceaction URLs (from #1143) are prefixed with the home URL path and not the site URL path.Fixes #1163
Relevant technical choices
The result of calling
array_unique()is passed intoarray_values()to ensure that the keys are numerically sequential from zero. Tests are updated to ensure this is the case.Additionally, the
$base_href_exclude_pathsare no longer provided as the initial value to pass into theplsr_speculation_rules_href_exclude_pathsfilter. The values in$base_href_exclude_pathsget merged again with the result of the filter, meaning they will always be present regardless. It is misleading to pass them into the filter, as a developer may think they can remove a path by eliminating it from the array passed to the filter when they cannot. Eliminating this redundancy alone fixes the symptom of non-sequential array keys, but the changes in this PR go further to ensure there is no possibility of non-sequential array keys when the the base exclude paths are merged with the return value from the filter.