Skip to content

Fix: parse Accept header q-values in wpsc_get_accept_header()#1057

Open
thisismyurl wants to merge 1 commit into
Automattic:trunkfrom
thisismyurl:fix/accept-header-q-value-parsing
Open

Fix: parse Accept header q-values in wpsc_get_accept_header()#1057
thisismyurl wants to merge 1 commit into
Automattic:trunkfrom
thisismyurl:fix/accept-header-q-value-parsing

Conversation

@thisismyurl
Copy link
Copy Markdown

Summary

Fixes #1045wpsc_get_accept_header() was using a naive str_contains() check that matched any JSON media type anywhere in the Accept string, ignoring RFC 7231 §5.3.2 quality values. The result: monitoring tools like New Relic Synthetics (and Datadog/Pingdom/UptimeRobot) that send valid HTML-preferring headers such as:

Accept: text/html,application/xhtml+xml,application/json;q=0.9,...

were misclassified as JSON requests. Two downstream effects:

  1. wp_cache_serve_cache_file() refused to serve the cached file on every synthetic check.
  2. A separate application/json cache bucket was populated on each check, triggering a fresh page build every time.

Changes

wp-cache-phase2.php

  • Extracted a new wpsc_parse_accept_header( $raw_accept, $json_types ) helper. It parses q-values per RFC 7231 §5.3.2 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).
  • wpsc_get_accept_header() now delegates to this helper after the empty-header early-return. The static memoization and wpsc_accept_headers filter contract are unchanged. Callers are unaffected.

Edge cases handled:

  • Malformed q-values (non-numeric, out-of-range) — treated as q=1.0, no warning or fatal.
  • */* wildcard — provides effective html q-value only when text/html is not explicitly present.
  • Whitespace around media-range tokens.

tests/php/WpscParseAcceptHeaderTest.php (new file)

14 unit tests covering the seven acceptance criteria from the issue brief plus additional edge cases (wildcard behaviour, custom types via the filter, typical browser headers, out-of-range q-values). The test exercises wpsc_parse_accept_header() directly — it has no WordPress dependencies, so no WP test scaffolding is required.

PHPUnit 12.5.28 — 14 / 14 (100%) — 0.003s

Note on test harness: the issue brief mentions a harness being stood up in #1051. I've added the test class here so the coverage lands with the fix rather than waiting. If #1051 introduces a shared bootstrap or test base class, this file is easy to migrate.

Acceptance criteria from the issue brief

  • New Relic Synthetics header (text/html implicit q=1.0, application/json;q=0.9) → text/html
  • Accept: application/json alone → application/json
  • Accept: text/html,application/json (tie, both q=1.0) → text/html
  • Accept: application/json,text/html;q=0.9 (json strictly higher) → application/json
  • Accept: application/ld+json;q=1.0,text/html;q=0.8 (extended type via filter) → application/json
  • Absent or empty Accept → text/html
  • Malformed q-values → no warning or fatal, deterministic result
  • wpsc_accept_headers filter types participate in q-value comparison
  • Function remains free of late-loaded dependencies; static memoization preserved

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 Automattic#1045
Copilot AI review requested due to automatic review settings May 29, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wpsc_get_accept_header() ignores Accept header quality values (q-values)

1 participant