-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Site Health test to verify that static assets are served with far-future expires #1727
Changes from 4 commits
dc9e364
4cb07aa
ec9a552
dd8e70d
0078cfc
3e5d958
35e7977
3db5886
655d4e5
20099bf
54f3621
0a776c5
3c98337
7afd746
3fcd641
54c73ec
6a2da47
48166ac
17d380e
200b518
1b4cef4
ed28196
ee3a57d
f6ba392
0620840
7ebdc87
49ca83f
12d0b71
aec6f80
2190d60
4f1a00d
2d08732
a048e9f
2c70bd0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,183 @@ | ||||||||||
<?php | ||||||||||
/** | ||||||||||
* Helper function to detect if static assets have far-future expiration headers. | ||||||||||
* | ||||||||||
* @package performance-lab | ||||||||||
* @since n.e.x.t | ||||||||||
*/ | ||||||||||
|
||||||||||
if ( ! defined( 'ABSPATH' ) ) { | ||||||||||
exit; // Exit if accessed directly. | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* Callback for the far-future caching test. | ||||||||||
* | ||||||||||
* @since n.e.x.t | ||||||||||
* | ||||||||||
* @return array{label: string, status: string, badge: array{label: string, color: string}, description: string, actions: string, test: string} Result. | ||||||||||
*/ | ||||||||||
function perflab_ffh_assets_test(): array { | ||||||||||
$result = array( | ||||||||||
'label' => __( 'Your site serves static assets with far-future expiration headers', 'performance-lab' ), | ||||||||||
felixarntz marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
'status' => 'good', | ||||||||||
'badge' => array( | ||||||||||
'label' => __( 'Performance', 'performance-lab' ), | ||||||||||
'color' => 'blue', | ||||||||||
), | ||||||||||
'description' => sprintf( | ||||||||||
'<p>%s</p>', | ||||||||||
esc_html__( | ||||||||||
'Serving static assets with far-future expiration headers improves performance by allowing browsers to cache files for a long time, reducing repeated requests.', | ||||||||||
'performance-lab' | ||||||||||
) | ||||||||||
), | ||||||||||
'actions' => '', | ||||||||||
'test' => 'is_far_future_headers_enabled', | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we maybe use This applies to all the identifiers using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should use So maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @westonruter That works for me! |
||||||||||
); | ||||||||||
|
||||||||||
// List of assets to check. | ||||||||||
$assets = array( | ||||||||||
plugins_url( 'far-future-headers/assets/test.css', __DIR__ ), | ||||||||||
); | ||||||||||
westonruter marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
||||||||||
// Check if far-future headers are enabled for all assets. | ||||||||||
$status = perflab_ffh_check_assets( $assets ); | ||||||||||
|
||||||||||
if ( 'good' !== $status ) { | ||||||||||
$result['status'] = $status; | ||||||||||
$result['label'] = __( 'Your site does not serve static assets with recommended far-future expiration headers', 'performance-lab' ); | ||||||||||
$result['actions'] = sprintf( | ||||||||||
'<p>%s</p>', | ||||||||||
esc_html__( 'Far-future Cache-Control or Expires headers can be added or adjusted with a small configuration change by your hosting provider.', 'performance-lab' ) | ||||||||||
); | ||||||||||
} | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are two different checks which are getting performed one with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the new 3db5886 commit a table is used to display reason for different failure cases. |
||||||||||
|
||||||||||
return $result; | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* Checks if far-future expiration headers are enabled for a list of assets. | ||||||||||
* | ||||||||||
* @since n.e.x.t | ||||||||||
* | ||||||||||
* @param string[] $assets List of asset URLs to check. | ||||||||||
* @return string 'good' if far-future headers are enabled for all assets, 'recommended' if some assets are missing headers. | ||||||||||
*/ | ||||||||||
function perflab_ffh_check_assets( array $assets ): string { | ||||||||||
$final_status = 'good'; | ||||||||||
|
||||||||||
foreach ( $assets as $asset ) { | ||||||||||
$response = wp_remote_get( $asset, array( 'sslverify' => false ) ); | ||||||||||
|
||||||||||
if ( is_wp_error( $response ) ) { | ||||||||||
continue; | ||||||||||
} | ||||||||||
|
||||||||||
$headers = wp_remote_retrieve_headers( $response ); | ||||||||||
|
||||||||||
if ( is_array( $headers ) ) { | ||||||||||
continue; | ||||||||||
} | ||||||||||
|
||||||||||
if ( perflab_ffh_check_headers( $headers ) ) { | ||||||||||
continue; | ||||||||||
} | ||||||||||
|
||||||||||
// If far-future headers are not enabled, attempt a conditional request. | ||||||||||
if ( perflab_ffh_try_conditional_request( $asset, $headers ) ) { | ||||||||||
$final_status = 'recommended'; | ||||||||||
continue; | ||||||||||
} | ||||||||||
|
||||||||||
$final_status = 'recommended'; | ||||||||||
} | ||||||||||
|
||||||||||
return $final_status; | ||||||||||
} | ||||||||||
westonruter marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
||||||||||
/** | ||||||||||
* Checks if far-future expiration headers are enabled. | ||||||||||
* | ||||||||||
* @since n.e.x.t | ||||||||||
* | ||||||||||
* @param WpOrg\Requests\Utility\CaseInsensitiveDictionary $headers Response headers. | ||||||||||
* @return bool True if far-future headers are enabled, false otherwise. | ||||||||||
*/ | ||||||||||
function perflab_ffh_check_headers( WpOrg\Requests\Utility\CaseInsensitiveDictionary $headers ): bool { | ||||||||||
/** | ||||||||||
* Filters the threshold for far-future headers. | ||||||||||
* | ||||||||||
* @since n.e.x.t | ||||||||||
* | ||||||||||
* @param int $threshold Threshold in seconds. | ||||||||||
*/ | ||||||||||
$threshold = apply_filters( 'perflab_far_future_headers_threshold', YEAR_IN_SECONDS ); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe better as:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per above:
Suggested change
|
||||||||||
|
||||||||||
$cache_control = isset( $headers['cache-control'] ) ? $headers['cache-control'] : ''; | ||||||||||
$expires = isset( $headers['expires'] ) ? $headers['expires'] : ''; | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this works even when
Suggested change
|
||||||||||
|
||||||||||
// Check Cache-Control header for max-age. | ||||||||||
$max_age = 0; | ||||||||||
if ( '' !== $cache_control ) { | ||||||||||
// Cache-Control can have multiple directives; we only care about max-age. | ||||||||||
$controls = is_array( $cache_control ) ? $cache_control : array( $cache_control ); | ||||||||||
foreach ( $controls as $control ) { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems this could be simplified a bit:
Suggested change
|
||||||||||
if ( (bool) preg_match( '/max-age\s*=\s*(\d+)/', $control, $matches ) ) { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because PhpStorm complains about the unnecessary cast, but then without it PHPStan complains about "Only booleans are allowed in an if condition, int|false given."
Suggested change
|
||||||||||
$max_age = (int) $matches[1]; | ||||||||||
break; | ||||||||||
} | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
// If max-age meets or exceeds the threshold, we consider it good. | ||||||||||
if ( $max_age >= $threshold ) { | ||||||||||
return true; | ||||||||||
} | ||||||||||
|
||||||||||
// If max-age is not sufficient, check Expires. | ||||||||||
// Expires is a date; we want to ensure it's far in the future. | ||||||||||
if ( is_string( $expires ) && '' !== $expires ) { | ||||||||||
$expires_time = strtotime( $expires ); | ||||||||||
if ( (bool) $expires_time && ( $expires_time - time() ) >= $threshold ) { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
return true; | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
return false; | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* Attempt a conditional request with ETag/Last-Modified. | ||||||||||
* | ||||||||||
* @param string $url The asset URL. | ||||||||||
* @param WpOrg\Requests\Utility\CaseInsensitiveDictionary $headers The initial response headers. | ||||||||||
* @return bool True if a 304 response was received. | ||||||||||
*/ | ||||||||||
function perflab_ffh_try_conditional_request( string $url, WpOrg\Requests\Utility\CaseInsensitiveDictionary $headers ): bool { | ||||||||||
westonruter marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
$etag = isset( $headers['etag'] ) ? $headers['etag'] : ''; | ||||||||||
$last_modified = isset( $headers['last-modified'] ) ? $headers['last-modified'] : ''; | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor improvement.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 12d0b71 |
||||||||||
|
||||||||||
$conditional_headers = array(); | ||||||||||
if ( '' !== $etag ) { | ||||||||||
$conditional_headers['If-None-Match'] = $etag; | ||||||||||
} | ||||||||||
if ( '' !== $last_modified ) { | ||||||||||
$conditional_headers['If-Modified-Since'] = $last_modified; | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like test coverage for this line is warranted. |
||||||||||
} | ||||||||||
|
||||||||||
$response = wp_remote_get( | ||||||||||
$url, | ||||||||||
array( | ||||||||||
'sslverify' => false, | ||||||||||
'headers' => $conditional_headers, | ||||||||||
) | ||||||||||
); | ||||||||||
|
||||||||||
if ( is_wp_error( $response ) ) { | ||||||||||
return false; | ||||||||||
westonruter marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
} | ||||||||||
|
||||||||||
$status_code = wp_remote_retrieve_response_code( $response ); | ||||||||||
return ( 304 === $status_code ); | ||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,28 @@ | ||||||||||||||||||
<?php | ||||||||||||||||||
/** | ||||||||||||||||||
* Hook callbacks used for far-future headers. | ||||||||||||||||||
* | ||||||||||||||||||
* @package performance-lab | ||||||||||||||||||
* @since n.e.x.t | ||||||||||||||||||
*/ | ||||||||||||||||||
|
||||||||||||||||||
if ( ! defined( 'ABSPATH' ) ) { | ||||||||||||||||||
exit; // Exit if accessed directly. | ||||||||||||||||||
} | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per #1815:
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Adds tests to site health. | ||||||||||||||||||
* | ||||||||||||||||||
* @since n.e.x.t | ||||||||||||||||||
* | ||||||||||||||||||
* @param array{direct: array<string, array{label: string, test: string}>} $tests Site Health Tests. | ||||||||||||||||||
* @return array{direct: array<string, array{label: string, test: string}>} Amended tests. | ||||||||||||||||||
*/ | ||||||||||||||||||
function perflab_ffh_add_test( array $tests ): array { | ||||||||||||||||||
$tests['direct']['far_future_headers'] = array( | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per above:
Suggested change
|
||||||||||||||||||
'label' => __( 'Far-Future Caching Headers', 'performance-lab' ), | ||||||||||||||||||
felixarntz marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
'test' => 'perflab_ffh_assets_test', | ||||||||||||||||||
); | ||||||||||||||||||
return $tests; | ||||||||||||||||||
} | ||||||||||||||||||
add_filter( 'site_status_tests', 'perflab_ffh_add_test' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per #1815: