![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
#64370 closed defect (bug) (fixed)
Response header detection in page cache test for Site Health should be more robust
| Reported by: | westonruter | Owned by: | westonruter |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.0 |
| Component: | Site Health | Version: | 6.1 |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: | performance |
Description (last modified by )
This is a follow-up to #63748. See PR comment from @dmsnell:
I wish that the
$cache_hit_callbackwere more robust than it is, since it also matches on values like “this cache is *hit, don’t use it” and “not a hit” but that’s not part of this ticket or work.
would be awesome to have some example strings from each of these new headers as a comment to the right of them.
for example, the varnish docs suggest that the _full_ match is
hit, meaning we could add=> static function ( $v ) { return 'hit' === $v; } /** @see https://www.varnish-software.com/developers/tutorials/logging-cache-hits-misses-varnish/ */
the extra examples are icing on the cake and not necessary here.
according to this random survey
x-cache-statusis expected to only containhit, though for x-cache there is an insignificant but measureable count of requests containingHIT, MISS
Also, in Slack:
the only thing that really caught my eye is that we’re needly allocating to do case-insensitive compare instead of calling
stripos()
Attachments (1)
Change History (16)
This ticket was mentioned in Slack in #core-performance by westonruter. View the logs.
7 months ago
#3
@
7 months ago
- Keywords has-patch added; needs-patch removed
Replaces the old string-based cache header detection in Site Health with a more robust method.
Adds case-insensitive matching, supports additional cache headers, and prevents false positives.
Improves accuracy across Varnish, NGINX, Cloudflare, LiteSpeed, and host-level caching systems.
#4
@
7 months ago
- Keywords needs-patch added; has-patch removed
@solankisoftware Your patch seems malformed. It is introducing an entire new file. Please consider opening a pull request instead to facilitate review and testing.
This ticket was mentioned in PR #10606 on WordPress/wordpress-develop by @solankisoftware.
7 months ago
#5
- Keywords has-patch added; needs-patch removed
Trac Ticket: https://core.trac.wordpress.org/ticket/64370
This PR enhances the WordPress Site Health page cache test by introducing
a robust cache header detection mechanism. The previous method could
produce false positives or negatives depending on the caching system used.
Changes include:
- Added detect_cache_headers() method for strict, case-insensitive header matching
- Improved support for common cache headers (x-cache, x-cache-status, cf-cache-status, x-litespeed-cache, x-varnish, etc.)
- Updated get_test_page_cache() to use detect_cache_headers()
- Prevents false positives when unrelated text contains words like "HIT" or "MISS"
Testing:
- Enable any caching layer (Varnish, NGINX fastcgi_cache, Cloudflare, LiteSpeed, host-level caching)
- Go to Tools → Site Health → Status
- Verify that page cache detection reflects the real server cache status
This PR is backward compatible and does not change existing Site Health UI behavior.
This ticket was mentioned in Slack in #core-performance by westonruter. View the logs.
6 months ago
@westonruter commented on PR #10606:
6 months ago
#7
Closing due to lack of response and the PR not being touching many unrelated lines.
This ticket was mentioned in PR #10855 on WordPress/wordpress-develop by @westonruter.
5 months ago
#9
- Keywords has-patch has-unit-tests added; needs-patch removed
### Summary of Changes
This PR improves the robustness of page cache detection in Site Health by refining the response header validation and expanding support for common caching layers.
#### Logic & Header Improvements
- Stricter Cache Hit Matching: The
$cache_hit_callbacknow uses a word-boundary regular expression (/\bhit\b/i) instead of a simple substring check. This prevents false positives from header values likeshit. -
X-SRCache-Store-StatusCorrection: Fixed the validation for this header to check for the correct valueSTORE(the module's indicator for a successful cache storage) instead ofHIT. - Varnish Support: Added detection for the
X-Varnishheader, specifically identifying a hit when the header contains two request IDs (e.g.,123 456), as documented in the Varnish FAQ. - Standard Headers: Restored
Last-Modified,ETag, andViaas valid indicators of a caching layer, while ensuring they are handled as existence-based detections or validated where appropriate. - Documentation: Added detailed docblocks and source links for caching headers (e.g.,
srcache,Varnish) and explicitly listed supported generic proxies (Squid, Go, Fastly, LiteSpeed) in comments.
#### PHPUnit Test Enhancements
- Comprehensive Test Cases: Added new test scenarios to
Tests_Admin_wpSiteHealthto verify:-
X-Varnishhits vs. misses. -
X-SRCache-Store-Status(STORE) andX-SRCache-Fetch-Status(HIT). - Prevention of false positives from substrings (e.g.,
x-cache: shit). - Robustness of standard headers like
ETag,Last-Modified, andVia.
-
- Improved Typing: Updated the test suite with strict parameter and return types for data providers and test methods, aligning with modern PHP standards and resolving PHPStan reported issues.
#### Code Quality
- Applied consistent formatting to
class-wp-site-health.phpand the test suite usingcomposer formatto ensure adherence to WordPress Coding Standards.
Trac ticket: https://core.trac.wordpress.org/ticket/64370
@westonruter commented on PR #10855:
5 months ago
#10
_Gemini review:_
I have completed the review of the changes committed to the trac-64370 branch.
### Commit Context
The changes aim to improve the robustness of page cache detection in WordPress Site Health. Key improvements include:
- Using stricter regex (
\bhit\b) to prevent false positives from header values like "shit". - Adding support for the
X-Varnishheader by detecting the specific hit format (two request IDs). - Correcting
x-srcache-store-statusdetection to look forSTOREinstead ofHIT. - Improving documentation and PHPStan typing for the detection methods and tests.
### Code Review
#### src/wp-admin/includes/class-wp-site-health.php
- Logic Improvement: The move from
str_containstopreg_match( '/\bhit\b/i', ... )is an excellent fix for substring-related false positives. - Varnish Header: The
x-varnishcallback correctly implements the detection of a hit by checking for two space-separated digits, matching Varnish's debugging header behavior. - SRCache Correction: Updating
x-srcache-store-statusto check forSTOREis correct as per the module's documentation. - Coding Standards:
- The use of array shape return types (
array<string, ?callable>) in docblocks aligns with current project goals for PHPStan level 10 compatibility. - The addition of the
: arrayreturn type hint is compatible with PHP 7.4.
- The use of array shape return types (
- Nitpick: The
viaheader is restored but set tonull(existence check). While standard, many proxies useViawithout caching. However, given the context of other headers, this is a reasonable heuristic for WordPress core.
#### tests/phpunit/tests/admin/wpSiteHealth.php
- Type Safety: The addition of strict types to properties and method signatures (
private WP_Site_Health $instance;,test_get_page_cache( array $responses, ... )) is a great improvement for the test suite's reliability. - PHPStan Resolution: The return type for
data_get_page_cachehas been meticulously defined to match the complex array structure returned, which should resolve the reported PHPStan errors. - Test Coverage:
- New cases for
false-positive-hit-in-word,varnish-header, andsrcachevariants provide thorough coverage for the logic changes. - The
last-modifiedtest case uses a realistic date string, which is better than the previous "hit" value.
- New cases for
- Consistency: The change from
nulltofalseforgood_basic_authin the data provider, coupled with the signature change tobool, ensures type consistency.
### Summary
The changes are high quality, follow WordPress coding standards, and significantly improve the reliability of Site Health's page cache detection. No regressions or standards violations were identified.
Recommendation: Ready for merge.
FIX :: Response header detection in page cache test for Site Health should be more robust