Skip to content

Enqueued Assets audit: treat zero-size and no-store responses as errors#2481

Merged
westonruter merged 9 commits into
WordPress:trunkfrom
Gilmoursa:fix/2417-enqueued-assets-zero-size-no-store
Jun 19, 2026
Merged

Enqueued Assets audit: treat zero-size and no-store responses as errors#2481
westonruter merged 9 commits into
WordPress:trunkfrom
Gilmoursa:fix/2417-enqueued-assets-zero-size-no-store

Conversation

@Gilmoursa

@Gilmoursa Gilmoursa commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #2417.

perflab_aea_get_asset_size() had two TODOs in the code for cases that silently passed the audit when they should be flagged as errors:

  1. Zero-size response body — a 200 response with an empty body means the asset is effectively broken. The audit was reporting it as OK with a size of 0 bytes.
  2. Non-cacheable / poorly-cacheable responses — assets that browsers cannot cache (or cannot cache effectively) are a performance problem and should surface in the audit. The audit was reporting these as OK as long as the status was 200.

Behaviour change

Both new failure modes return a WP_Error, which feeds into the existing error display path in the audit table (red row, error message shown, recommended status), so no additional UI changes are needed. An asset with an error also fails the test even when the count/byte thresholds are not exceeded.

Zero-size

Returns WP_Error( 'zero_size' ) instead of 0 — message: "The asset returned an empty response body."

Cache policy

Returns WP_Error( 'not_cacheable' ) when the response is not cacheable or only weakly cacheable. The error message is tailored to the severity of the directive:

  • Cache-Control: no-store"The asset response cannot be cached by browsers."
    no-store forbids caching outright, so the wording is unqualified.
  • Cache-Control: no-cache and Cache-Control: max-age=0"The asset response may not be fully cacheable by browsers."
    These permit caching but force revalidation on every request, so the wording is softened.

The Cache-Control directives are parsed into a name⇒value map, so max-age is compared numerically. This catches variant forms such as max-age="0" and max-age = 0 (whitespace around the =), while max-age=3600 still passes.

When there is no Cache-Control header, the check falls back to Expires:

  • Expires with a past date (and no Cache-Control) → flagged (soft message). The asset is already expired.
  • An invalid/unparseable Expires value → treated as already expired per RFC 7234, and flagged.
  • Expires with a future date → passes.

When there is neither Cache-Control nor Expires, the browser can only fall back to heuristic caching, which is only viable when a validator is present:

  • No Cache-Control, no Expires, and no Last-Modified/ETag → flagged (soft message). The asset effectively cannot be cached.
  • No Cache-Control, no Expires, but a Last-Modified or ETag is present → passes. Heuristic caching is viable, and most servers send Last-Modified for static files by default, so this avoids false positives.

Changes

  • plugins/performance-lab/includes/site-health/audit-enqueued-assets/helper.php: implement the zero-size and cache-policy checks, remove the TODO comments.
  • plugins/performance-lab/tests/includes/site-health/audit-enqueued-assets/test-audit-enqueued-assets-helper.php: update the zero-size assertion to expect WP_Error; add test_perflab_aea_get_asset_size_not_cacheable() covering no-store, no-cache, max-age=0, max-age="0", max-age = 0, array-valued Cache-Control, past/future/invalid Expires, no-headers (with and without a Last-Modified/ETag validator), and a normal cacheable response.
  • plugins/performance-lab/tests/data/class-audit-assets-mock-assets.php: pass headers from mocked responses through the pre_http_request filter so header-dependent tests work end-to-end, and give the default good-asset mock a Cache-Control: max-age=3600 header so existing audit tests continue to represent cacheable assets.

Test plan

  • Run composer test:integration -- --filter test_perflab_aea_get_asset_size
  • Run composer test:integration -- --filter test_perflab_aea_get_asset_size_not_cacheable
  • Confirm an asset returning a zero-byte body shows as an error row in the Site Health blocking assets table
  • Confirm an asset served with Cache-Control: no-store shows as an error row in the table
  • Confirm an asset served with Cache-Control: no-cache shows as an error row in the table
  • Confirm an asset served with Cache-Control: max-age=0 shows as an error row in the table
  • Confirm an asset with a past Expires and no Cache-Control shows as an error row in the table
  • Confirm an asset with no Cache-Control, no Expires, and no Last-Modified/ETag shows as an error row in the table
  • Confirm an asset with no Cache-Control/Expires but a Last-Modified or ETag still passes the audit
  • Confirm an asset served with Cache-Control: max-age=3600 still passes the audit

@github-actions

github-actions Bot commented May 15, 2026

