Make WordPress Core

Opened 6 weeks ago

Last modified 6 weeks ago

#63501 new defect (bug)

wp_iframe sends output before send_headers hook causing PHP headers sent notice

Reported by: kkmuffme's profile kkmuffme Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Media Keywords:
Focuses: Cc:

Description

/wp-admin/media-upload.php?post_id=1234&type=image&TB_iframe=1&tab=library

<?php
add_action( 'send_headers', function() { header( 'X-Foo: bar' ); }, 10, 0 );

will report a PHP notice for headers already sent.

media_upload_library() => wp_iframe() => media_upload_library_form() => wp_edit_attachments_query() executes the main query, which calls the send_headers hook

However wp_iframe has already started output at that point.

The easy fix is to just ob_start() buffer the whole thing, however, since this is media, this has massive performance drawbacks both on the server (memory consumption, nginx potentially buffering to file if output exceeds buffer size) and the user (loading is delayed until whole page is loaded instead of incrementally)
The correct fix would be to ensure we run the main query early before wp_iframe starts output

Secondly, I think adding a wp doing it wrong in send_headers if headers_sent() would make sense to prevent similar issues in the future and to warn other people of making a similar mistake.

Change History (3)

#1 @kkmuffme
6 weeks ago

#62988 was marked as a duplicate.

#2 @adamsilverstein
6 weeks ago

Hi @kkmuffme and thanks for the bug report.

Can you provide some steps to reproduce? How are you reaching the provided URL? (/wp-admin/media-upload.php?post_id=1234&type=image&TB_iframe=1&tab=library)

#3 @kkmuffme
6 weeks ago

I don't know how exactly our users got to that page - I just got tons of PHP notices in Sentry, which is why I reported it. What I do know is that that link exists in HTML directly and is not an URL that is added/generated dynamically with JS.

Note: See TracTickets for help on using tickets.