Boost: let admins test extra Concatenate JS/CSS excludes per-request via GET parameters#49555
Draft
kraftbj wants to merge 3 commits into
Draft
Boost: let admins test extra Concatenate JS/CSS excludes per-request via GET parameters#49555kraftbj wants to merge 3 commits into
kraftbj wants to merge 3 commits into
Conversation
When JS/CSS concatenation breaks a page, support staff and developers previously had to edit the site's saved exclude lists to test whether excluding a particular handle fixes it. This adds per-request debug parameters instead: - ?jb-minify-js-excludes=handle1,handle2 appends handles to the JS concatenate exclude list for that request only. - ?jb-minify-css-excludes=... does the same for CSS. The merge happens in jetpack_boost_page_optimize_js_exclude_list() / ..._css_exclude_list() (read from Concatenate_JS / Concatenate_CSS during wp_head/wp_footer, i.e. within normal WordPress runtime), and: - is only honored for logged-in users with manage_options; - validates each handle against a sanitize_key-style allowlist (lowercase alphanumerics, dash, underscore, dot), ignoring the rest; - never persists anything to the data sync store or options; - has no Page Cache interaction, since logged-in users always bypass Boost's page cache (Request::is_cacheable() returns false). Fixes #31682 https://claude.ai/code/session_01PgpTrtTCH4hpz6Krh3ssho
kraftbj
added
[Status] In Progress
[Plugin] Boost
Claude
Code Coverage SummaryCoverage changed in 1 file.
|
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Boost plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
…d lists The defensive-null unit test passes null on purpose; the docblock now says mixed so Phan's ProbablyReal check matches the implementation. https://claude.ai/code/session_01PgpTrtTCH4hpz6Krh3ssho
Fixes from a code review + adversarial pass over the per-request jb-minify-js-excludes / jb-minify-css-excludes debug parameters: - Preserve handle case instead of lowercasing. Script/style handles are matched case-sensitively downstream, so lowercasing silently failed to exclude any handle registered with uppercase letters (e.g. MyPlugin-script). The allowlist now accepts A-Z and case is kept verbatim, matching how the saved exclude list already behaves. - Ignore oversized parameter values (over 2000 chars) so the per-asset parse cannot be made to do unbounded work. - Document that rendering distinct exclude combinations generates concat-path cache artifacts (reclaimed by the normal Cleanup_Stored_Paths schedule), and that the parameter is intentionally nonce-free so it stays shareable as a URL. - Tighten the function_exists() rationale comment. - Tests: assert case is preserved, cover the all-invalid-handles and empty-string branches, and the oversized-value guard. Drop the test that asserted the old lowercasing behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #31682
Proposed changes
jb-minify-js-excludesandjb-minify-css-excludes, each accepting comma-separated script/style handles.manage_options), the listed handles are appended to the respective saved exclude list for that request only, letting support/developers test "would excluding handle X fix this page?" without editing settings or needing a staging copy.jetpack_boost_page_optimize_merge_debug_excludes()infunctions-helpers.php, called from the two exclude-list readers consumed byConcatenate_JS::do_items()/Concatenate_CSS::do_items()— which run during normal WordPress runtime (wp_head/wp_footer), socurrent_user_can()is reliably available (with afunction_existsguard as belt-and-braces; the pre-WordPress_jb_staticservice path never reads exclude lists).Request::is_cacheable()returnsfalseon both the serve and the write paths — and GET parameters are part of the cache key anyway, so these parameters can neither hit nor poison cached pages.Cleanup_Stored_Pathsschedule.Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
/_jb_static/??…URLs.?jb-minify-js-excludes=jquery-coreappended. Confirmjquery-coreis now served as a standalone<script>tag (withWP_DEBUGon you'll see<!-- No Concat JS jquery-core => Excluded option -->), while other scripts remain concatenated. Repeat with?jb-minify-css-excludes=<style-handle>for CSS.?jb-minify-js-excludes=<script>alert(1)</script>— the invalid handle is ignored and the page renders normally.jp test php plugins/boost); theDebug_Excludes_Testcases exercise this feature.