Copy link
Copy Markdown

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Gilmoursa <gilmoursa@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: tejas0306 <suhan2411@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

In perflab_aea_get_asset_size(), two cases that should have been
treated as errors were silently passing the audit:

- A 200 response with an empty body now returns WP_Error('zero_size').
  An asset that returns nothing is effectively broken, regardless of the
  status code.

- A 200 response with Cache-Control: no-store now returns
  WP_Error('not_cacheable'). An asset that browsers are forbidden from
  caching is a clear performance problem and should surface in the audit.

  Cache-Control: no-cache (revalidate) is intentionally left as passing
  since the asset can still be served from cache after revalidation.

Both cases were already noted as TODOs in the original code. Removes
those TODO comments and adds tests covering zero-size, no-store,
no-cache (should pass), and normal cacheable responses. Also updates
the mock class to pass response headers through so header-dependent
tests can work end-to-end.

Fixes WordPress#2417.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Gilmoursa Gilmoursa force-pushed the fix/2417-enqueued-assets-zero-size-no-store branch from 7168784 to 4bcfe31 Compare May 15, 2026 14:58
@codecov

codecov Bot commented May 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.55556% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.87%. Comparing base (04960a9) to head (3dfa7b5).
⚠️ Report is 6 commits behind head on trunk.

Files with missing lines Patch % Lines
...ludes/site-health/audit-enqueued-assets/helper.php 95.55% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk    #2481      +/-   ##
==========================================
+ Coverage   69.72%   69.87%   +0.14%     
==========================================
  Files          90       90              
  Lines        7753     7797      +44     
