#14730 closed defect (bug) (fixed)
ms-files.php required ob_clean() and flush()
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | Multisite | Keywords: | has-patch |
Focuses: | multisite | Cc: |
Description
I recently migrated a development wp 3 multisite install to 'live' mode for a site launch. One problem I had is that images served out of /files stopped working. Everything looked fine as far as the response / headers, but the image would open up as corrupted.
After quite a bit of troubleshooting, the problem went away when I added ob_clean() and flush() calls to ms-files.php just before the readfile($file) call at the end of the file. Perhaps this is due to a problem elsewhere in my site, but I'm creating this bug so someone can decide if adding the buffer flush would be a reasonable addition to the codebase. It seems like this is the recommended way of doing things according to the sample code here:
Attachments (2)
Change History (38)
#2
@
14 years ago
- Cc wordpress@… added
I encountered a similar problem on two different installations of WordPress when upgrading from wpmu 2.9.2 to wp 3.0.1.
This is a long entry, but hopefully has plenty of concrete information.
On the first server:
- Ubuntu 8.04.4 LTS
- PHP Version 5.2.4-2ubuntu5.10
- Apache/2.2.8 (Ubuntu) DAV/2 SVN/1.4.6 PHP/5.2.4-2ubuntu5.10 with Suhosin-Patch proxy_html/3.0.0
I first noticed ms-files.php cutting off an image at 16k. Here is the output of a wget:
wget http://ias.version-control.com/files/2010/01/DarthVader.png --2010-09-08 10:00:42-- http://ias.version-control.com/files/2010/01/DarthVader.png Resolving ias.version-control.com... 87.117.205.87 Connecting to ias.version-control.com|87.117.205.87|:80... connected. HTTP request sent, awaiting response... 200 OK Length: 16631 (16K) [image/png] Saving to: `DarthVader.png' 98% [=...==> ] 16,384 --.-K/s in 0.04s 2010-09-08 10:00:42 (365 KB/s) - Connection closed at byte 16384. Retrying.
Adding a call to flush() after readfile($file) solved the problem.
wget http://ias.version-control.com/files/2010/01/DarthVader.png --2010-09-08 10:01:50-- http://ias.version-control.com/files/2010/01/DarthVader.png Resolving ias.version-control.com... 87.117.205.87 Connecting to ias.version-control.com|87.117.205.87|:80... connected. HTTP request sent, awaiting response... 200 OK Length: 16631 (16K) [image/png] Saving to: `DarthVader.png.1' 100%[=...==>] 16,631 --.-K/s in 0.05s 2010-09-08 10:01:50 (343 KB/s) - `DarthVader.png.1' saved [16631/16631]
I then noticed the problem (or rather the client did) on the live site (which had the call to flush in place) on a larger file.
On the live server:
- Red Hat Enterprise Linux Server release 5.4 (Tikanga)
- PHP Version 5.1.6
- Apache/2.2.3 (Red Hat)
ms-files.php was cutting off at 1097728 bytes. Here is the output of a wget:
wget http://imascientist.org.uk/files/2010/01/Im-a-Scientist-evaluation-report.pdf --2010-09-07 16:38:34-- http://imascientist.org.uk/files/2010/01/Im-a-Scientist-evaluation-report.pdf Resolving imascientist.org.uk... 94.236.120.219 Connecting to imascientist.org.uk|94.236.120.219|:80... connected. HTTP request sent, awaiting response... 200 OK Length: 1099885 (1.0M) [application/pdf] Saving to: `Im-a-Scientist-evaluation-report.pdf.4' 99% [=...===> ] 1,097,728 1.06M/s in 1.0s 2010-09-07 16:38:36 (1.06 MB/s) - Connection closed at byte 1097728. Retrying.
I went back to the first server and tried the larger file. Same result without the flush() in place. Here is the output of a wget:
wget http://ias.version-control.com/files/2010/01/Im-a-Scientist-evaluation-report.pdf --2010-09-08 09:53:23-- http://ias.version-control.com/files/2010/01/Im-a-Scientist-evaluation-report.pdf Resolving ias.version-control.com... 87.117.205.87 Connecting to ias.version-control.com|87.117.205.87|:80... connected. HTTP request sent, awaiting response... 200 OK Length: 1099885 (1.0M) [application/pdf] Saving to: `Im-a-Scientist-evaluation-report.pdf' 99% [==...==> ] 1,097,728 633K/s in 1.7s 2010-09-08 09:53:25 (633 KB/s) - Connection closed at byte 1097728. Retrying.
With the flush() in place the large file also downloads successfully.
Back on the live server, I spotted that even small files do not download with the flush in place: here is wget again for a much smaller file:
wget http://imascientist.org.uk/files/2010/04/ias-ticket-wallet-18th-june-09-thumbnail1-150x117.jpg --2010-09-08 10:49:24-- http://imascientist.org.uk/files/2010/04/ias-ticket-wallet-18th-june-09-thumbnail1-150x117.jpg Resolving imascientist.org.uk... 94.236.120.219 Connecting to imascientist.org.uk|94.236.120.219|:80... connected. HTTP request sent, awaiting response... 200 OK Length: 9297 (9.1K) [image/jpeg] Saving to: `ias-ticket-wallet-18th-june-09-thumbnail1-150x117.jpg.1' 88% [=...==> ] 8,192 --.-K/s in 0.009s 2010-09-08 10:49:24 (925 KB/s) - Connection closed at byte 8192. Retrying.
Here it is cutting off at 8k rather than 16k
This time after much googling, I found a replacement readfile here http://us2.php.net/manual/en/function.readfile.php#88549
Adding that function to ms-files.php and calling that in place of readfile solves the problem on the live site: both small and large files are downloaded ok.
In both cases the problem did not occur with WordPress MU 2.9.2.
In neither case has the OS, PHP, or Apache changed. They are both dedicated servers and I have full control.
I noticed that for both the large and small files, the cut off point was an 'exact' size (16K and 67 x 16K for server 1, 8K and 134 x 8K for the live server), so this is probably an interaction with a PHP buffer somewhere.
Summary:
ms-files.php call to readfile() seems to be not flushing it's last buffer for both large and small files.
On the test server, a call to flush() solves the problem for both large and small files.
On the live server, the call to flush was not enough for small or large. The replacement readfile function was required.
The servers have different OS, different version of PHP and different versions of Apache, so this is not a simple "bug in version x of y" issue.
The difference between blogs.php from mu 2.9.2 and ms-files.php in wp 3.0.1 is minimal, but something in the 3.0.1 environment is triggering this issue.
I hope this helps someone get to the heart of the issue.
#4
@
14 years ago
Just encountered another issue on the live server, where mod_deflate was interfering too. Uploaded file (/files/) files would not download to a browser with mod_deflate enabled. I didn't notice it with wget as it doesn't support deflate.
It should have only been turned on for certain files types
AddOutputFilterByType DEFLATE text/html text/css application/x-javascript text/plain text/xml
but seemed to be also applied to files served via ms-files.php
I've turned it off for now as I couldn't figure out how to turn it off just for those files. That is Apache related, but maybe the issue is with an Apache buffer not PHPs.
By the way, having turned off mod_deflate, I quickly rolled back to normal readfile in ms-files.php and the problem re-appeared.
Westi, if you need access to my test server I could give you that.
#5
@
14 years ago
I just ran into the same issue on a production wp3 multisite installation after the latest wp upgrade wiped out my hard-coded changes. My sites are running on dreamhost and my install is quite typical. I've run plenty of php (Wordpress, Joomla, and custom sites) on this host and never encountered any similar problems. I'm happy to help debug this if anyone is interested in following up. As stated in my original post, it looks like php.net is recommending using the buffer flushing functions in combination with readfile(). The comments on the page linked above confirm that there are known issues using readfile() without flushing the output buffer beforehand, so it seems like the proper thing to do here is update ms-files.php in wordpress.
#6
@
14 years ago
Unexpected output can be caused by UTF-8 byte order mark or spaces before <?php
or after ?>
in wp-config.php
, theme's functions.php
or any of the plugins. In this case there might be other problems than serving files with ms-files.php
.
#7
@
14 years ago
Sergey, I think you have the wrong idea about this issue; it is not that the output is corrupted by something at the beginning, but that it is truncated -- the end is missing.
#8
@
14 years ago
It seems to me that you and pj_mfc actually have two different issues. Am I wrong?
#9
@
14 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to Future Release
#10
@
14 years ago
@Sergy, I don't think so.
At least, if, as pj_mfc states, the flush() after readfile() fixed the problem, then the problem is not extra data output before an image file, corrupting it: It would still be corrupted with the flush().
#11
@
14 years ago
@MikeLittle, I think Sergy is correct. You (assuming mikelittle == MikeLittle) are talking about putting a flush() after the readfile() call, while pj_mfc is talking about putting ob_clean() and flush() before the readfile() call.
I came here to report pj_mfc's problem/solution myself, so can confirm both (the problem and the fix).
As described here: http://core.trac.wordpress.org/ticket/16306 there's not much that can be done about errant whitespace in theme/plugin files, so WP should clean & flush before the readfile() call.
And, heck, let's flush() again, as you suggested, in case anything is left in the bowl, er, buffer.
#13
@
14 years ago
@fuscata OK. I'm happy to accept that whitespace output before ms-files.php gets to do it's thing will match the symptoms originally reported here.
I may have inadvertently coloured my reading of it with my own symptoms, but my case was definitely not that situation, and nor was it solved in all cases by a simple flush() after the call to readfile().
When I get a chance (likely after this week) I will try reproducing the problem again with 3.0.4 (i.e taking out my custom code)and if I *can* reproduce it will raise a new ticket for that problem.
#14
@
13 years ago
I reproduced this on WordPress 3.2.1 although it only affected files that were smaller than 8k.
Any image larger than 8k works fine, images smaller than 8k return nothing
$ wget http://triblocal.dev/evanston/files/cache/2012/01/Selver_3/786845602.jpg --2012-01-05 14:56:26-- http://triblocal.dev/evanston/files/cache/2012/01/Selver_3/786845602.jpg Resolving triblocal.dev (triblocal.dev)... 127.0.0.1 Connecting to triblocal.dev (triblocal.dev)|127.0.0.1|:80... connected. HTTP request sent, awaiting response... No data received. Retrying.
After I add flush();
after the readfile();
it works
$ wget http://triblocal.dev/evanston/files/cache/2012/01/Selver_3/786845602.jpg --2012-01-05 14:58:11-- http://triblocal.dev/evanston/files/cache/2012/01/Selver_3/786845602.jpg Resolving triblocal.dev (triblocal.dev)... 127.0.0.1 Connecting to triblocal.dev (triblocal.dev)|127.0.0.1|:80... connected. HTTP request sent, awaiting response... 200 OK Length: 7306 (7.1K) [image/jpeg] Saving to: `786845602.jpg.1' 100%[====================================================================================================================================================================================================================================================================================>] 7,306 --.-K/s in 0s 2012-01-05 14:58:11 (332 MB/s) - `786845602.jpg.1' saved [7306/7306]
#15
@
13 years ago
- Cc kpayne@… added
The following info will help other devs to diagnose and reproduce the problem and test the patch:
- What OS is your server using?
- What web server are you using?
- With or without gzip / deflate compression?
- Are you using mod_php or a cgi variant?
- What is your server's php version?
- With
ob_gzhandler
enabled orzlib.output_compression
enabled? - Are you going through a proxy (or have a reverse proxy in front of your web servers)?
- Can you reproduce this different browsers, or just wget?
- Have you tried different file types or just images?
By the way, I had a good discussion with dd32 in #18525 about buffering / caching related to flush()
that may be in some way related.
#18
@
12 years ago
- Keywords close added
Now that ms-files.php
is no longer relied on for new installs, this issue is probably not a priority. Suggesting we close as wontfix unless we think there's a clear reason to move forward.
#19
@
11 years ago
- Keywords needs-testing close removed
- Milestone Future Release deleted
- Resolution set to wontfix
- Status changed from accepted to closed
The original issue reported sounds more like an issue where characters were being output unknowingly before readfile()
occured.
#20
@
11 years ago
- Resolution changed from wontfix to worksforme
If anyone does continue to run into this issue in 3.6 or later, I am OK with adding a flush() after readfile() as that seems harmless, if that indeed fixes said problem. Please reopen if so.
Changing to worksforme
as this is an issue of not knowing what the problem is or how to fix it; versus wontfix
which would indicate we are deliberately not fixing it.
#21
@
10 years ago
- Resolution worksforme deleted
- Status changed from closed to reopened
- Version set to 4.2.2
We have a multisite install where this issue keeps recurring (currently on 4.2.2).
A number of times after an update, we've had broken images, and have had to edit ms-files.php in order to fix it.
As far as I can tell, this is ongoing, and if adding the flush call is indeed harmless, it would certainly help.
There also appear to be a few support threads dotted around reporting similar issues with no clear fix. Who knows, this might resolve it for them..?
For the record, we just upgraded to 4.3 and had to edit ms-files.php again
This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.
9 years ago
#25
@
9 years ago
- Milestone changed from Awaiting Review to 4.6
- Owner changed from westi to iamfriendly
- Status changed from reopened to assigned
@iamfriendly is going to take a look at this with a large multisite install. :)
Let's do what we can to either patch or close in 4.6.
This ticket was mentioned in Slack in #core-multisite by richardtape. View the logs.
9 years ago
#27
follow-up:
↓ 28
@
9 years ago
Replicating this delightful issue isn’t easy. It doesn’t appear to happen on every upgrade. Nailing down exactly why it happens appears to be beyond me.
However, the patch — as-is — doesn’t ‘break’ anything as far as I can tell. It seems harmless. After applying it, and running several upgrades, I haven’t been able to replicate the broken images. :tada:
That being said, I’m starting to think that an even safer strategy is to add actions before and after the readfile()
rather than some magic flushing. This way, the change would be completely benign on our part, and if the flushing/cleaning is the answer to all life’s problems for some installs, then it can be added in a mu-plugin for those affected. Thoughts?
#28
in reply to:
↑ 27
@
9 years ago
Replying to iamfriendly:
That being said, I’m starting to think that an even safer strategy is to add actions before and after the readfile() rather than some magic flushing.
This would be great, but because of the SHORTINIT
in ms-files.php
, not even mu-plugins are available. :/
#29
@
9 years ago
Darn.
OK, in that case, my initial comment stands. The magic flushing certainly didn't break anything on our environment in any of the test updates I ran. I heavily suspect that adding them will not be harmful to other folks still using ms-files.php
- perhaps it's worthy of going in relatively early in the cycle to allow people to test.
#30
@
9 years ago
- Keywords 4.7-early added
- Milestone changed from 4.6 to Future Release
I'm going to bump this. I'd like to understand exactly what it is we're solving first and get it in early. Let's do 4.7 instead.
This ticket was mentioned in Slack in #core by helen. View the logs.
8 years ago
#32
@
8 years ago
- Keywords 4.7-early removed
- Milestone changed from Future Release to 4.7
- Owner changed from iamfriendly to jeremyfelt
- Status changed from assigned to reviewing
#33
@
8 years ago
@jeremyfelt What is needed to help prevent this from being bumped from 4.7? Can you lay out an attack plan?
#34
@
8 years ago
I'm going to go ahead and commit part of the original patch and place a flush()
after readfile()
. Based on @iamfriendly and my testing, I don't believe this will cause any issues. Based on the extensive testing in from several years ago, it seems like this is the right approach for something that appears to be incredibly hard to reproduce. :)
Notes on approaches:
- The replacement
readfile()
function no longer exists at php.net, but can be found around the web asreadfile_chunked()
(not sure what its licensing is). It usesfopen()
,fgets()
,fread()
, etc... to chunk through a larger file. - I can't make sense of adding
ob_clean()
andflush()
beforereadfile()
. There are no themes or plugins in play here, and only headers have been built byms-files.php
whenreadfile()
is called. It's very possible I'm missing something, but I'd rather not add more than we need. - Looking at the source of
readfile()
, it (viaphp_stream_passthru
) outputs all remaining data from stream to the active output buffer and returns the number of bytes output. If buffering is disabled, the data is written straight to the output. It seems harmless to then useflush()
after to ensure all of the output has been pushed.
I'm still not confident in the "why", but this should resolve the issue. We can readdress if it continues to appear.
You most likely have something else going on that is causing a few bytes to slip into the output of the page. I'm not sure of the purpose is for ob_clean() in that example other than to prevent unrelated output buffers from interfering. It may be a technique I simply haven't heard of before, but by default ms-files.php shouldn't cause those kinds of problems.