Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.If output_buffering is turned off in php.ini and after a call to file_transfer in file.inc line 794 is called the following notice will be returned.
"notice: ob_end_clean() [ref.outcontrol]: failed to delete buffer. No buffer to delete. in .../includes/file.inc on line 795."
One possible solution, as seen in obendclean_sol2.patch, would be to suppress the error by prefixing ob_end_clean() with an '@'.
<?php
@ob_end_clean();
?>
Another possible solution, as seen in obendclean_sol1.patch, would be to perform a check first.
<?php
if (ob_get_length() > 0) {
ob_end_clean();
}
?>
I checked against Drupal 5 versions of file.inc and it appears to have ob_end_clean() called at the beginning of file_transfer() as well. The once difference that I could think of that would make this issue appear now is the recent changes to common.inc involving E_ALL.
I don't know if this is something that we even want to change but users who meet the above conditions may be subjected to such notices, for example, every time a new imagecache image is generated.
I've attached two patches one for each method discussed above.
Thanks,
-mpare
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | file_205227.patch | 1.26 KB | drewish |
| #3 | d7-file-ob.patch | 696 bytes | mfb |
| obendclean_sol2.patch | 628 bytes | mpare | |
| obendclean_sol1.patch | 664 bytes | mpare | |
Comments
Comment #1
bdragon CreditAttribution: bdragon commentedJust chiming in as I was discussing this issue with mpare.
I believe the intention of the line was to make sure that output would be sent to the client rather than any output buffer that may be open...
Hmm, it should *probabaly* be closing as many buffers as needed, right?
The ob_end_clean() line was in the original version of file.inc. I'm guessing that the reason it's popping up now is that in the past, nobody had A) E_ALL enabled, B) output_buffering off, and C) filed an issue.
Comment #2
drewish CreditAttribution: drewish commentedit looks like it's used a few places in core now:
includes/batch.inc: ob_end_clean();
includes/common.inc: ob_end_clean();
includes/common.inc: ob_end_flush();
includes/file.inc: ob_end_clean();
includes/theme.inc: ob_end_clean(); // End buffering and discard
modules/system/system.module: ob_end_clean();
modules/system/system.module.orig: ob_end_clean();
Comment #3
mfbThis should be the one place in core where ob_end_clean() is called without ob_start(). Here's another proposed fix -- use ob_get_level() instead of ob_get_length().
Comment #4
Anonymous (not verified) CreditAttribution: Anonymous commentedIndeed ob_get_level is the correct function for this filter.
Comment #5
drewish CreditAttribution: drewish commented+1
i don't see any easy way to unit test this since it's just a warning message so i manually verified it. added a file, enabling private file transfers, downloaded it to get the warning, then patch and downloaded again and had no warning.
Comment #6
webchickI checked over the other instances of ob_end* and it appears they indeed all call start/get_contents ahead of time.
Committed to HEAD. Thanks! Moving back for consideration for 6.x.
Comment #7
drewish CreditAttribution: drewish commentedre-rolled for D6.
Comment #8
Gábor HojtsyCommitted to 6.x, thanks! Not sure whether this needs to be backported to 5.x, since it happens in E_ALL only and Drupal 5 does not really strive for E_ALL. Therefore marking fixed.
Comment #9
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.