==========================================
+ Hits         5406     5448      +42     
- Misses       2347     2349       +2     
Flag Coverage Δ
multisite 69.87% <95.55%> (+0.14%) ⬆️
single 35.33% <0.00%> (-0.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

wp_remote_retrieve_header() returns string|array (array when the same
header appears multiple times in the response). Joining array values
with implode() before passing to strtolower() resolves the PHPStan
error and also correctly handles multi-value Cache-Control headers.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@westonruter westonruter added the [Type] Enhancement A suggestion for improvement of an existing feature label May 15, 2026
@westonruter westonruter added this to the performance-lab n.e.x.t milestone May 15, 2026
@westonruter westonruter requested a review from Copilot May 15, 2026 15:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Enqueued Assets Site Health audit so empty asset responses and Cache-Control: no-store responses are surfaced as asset errors instead of passing as successful zero-byte/cacheable assets.

Changes:

  • Returns WP_Error for zero-size asset bodies and Cache-Control: no-store.
  • Adds tests for zero-size and cache-control handling.
  • Updates the audit asset test mock to pass response headers through.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
plugins/performance-lab/includes/site-health/audit-enqueued-assets/helper.php Adds empty-body and no-store error handling in asset size retrieval.
plugins/performance-lab/tests/includes/site-health/audit-enqueued-assets/test-audit-enqueued-assets-helper.php Updates and expands coverage for the new asset-size error behavior.
plugins/performance-lab/tests/data/class-audit-assets-mock-assets.php Includes mocked headers in HTTP responses for header-dependent tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@westonruter westonruter added the [Plugin] Performance Lab Issue relates to work in the Performance Lab Plugin only label May 15, 2026
…ze check

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@westonruter

Copy link
Copy Markdown
Member

Sorry for the delay in following up on this.

  • Confirm an asset served with Cache-Control: no-cache still passes the audit

Should it? A directive of no-cache or max-age=0 would require a browser to do a round-trip to do a conditional request to see if there is a newer version available.

Also, what if there is an Expires response header with a past date and there is no Cache-Control header (since Cache-Control supersedes Expires)?

So it seems there are additional scenarios that can be included in the test as failures.

@westonruter westonruter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per reviewer feedback:
- no-cache and max-age=0 force revalidation on every request, which is
  a performance problem for static assets. Flag these as not_cacheable
  alongside no-store.
- When no Cache-Control header is present, check the Expires header. A
  past date means the browser treats the asset as uncacheable.
- Update error message to be directive-agnostic.
- Add test cases for no-cache, max-age=0, past Expires, and future
  Expires.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Gilmoursa

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback. I've updated the PR to address both points:

  • no-cache and max-age=0 are now flagged alongside no-store, since both force revalidation on every request
  • Added a fallback check for Expires with a past date when no Cache-Control header is present

Test cases for all new scenarios are included.

westonruter and others added 5 commits June 18, 2026 19:15
… fix/2417-enqueued-assets-zero-size-no-store
Make the malformed Expires handling explicit rather than relying on
strtotime() returning false coercing to 0 in the comparison. An
unparseable Expires value is now deliberately treated as already
expired (and thus not cacheable) per RFC 7234.

Add test coverage for an invalid Expires value.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Parse Cache-Control into a name => value map so max-age is compared
numerically rather than matched against the exact "max-age=0" token.
This now also catches variant forms like max-age="0" and
max-age = 0 (whitespace around the equals sign).

Add test coverage for the quoted and whitespace-padded max-age forms.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
no-store truly forbids caching, so its message stays as "cannot be
cached by browsers." But no-cache and max-age=0 permit caching while
forcing revalidation, and a past Expires means the asset is merely
stale, so those cases now report "may not be fully cacheable by
browsers" to avoid overstating the problem.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dator

When a response has neither Cache-Control nor Expires, browsers can
only apply heuristic caching, and only when a validator (Last-Modified
or ETag) is present. Without any validator the asset effectively cannot
be cached, so flag it as not_cacheable with the soft message. Responses
that carry a validator still pass, since heuristic caching is viable.

Update the default good-asset mock to send Cache-Control: max-age=3600
so existing audit tests continue to represent cacheable assets, and add
coverage for the no-headers, Last-Modified-only, and ETag-only cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@westonruter

Copy link
Copy Markdown
Member

I had Claude create a test plugin for the various scenarios:

Plugin Name: Cache-Control Test Scripts
<?php
/**
 * Plugin Name: Cache-Control Test Scripts
 * Description: Registers and enqueues a set of render-blocking test scripts served via admin-ajax, each replicating one of the caching scenarios handled by the Enqueued Assets audit. See https://github.com/WordPress/performance/pull/2481.
 * Author: Weston Ruter
 * Version: 1.0.0
 * License: GPL-2.0-or-later
 *
 * @package CacheControlTestScripts
 */

namespace CacheControlTestScripts;

const ACTION_PREFIX = 'cctest_';

/**
 * Gets the test scenarios.
 *
 * Each scenario is rendered as a blocking `<script src>` pointing at admin-ajax, and the
 * corresponding ajax handler emits the body and headers below with a `text/javascript` content type.
 *
 * The `headers` are the *only* caching-related headers sent: the handler first strips the
 * `Cache-Control`, `Expires`, `Last-Modified`, and `ETag` headers that admin-ajax would otherwise
 * send (via nocache_headers()), then applies exactly what each scenario specifies.
 *
 * @return array<string, array{label: string, body: string, headers: array<string, string>, expected: string}>
 */
function get_scenarios(): array {
	$past   = gmdate( 'D, d M Y H:i:s', time() - DAY_IN_SECONDS ) . ' GMT';
	$future = gmdate( 'D, d M Y H:i:s', time() + YEAR_IN_SECONDS ) . ' GMT';
	$now    = gmdate( 'D, d M Y H:i:s' ) . ' GMT';

	return array(
		// Cacheable: passes the audit.
		'max_age_3600'    => array(
			'label'    => 'Cache-Control: max-age=3600 (cacheable)',
			'body'     => "console.log( 'Hello World: cacheable' );",
			'headers'  => array( 'Cache-Control' => 'max-age=3600' ),
			'expected' => 'pass',
		),

		// Zero-byte body but cacheable: flagged as zero_size.
		'zero_size'       => array(
			'label'    => 'Zero-byte body (max-age=3600)',
			'body'     => '',
			'headers'  => array( 'Cache-Control' => 'max-age=3600' ),
			'expected' => 'error: zero_size',
		),

		// no-store: cannot be cached at all.
		'no_store'        => array(
			'label'    => 'Cache-Control: no-store',
			'body'     => "console.log( 'Hello World: not stored' );",
			'headers'  => array( 'Cache-Control' => 'no-store' ),
			'expected' => 'error: not_cacheable (unqualified)',
		),

		// no-cache: revalidation forced on every request.
		'no_cache'        => array(
			'label'    => 'Cache-Control: no-cache',
			'body'     => "console.log( 'Hello World: not cached' );",
			'headers'  => array( 'Cache-Control' => 'no-cache' ),
			'expected' => 'error: not_cacheable (soft)',
		),

		// max-age=0: permits caching but forces revalidation.
		'max_age_0'       => array(
			'label'    => 'Cache-Control: max-age=0',
			'body'     => "console.log( 'Hello World: max-age=0' );",
			'headers'  => array( 'Cache-Control' => 'max-age=0' ),
			'expected' => 'error: not_cacheable (soft)',
		),

		// max-age="0" with whitespace: parsed numerically, still flagged.
		'max_age_quoted'  => array(
			'label'    => 'Cache-Control: max-age = "0"',
			'body'     => "console.log( 'Hello World: max-age quoted zero' );",
			'headers'  => array( 'Cache-Control' => 'max-age = "0"' ),
			'expected' => 'error: not_cacheable (soft)',
		),

		// Past Expires, no Cache-Control: already expired.
		'expires_past'    => array(
			'label'    => 'Expires in the past (no Cache-Control)',
			'body'     => "console.log( 'Hello World: expired' );",
			'headers'  => array( 'Expires' => $past ),
			'expected' => 'error: not_cacheable (soft)',
		),

		// Invalid Expires, no Cache-Control: treated as already expired.
		'expires_invalid' => array(
			'label'    => 'Expires invalid/unparseable (no Cache-Control)',
			'body'     => "console.log( 'Hello World: invalid expires' );",
			'headers'  => array( 'Expires' => 'this-is-not-a-date' ),
			'expected' => 'error: not_cacheable (soft)',
		),

		// Future Expires, no Cache-Control: passes.
		'expires_future'  => array(
			'label'    => 'Expires in the future (no Cache-Control)',
			'body'     => "console.log( 'Hello World: future expires' );",
			'headers'  => array( 'Expires' => $future ),
			'expected' => 'pass',
		),

		// No Cache-Control, no Expires, no validator: cannot be cached.
		'no_headers'      => array(
			'label'    => 'No Cache-Control, no Expires, no validator',
			'body'     => "console.log( 'Hello World: no cache headers' );",
			'headers'  => array(),
			'expected' => 'error: not_cacheable (soft)',
		),

		// No Cache-Control, no Expires, but Last-Modified validator present: passes via heuristic caching.
		'validator'       => array(
			'label'    => 'No Cache-Control/Expires but Last-Modified present',
			'body'     => "console.log( 'Hello World: heuristic via Last-Modified' );",
			'headers'  => array( 'Last-Modified' => $now ),
			'expected' => 'pass',
		),
	);
}

/**
 * Gets the admin-ajax URL for a scenario.
 *
 * @param string $slug Scenario slug.
 * @return string URL.
 */
function get_scenario_url( string $slug ): string {
	return add_query_arg( 'action', ACTION_PREFIX . $slug, admin_url( 'admin-ajax.php' ) );
}

/**
 * Registers the admin-ajax handlers for each scenario.
 */
function register_ajax_handlers(): void {
	foreach ( array_keys( get_scenarios() ) as $slug ) {
		$handler = static function () use ( $slug ): void {
			serve_scenario( $slug );
		};
		add_action( 'wp_ajax_' . ACTION_PREFIX . $slug, $handler );
		add_action( 'wp_ajax_nopriv_' . ACTION_PREFIX . $slug, $handler );
	}
}
add_action( 'init', __NAMESPACE__ . '\\register_ajax_handlers' );

/**
 * Serves a single scenario's script body with its configured headers.
 *
 * @param string $slug Scenario slug.
 */
function serve_scenario( string $slug ): void {
	$scenarios = get_scenarios();
	if ( ! isset( $scenarios[ $slug ] ) ) {
		status_header( 404 );
		exit;
	}
	$scenario = $scenarios[ $slug ];

	// admin-ajax sends nocache headers and a text/html content type; strip the caching ones so each
	// scenario starts from a clean slate and controls exactly which caching headers are present.
	header_remove( 'Cache-Control' );
	header_remove( 'Expires' );
	header_remove( 'Last-Modified' );
	header_remove( 'ETag' );

	header( 'Content-Type: text/javascript' );

	foreach ( $scenario['headers'] as $name => $value ) {
		header( $name . ': ' . $value );
	}

	echo $scenario['body']; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- This is JavaScript, not HTML.
	exit;
}

/**
 * Enqueues all scenario scripts as render-blocking scripts in the head on the front end.
 *
 * They are enqueued without a loading strategy (no defer/async) and in the head so that the
 * Enqueued Assets audit treats them as render-blocking external scripts to be audited.
 */
function enqueue_scripts(): void {
	foreach ( array_keys( get_scenarios() ) as $slug ) {
		$handle = 'cctest-' . str_replace( '_', '-', $slug );
		wp_enqueue_script(
			$handle,
			get_scenario_url( $slug ),
			array(),
			null, // Do not append a ver query arg.
			false // In the head (render-blocking).
		);
	}
}
add_action( 'wp_enqueue_scripts', __NAMESPACE__ . '\\enqueue_scripts' );

The Site Health test as expected:

image

@westonruter westonruter merged commit ea77bb1 into WordPress:trunk Jun 19, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Plugin] Performance Lab Issue relates to work in the Performance Lab Plugin only [Type] Enhancement A suggestion for improvement of an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enqueued Assets audit: treat non-cacheable and zero-size responses as errors

3 participants