Remove `flush()` call in wp_cache_get_response_headers() #127

Merged
merged 1 commit into from Feb 3, 2017

Conversation

Projects
None yet
4 participants
@zytzagoo
Contributor

zytzagoo commented Aug 11, 2016

For more details see: #126

Fixes #126

@kraftbj

This comment has been minimized.

Show comment
Hide comment
@kraftbj

kraftbj Aug 11, 2016

Contributor

At first read, your explanation and process makes sense to me. Donncha (primary dev) is on vacation and want to get his nod for modifying something this ancient ;-).

Contributor

kraftbj commented Aug 11, 2016

At first read, your explanation and process makes sense to me. Donncha (primary dev) is on vacation and want to get his nod for modifying something this ancient ;-).

@kraftbj kraftbj added this to the 1.4.9 milestone Aug 11, 2016

@kraftbj kraftbj added the bug label Aug 11, 2016

@donnchawp

This comment has been minimized.

Show comment
Hide comment
@donnchawp

donnchawp Feb 3, 2017

Contributor

We should check it in BUT we'll have to modify it because for some reason apache_response_headers() doesn't always behave and needs flush() to do the job. See this comment for more details.

I'll check the headers and if it's empty I'll call flush() and get the headers again.

Contributor

donnchawp commented Feb 3, 2017

We should check it in BUT we'll have to modify it because for some reason apache_response_headers() doesn't always behave and needs flush() to do the job. See this comment for more details.

I'll check the headers and if it's empty I'll call flush() and get the headers again.

@donnchawp donnchawp merged commit 67c53e9 into Automattic:master Feb 3, 2017

donnchawp added a commit that referenced this pull request Feb 3, 2017

Check if headers are empty and then flush and get the headers again
In #127 flush() was removed but a comment on apache_response_headers()
said that function may return an empty array, so I check for that here
and do a flush() before getting the headers again.
@DrLightman

This comment has been minimized.

Show comment
Hide comment
@DrLightman

DrLightman Feb 9, 2017

Buf flush() can't be used inside an output buffer display handler anyway, otherwise it triggers this PHP Fatal error:

PHP Fatal error: Unknown: Cannot use output buffering in output buffering display handlers in Unknown on line 0

Because flush() is used inside wp_cache_get_response_headers() that is used inside wp_cache_shutdown_callback() that is used inside wp_cache_ob_callback() that has been registered as an output callback handler with:

ob_start( 'wp_cache_ob_callback' );

DrLightman commented Feb 9, 2017

Buf flush() can't be used inside an output buffer display handler anyway, otherwise it triggers this PHP Fatal error:

PHP Fatal error: Unknown: Cannot use output buffering in output buffering display handlers in Unknown on line 0

Because flush() is used inside wp_cache_get_response_headers() that is used inside wp_cache_shutdown_callback() that is used inside wp_cache_ob_callback() that has been registered as an output callback handler with:

ob_start( 'wp_cache_ob_callback' );

@donnchawp

This comment has been minimized.

Show comment
Hide comment
@donnchawp

donnchawp Feb 9, 2017

Contributor

Ah, you're right. Take a look at #196 - I removed the flush() and changed the flow of the code so that headers_list() is used if apache_response_headers() fails.

Contributor

donnchawp commented Feb 9, 2017

Ah, you're right. Take a look at #196 - I removed the flush() and changed the flow of the code so that headers_list() is used if apache_response_headers() fails.

kraftbj pushed a commit that referenced this pull request Feb 9, 2017

donncha
* Github merge:
* Make sure $cache_path has a trailing slash (#77)
* Remove flush() (#127) but also check if headers are empty and flush and get headers again. (#179)
* Add fix for customizer (#161) and don't cache PUT AND DELETE requests (#178)
* Check for superglobals before using them. (#131)



git-svn-id: http://plugins.svn.wordpress.org/wp-super-cache/trunk@1588365 b8457f37-d9ea-0310-8a92-e5e31aec5664
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment