Skip to content

Commit fa7e141

Browse files
thisismyurlivonneprietoclaude
authored
Fix: parse Accept header q-values in wpsc_get_accept_header() (#1057)
* Fix: parse Accept header q-values in wpsc_get_accept_header() The previous implementation used a naive str_contains() check: if any known JSON media type appeared anywhere in the Accept header string, the request was classified as application/json. This ignored RFC 7231 §5.3.2 quality values (q=), so a valid HTML-preferring header such as the one sent by New Relic Synthetics — Accept: text/html,application/xhtml+xml,application/json;q=0.9,... — was misclassified as a JSON request. Two downstream effects: 1. wp_cache_serve_cache_file() refused to serve the cached file. 2. A separate application/json cache bucket was populated on every synthetic check, triggering a fresh page build each time. This commit extracts a new wpsc_parse_accept_header() helper that parses q-values and classifies the request as application/json only when a known JSON type has a strictly higher q-value than text/html. Ties resolve to text/html (safe default: serve the cached page). The wpsc_accept_headers filter continues to work — filtered types participate in the q-value comparison as JSON types. Fixes #1045 * fix: drop */* wildcard fallback from wpsc_parse_accept_header() JSON/Fediverse clients commonly send Accept: application/json, */*. The */* wildcard was lifting effective_html_q to 1.0 when text/html was absent, causing ties that resolved to text/html — serving cached HTML to clients that explicitly requested JSON. Fix: ignore */* entirely when computing effective_html_q. Use only the explicit text/html q-value, defaulting to 0.0 when absent. This preserves the New Relic Synthetics case (explicit text/html;q=1.0 beats json;q=0.9) while correctly routing application/json, */* to the application layer. Test changes: - Rename and flip test_wildcard_covers_html_when_not_explicit to test_wildcard_does_not_cover_html_when_not_explicit (asserts JSON) - Add test_fediverse_json_with_wildcard_classifies_as_json as a regression guard for the specific Fediverse case from the review Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address CI failures — PHPUnit attribute, PHPCS, stale comments PHPUnit (PHP 8.2): replace @Covers doc-comment with #[CoversFunction] attribute. phpunit.11.xml.dist sets failOnPhpunitDeprecation=true and PHPUnit 11 (used on 8.2) still reads doc-comment metadata and emits a deprecation; PHPUnit 12 (8.3+) ignores it. The attribute works on both. PHPCS: the apply_filters() stub triggered "file should contain either function declarations or OO structure declarations, but not both". Moved the stub to tests/php/bootstrap.php (where it belongs — the stub is a test infrastructure concern, not part of the test class) and fixed the bootstrap @Package tag while there. Remaining PHPCS fixes in WpscParseAcceptHeaderTest.php: - $json_types property: add @var string[] to the docblock - Four wildcard-behaviour method comments: // → /** */ - Lowercase short description: "application/activity+json classified" → "Classifies application/activity+json as JSON" - Stale inline comment in test_malformed_q_value_treated_as_default: removed the "effective html_q from */*=0.7" reasoning (wildcard no longer applies); updated to "text/html absent → effective_html_q = 0.0" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Christopher Ross <thisismyurl@users.noreply.github.com> Co-authored-by: Christopher Ross <184154038+thisismyurl@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 6ff7c35 commit fa7e141

3 files changed

Lines changed: 187 additions & 10 deletions

File tree

‎tests/php/WpscParseAcceptHeaderTest.php‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
<?php
2+
/**
3+
* Tests for wpsc_parse_accept_header().
4+
*
5+
* @package automattic/wp-super-cache
6+
*/
7+
8+
require_once dirname( __DIR__, 2 ) . '/wp-cache-phase2.php';
9+
10+
use PHPUnit\Framework\Attributes\CoversFunction;
11+
use PHPUnit\Framework\TestCase;
12+
13+
#[CoversFunction( 'wpsc_parse_accept_header' )]
14+
class WpscParseAcceptHeaderTest extends TestCase {
15+
16+
/**
17+
* Default JSON types mirroring the wpsc_accept_headers default.
18+
*
19+
* @var string[]
20+
*/
21+
private array $json_types = array(
22+
'application/json',
23+
'application/activity+json',
24+
'application/ld+json',
25+
);
26+
27+
// ── RFC 7231 acceptance criteria from issue #1045 ─────────────────────────
28+
29+
/**
30+
* New Relic Synthetics header: text/html has implicit q=1.0, JSON is
31+
* deprioritised at q=0.9 — must classify as text/html.
32+
*/
33+
public function test_nr_synthetics_header_classifies_as_html(): void {
34+
$accept = 'text/html,application/xhtml+xml,application/json;q=0.9,application/javascript;q=0.9,text/javascript;q=0.9,application/xml;q=0.9,text/plain;q=0.8,*/*;q=0.7';
35+
$this->assertSame( 'text/html', wpsc_parse_accept_header( $accept, $this->json_types ) );
36+
}
37+
38+
/** Bare JSON-only Accept: application/json should classify as JSON. */
39+
public function test_bare_json_classifies_as_json(): void {
40+
$this->assertSame( 'application/json', wpsc_parse_accept_header( 'application/json', $this->json_types ) );
41+
}
42+
43+
/** Tie (both implicit q=1.0) resolves to text/html (safe default). */
44+
public function test_tie_resolves_to_html(): void {
45+
$this->assertSame( 'text/html', wpsc_parse_accept_header( 'text/html,application/json', $this->json_types ) );
46+
}
47+
48+
/** JSON strictly higher q than text/html must classify as JSON. */
49+
public function test_json_higher_q_classifies_as_json(): void {
50+
$this->assertSame( 'application/json', wpsc_parse_accept_header( 'application/json,text/html;q=0.9', $this->json_types ) );
51+
}
52+
53+
/** Extended JSON type (application/ld+json) via filter participates in comparison. */
54+
public function test_extended_json_type_participates(): void {
55+
$this->assertSame( 'application/json', wpsc_parse_accept_header( 'application/ld+json;q=1.0,text/html;q=0.8', $this->json_types ) );
56+
}
57+
58+
/** Malformed q-value (non-numeric) treated as q=1.0, no warning/fatal. */
59+
public function test_malformed_q_value_treated_as_default(): void {
60+
// application/json;q=bad → treated as q=1.0; text/html absent → effective_html_q = 0.0; 1.0 > 0.0 → application/json.
61+
$this->assertSame( 'application/json', wpsc_parse_accept_header( 'application/json;q=bad,*/*;q=0.7', $this->json_types ) );
62+
}
63+
64+
/** Out-of-range q-value clamped — q=2 treated as 1.0. */
65+
public function test_out_of_range_q_clamped(): void {
66+
$this->assertSame( 'text/html', wpsc_parse_accept_header( 'text/html;q=2,application/json;q=0.9', $this->json_types ) );
67+
}
68+
69+
// ── Wildcard behaviour ────────────────────────────────────────────────────
70+
71+
/**
72+
* The *\/* wildcard does NOT cover text/html when text/html is not explicit.
73+
* A JSON client sending "*\/*,application/json;q=0.9" must classify as JSON,
74+
* not as text/html, to avoid serving cached HTML to non-browser clients.
75+
*/
76+
public function test_wildcard_does_not_cover_html_when_not_explicit(): void {
77+
// html absent → effective_html_q = 0.0; json q=0.9 > 0.0 → application/json.
78+
$this->assertSame( 'application/json', wpsc_parse_accept_header( '*/*,application/json;q=0.9', $this->json_types ) );
79+
}
80+
81+
/** Fediverse regression: "application/json, *\/*" must NOT serve cached HTML. */
82+
public function test_fediverse_json_with_wildcard_classifies_as_json(): void {
83+
// html absent → effective_html_q = 0.0; json q=1.0 > 0.0 → application/json.
84+
$this->assertSame( 'application/json', wpsc_parse_accept_header( 'application/json, */*', $this->json_types ) );
85+
}
86+
87+
/** Explicit text/html;q=0.5 wins over *\/*;q=1.0 — wildcard does not boost an explicit html q. */
88+
public function test_explicit_html_takes_precedence_over_wildcard(): void {
89+
// html explicit q=0.5; wildcard q=1.0; json q=0.8 → 0.8 > 0.5 → application/json.
90+
$this->assertSame( 'application/json', wpsc_parse_accept_header( 'text/html;q=0.5,*/*;q=1.0,application/json;q=0.8', $this->json_types ) );
91+
}
92+
93+
/** No explicit text/html and no wildcard, but JSON present — effective html_q = 0.0 → application/json. */
94+
public function test_no_html_no_wildcard_json_present(): void {
95+
$this->assertSame( 'application/json', wpsc_parse_accept_header( 'application/json;q=0.5', $this->json_types ) );
96+
}
97+
98+
// ── Edge cases ────────────────────────────────────────────────────────────
99+
100+
/** Whitespace around media types is handled. */
101+
public function test_whitespace_around_media_types(): void {
102+
$this->assertSame( 'text/html', wpsc_parse_accept_header( ' text/html , application/json;q=0.9 ', $this->json_types ) );
103+
}
104+
105+
/** Classifies application/activity+json as JSON. */
106+
public function test_activity_json_classified_as_json(): void {
107+
$this->assertSame( 'application/json', wpsc_parse_accept_header( 'application/activity+json', $this->json_types ) );
108+
}
109+
110+
/** Custom type added via wpsc_accept_headers filter participates. */
111+
public function test_custom_json_type_via_filter_participates(): void {
112+
$extended = array_merge( $this->json_types, array( 'application/vnd.api+json' ) );
113+
$this->assertSame( 'application/json', wpsc_parse_accept_header( 'application/vnd.api+json;q=1.0,text/html;q=0.8', $extended ) );
114+
}
115+
116+
/** Typical browser Accept header classifies as text/html. */
117+
public function test_typical_browser_accept_header(): void {
118+
$accept = 'text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8';
119+
$this->assertSame( 'text/html', wpsc_parse_accept_header( $accept, $this->json_types ) );
120+
}
121+
}

‎tests/php/bootstrap.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,17 @@
22
/**
33
* Bootstrap.
44
*
5-
* @package automattic/
5+
* @package automattic/wp-super-cache
66
*/
77

8+
// apply_filters() is a WP core function not defined in wp-cache-phase2.php;
9+
// stub it here so test files can require that file without a full WP bootstrap.
10+
if ( ! function_exists( 'apply_filters' ) ) {
11+
function apply_filters( $tag, $value ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement
12+
return $value;
13+
}
14+
}
15+
816
/**
917
* Include the composer autoloader.
1018
*/

‎wp-cache-phase2.php‎

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -537,23 +537,71 @@ function wpsc_get_accept_header() {
537537
$accept_headers = apply_filters( 'wpsc_accept_headers', array( 'application/json', 'application/activity+json', 'application/ld+json' ) );
538538
$accept_headers = array_map( 'strtolower', $accept_headers );
539539
// phpcs:ignore WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase
540-
// phpcs:ignore WordPress.Security.ValidatedSanitizedInput.MissingUnslash -- $accept is checked and set below.
541-
$accept = isset( $_SERVER['HTTP_ACCEPT'] ) ? strtolower( filter_var( $_SERVER['HTTP_ACCEPT'] ) ) : '';
540+
// phpcs:ignore WordPress.Security.ValidatedSanitizedInput.MissingUnslash -- $raw is checked and set below.
541+
$raw = isset( $_SERVER['HTTP_ACCEPT'] ) ? strtolower( filter_var( $_SERVER['HTTP_ACCEPT'] ) ) : '';
542542

543-
foreach ( $accept_headers as $header ) {
544-
if ( str_contains( $accept, $header ) ) {
545-
$accept = 'application/json';
543+
$accept = empty( $raw ) ? 'text/html' : wpsc_parse_accept_header( $raw, $accept_headers );
544+
545+
wp_cache_debug( 'ACCEPT: ' . $accept );
546+
}
547+
548+
return $accept;
549+
}
550+
551+
/**
552+
* Classify an Accept header as 'text/html' or 'application/json'.
553+
*
554+
* Parses RFC 7231 §5.3.2 quality values (q=) so that a request where
555+
* text/html has a higher or equal q-value than any known JSON type is
556+
* correctly served from cache. The previous str_contains() approach treated
557+
* any mention of a JSON media type as a JSON request, regardless of priority.
558+
*
559+
* Classification rules:
560+
* - Classify as 'application/json' only when a known JSON type has a
561+
* strictly higher q-value than text/html.
562+
* - Ties, or text/html strictly higher, resolve to 'text/html'.
563+
* - If text/html is absent, its effective q-value is 0.0. The *\/* wildcard
564+
* is intentionally not applied to text/html: a JSON client sending
565+
* "application/json, *\/*" would otherwise receive cached HTML.
566+
* - Malformed q-values (non-numeric, out-of-range) are treated as q=1.0.
567+
*
568+
* @param string $raw_accept Lowercased, non-empty Accept header value.
569+
* @param string[] $json_types Media types to treat as JSON (from wpsc_accept_headers filter).
570+
* @return string 'text/html' or 'application/json'.
571+
*/
572+
function wpsc_parse_accept_header( $raw_accept, $json_types ) {
573+
$html_q = null; // null = not explicitly present in Accept header.
574+
$json_q = 0.0;
575+
576+
foreach ( explode( ',', $raw_accept ) as $part ) {
577+
$segments = explode( ';', trim( $part ) );
578+
$media_type = trim( $segments[0] );
579+
$q = 1.0; // Default q-value per RFC 7231 §5.3.1.
580+
581+
for ( $i = 1, $len = count( $segments ); $i < $len; $i++ ) {
582+
$param = ltrim( $segments[ $i ] );
583+
if ( strncmp( $param, 'q=', 2 ) === 0 ) {
584+
$q_str = substr( $param, 2 );
585+
$q = is_numeric( $q_str ) ? max( 0.0, min( 1.0, (float) $q_str ) ) : 1.0;
586+
break;
546587
}
547588
}
548589

549-
if ( $accept !== 'application/json' ) {
550-
$accept = 'text/html';
590+
if ( 'text/html' === $media_type ) {
591+
$html_q = null === $html_q ? $q : max( $html_q, $q );
551592
}
552593

553-
wp_cache_debug( 'ACCEPT: ' . $accept );
594+
if ( in_array( $media_type, $json_types, true ) ) {
595+
$json_q = max( $json_q, $q );
596+
}
554597
}
555598

556-
return $accept;
599+
// Effective html q: explicit text/html only. A */* wildcard must not lift
600+
// text/html above an explicitly requested JSON type, or JSON/Fediverse
601+
// clients sending e.g. "application/json, */*" get served cached HTML.
602+
$effective_html_q = ( null !== $html_q ) ? $html_q : 0.0;
603+
604+
return $json_q > $effective_html_q ? 'application/json' : 'text/html';
557605
}
558606

559607
function wp_cache_get_cookies_values() {

0 commit comments

Comments
 (0)