Skip oversized Link preload header in Optimization Detective#2572
Skip oversized Link preload header in Optimization Detective#2572faisalahammad wants to merge 2 commits into
Conversation
- Add od_get_maximum_link_response_header_length() with filter od_link_response_header_max_length (default 4KB) - Skip sending the Link response header when it would exceed the limit, falling back to the existing HTML link tag output - Add tests covering the new getter and the HTML fallback Some reverse proxies fail requests with an oversized response header, which can happen when responsive images have long or non-ASCII filenames across multiple viewport breakpoints. The HTML link tags already carry the same preload info, so skipping the header when it is too large keeps the request from failing without losing preload behavior. Fixes WordPress#2304
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
github-actions
Bot
added
the
[Plugin] Optimization Detective
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #2572 +/- ##
==========================================
- Coverage 70.35% 70.31% -0.04%
==========================================
Files 91 91
Lines 7867 7857 -10
==========================================
- Hits 5535 5525 -10
Misses 2332 2332
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
westonruter
modified the milestones:
image-prioritizer n.e.x.t,
optimization-detective 1.0.0-beta6
westonruter
left a comment
westonruter
left a comment
There was a problem hiding this comment.
Sorry for the delay. Please see my comment on the issue.
…ader Reworks the oversized Link preload header fix per reviewer feedback: instead of capping/skipping the header when too large, permanently strip imagesrcset/imagesizes from the Link response header since those attributes cause the bloat and web.dev now recommends relying on the HTML link tag for responsive image preloading anyway. - Remove od_get_maximum_link_response_header_length() and the od_link_response_header_max_length filter added previously; the header guard is back to its original simple form. - OD_Link_Collection::get_response_header() now omits imagesrcset and imagesizes; get_html() is unchanged and still emits them. - Update tests and docs accordingly. Addresses PR feedback. Refs WordPress#2572
|
@westonruter Thanks, I read your comment on #2304 and agree, this is a better fix. Changed approach: instead of capping the Removed the Ready for another look. |
|
Closing in favor of #2578, which better matches the direction from @westonruter's feedback on #2304 and the web.dev guidance on responsive-image preloads. #2578 excludes links that have Thanks @westonruter for the review that drove the approach change — continuing review on #2578. |
2 participants
Summary
Fixes #2304
Image Prioritizer (via Optimization Detective) can generate a
Link: rel=preloadresponse header that gets too large when responsive images have long or non-ASCII filenames across multiple viewport breakpoints with different LCP images. Some reverse proxies (Nginx) reject the response with "upstream sent too big header", causing intermittent 502s for logged-in users.Relevant technical choices
od_optimize_template_output_buffer()inoptimization.phpalready sends both aLinkresponse header and equivalent HTML<link>tags for every collected preload link, unconditionally. The HTML tags carry the same preload info, so the fix skips sending the header when it would exceed a length cap and just relies on the HTML tags in that case.od_get_maximum_link_response_header_length(), mirroring the existingod_get_maximum_url_metric_size()pattern (filterod_link_response_header_max_length, default 4 KB,_doing_it_wrong()fallback on invalid values).header()call inoptimization.php; theappend_head_html()call stays unconditional so preloading keeps working through HTML tags when the header is skipped.docs/hooks.md.headers_sent()is alwaystruein this repo's PHPUnit harness, so the actualheader()call itself can't be asserted on in automated tests (this was already true before this change, nothing here regresses that).Testing
Manual: page with responsive images across several breakpoints resolving to different LCP images with long filenames, logged in, uncached request — confirmed no single
Linkheader exceeds the cap and<link rel="preload">tags are still present in<head>. Verified the filter raises/lowers the cap as expected.Automated:
npm run test-php:optimization-detective(360 tests pass),composer lint:optimization-detective(clean),npm run phpstan(no errors).Use of AI Tools
Used an AI coding assistant (Claude Code) to investigate the codebase, draft the implementation, tests, and docs, and to run the test/lint/static-analysis suite. I reviewed the change, ran the tests myself, and take responsibility for the code.