WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 8 months ago

Last modified 8 months ago

#14730 closed defect (bug) (worksforme)

ms-files.php required ob_clean() and flush()

Reported by: pj_mfc Owned by: westi
Milestone: Priority: normal
Severity: normal Version:
Component: Multisite Keywords: has-patch
Focuses: 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:

http://us2.php.net/manual/en/function.readfile.php

Attachments (1)

ms-files.php.patch (185 bytes) - added by fuscata 3 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 nacin4 years ago

  • Component changed from General to Multisite

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.

comment:2 mikelittle4 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.

comment:3 westi4 years ago

  • Owner set to westi
  • Status changed from new to accepted

comment:4 mikelittle4 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.

comment:5 pj_mfc3 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.

comment:6 SergeyBiryukov3 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.

comment:7 MikeLittle3 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.

comment:8 SergeyBiryukov3 years ago

It seems to me that you and pj_mfc actually have two different issues. Am I wrong?

comment:9 nacin3 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

comment:10 MikeLittle3 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().

comment:11 fuscata3 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.

fuscata3 years ago

comment:12 fuscata3 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

comment:13 MikeLittle3 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.

comment:14 willmot2 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]
Last edited 2 years ago by willmot (previous) (diff)

comment:15 kurtpayne2 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 or zlib.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.

comment:16 Ipstenu2 years ago

  • Cc ipstenu@… added

comment:17 dragunoff15 months ago

  • Cc dragunoff@… added

comment:18 jeremyfelt8 months 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.

comment:19 jeremyfelt8 months 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.

comment:20 nacin8 months 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.

Note: See TracTickets for help on using tickets.