Skip to content

Commit 5fb247b

Browse files
authored
Merge pull request #1851 from WordPress/add/url-metric-length-budget
Account for 64 KiB limit for sending beacon data
2 parents ab6b54e + 6f96d20 commit 5fb247b

File tree

3 files changed

+304
-97
lines changed

3 files changed

+304
-97
lines changed

plugins/optimization-detective/detect.js

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,7 @@ export default async function detect( {
688688
return;
689689
}
690690

691+
// Finalize extensions.
691692
if ( extensions.size > 0 ) {
692693
/** @type {Promise[]} */
693694
const extensionFinalizePromises = [];
@@ -740,6 +741,36 @@ export default async function detect( {
740741
}
741742
}
742743

744+
/*
745+
* Now prepare the URL Metric to be sent as JSON request body.
746+
*/
747+
748+
const maxBodyLengthKiB = 64;
749+
const maxBodyLengthBytes = maxBodyLengthKiB * 1024;
750+
751+
// TODO: Consider adding replacer to reduce precision on numbers in DOMRect to reduce payload size.
752+
const jsonBody = JSON.stringify( urlMetric );
753+
const percentOfBudget =
754+
( jsonBody.length / ( maxBodyLengthKiB * 1000 ) ) * 100;
755+
756+
/*
757+
* According to the fetch() spec:
758+
* "If the sum of contentLength and inflightKeepaliveBytes is greater than 64 kibibytes, then return a network error."
759+
* This is what browsers also implement for navigator.sendBeacon(). Therefore, if the size of the JSON is greater
760+
* than the maximum, we should avoid even trying to send it.
761+
*/
762+
if ( jsonBody.length > maxBodyLengthBytes ) {
763+
if ( isDebug ) {
764+
error(
765+
`Unable to send URL Metric because it is ${ jsonBody.length.toLocaleString() } bytes, ${ Math.round(
766+
percentOfBudget
767+
) }% of ${ maxBodyLengthKiB } KiB limit:`,
768+
urlMetric
769+
);
770+
}
771+
return;
772+
}
773+
743774
// Even though the server may reject the REST API request, we still have to set the storage lock
744775
// because we can't look at the response when sending a beacon.
745776
setStorageLock( getCurrentTime() );
@@ -751,7 +782,16 @@ export default async function detect( {
751782
);
752783

753784
if ( isDebug ) {
754-
log( 'Sending URL Metric:', urlMetric );
785+
const message = `Sending URL Metric (${ jsonBody.length.toLocaleString() } bytes, ${ Math.round(
786+
percentOfBudget
787+
) }% of ${ maxBodyLengthKiB } KiB limit):`;
788+
789+
// The threshold of 50% is used because the limit for all beacons combined is 64 KiB, not just the data for one beacon.
790+
if ( percentOfBudget < 50 ) {
791+
log( message, urlMetric );
792+
} else {
793+
warn( message, urlMetric );
794+
}
755795
}
756796

757797
const url = new URL( restApiEndpoint );
@@ -769,7 +809,7 @@ export default async function detect( {
769809
url.searchParams.set( 'hmac', urlMetricHMAC );
770810
navigator.sendBeacon(
771811
url,
772-
new Blob( [ JSON.stringify( urlMetric ) ], {
812+
new Blob( [ jsonBody ], {
773813
type: 'application/json',
774814
} )
775815
);

plugins/optimization-detective/storage/rest-api.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,27 @@ function od_handle_rest_request( WP_REST_Request $request ) {
214214
);
215215
}
216216

217+
/*
218+
* The limit for data sent via navigator.sendBeacon() is 64 KiB. This limit is checked in detect.js so that the
219+
* request will not even be attempted if the payload is too large. This server-side restriction is added as a
220+
* safeguard against clients sending possibly malicious payloads much larger than 64 KiB which should never be
221+
* getting sent.
222+
*/
223+
$max_size = 64 * 1024;
224+
$content_length = strlen( (string) wp_json_encode( $url_metric ) );
225+
if ( $content_length > $max_size ) {
226+
return new WP_Error(
227+
'rest_content_too_large',
228+
sprintf(
229+
/* translators: 1: the size of the payload, 2: the maximum allowed payload size */
230+
__( 'JSON payload size is %1$s bytes which is larger than the maximum allowed size of %2$s bytes.', 'optimization-detective' ),
231+
number_format_i18n( $content_length ),
232+
number_format_i18n( $max_size )
233+
),
234+
array( 'status' => 413 )
235+
);
236+
}
237+
217238
// TODO: This should be changed from store_url_metric($slug, $url_metric) instead be update_post( $slug, $group_collection ). As it stands, store_url_metric() is duplicating logic here.
218239
$result = OD_URL_Metrics_Post_Type::store_url_metric(
219240
$request->get_param( 'slug' ),

0 commit comments

Comments
 (0)