Tomasz Muras
added a comment - 17/Jan/11 10:11 PM The code is on github: https://github.com/enovation/moodle/tree/MDL-26028 . One important bit is that I had to change:
protected function get_content_file_location()
to:
public function get_content_file_location()

Hello,
did you verify that the headers returned through X-Sendfile are 100% the same compared to the current way (preferably on all types of servers, not just apache)? If not it might create unexpected XSS issues.

I think that the benefit will be quite low because the biggest cost is our session, security checks and general overhead.

In any case thanks a lot for evaluating this potential performance improvement! I am going to make this a subtask in the existing meta issue.

Petr Skoda
added a comment - 17/Jan/11 10:22 PM Hello,
did you verify that the headers returned through X-Sendfile are 100% the same compared to the current way (preferably on all types of servers, not just apache)? If not it might create unexpected XSS issues.
I think that the benefit will be quite low because the biggest cost is our session, security checks and general overhead.
In any case thanks a lot for evaluating this potential performance improvement! I am going to make this a subtask in the existing meta issue.
Petr

Hubert Chathi
added a comment - 18/Jan/11 12:53 AM Lighttpd 1.4 uses the "X-LIGHTTPD-send-file" header for this. I'll try this out later on (today hopefully).
FWIW, the headers might not be 100% the same – the web server may calculate its own ETag header (and maybe others).

Trying with lighttpd, downloading a file from the front page (so all users have access), the headers are the same except for the Accept-Ranges: header – lighttpd sends Accept-Ranges: none, while plain Moodle sends Accept-Ranges: bytes. I'm going to do some more testing.

Nginx's X-Accel-Redirect doesn't seem very straight forward, and I think we should get someone who uses Nginx to test that setting out (and document how Nginx needs to be configured for it to work) before we add that option.

Hubert Chathi
added a comment - 18/Jan/11 5:56 AM - edited Trying with lighttpd, downloading a file from the front page (so all users have access), the headers are the same except for the Accept-Ranges: header – lighttpd sends Accept-Ranges: none, while plain Moodle sends Accept-Ranges: bytes. I'm going to do some more testing.
My updates (lighttpd support, added language strings, add support to the send_file() function, and some cleanups) are available here: http://vcs.uhoreg.ca/git/cgit/moodle/log/?h=MDL-26028
Nginx's X-Accel-Redirect doesn't seem very straight forward, and I think we should get someone who uses Nginx to test that setting out (and document how Nginx needs to be configured for it to work) before we add that option.
Edit: updated link to my git browser

The files uploaded by students have intentionally borked file headers (aka forcedownload), if anything fixes them we have a XSS problem.

For lighttpd the docs (well, blog post) says: "All the response headers from the backend are forwarded, just Content-Length is added.". And when I tested it, it maintained the "Content-Type: application/x-forcedownload" and "Content-Disposition: attachment; ..." headers.

From what I can tell, the Apache module should maintain those headers too (it looks like it just modifies ETag, and drops Content-Encoding), but I don't have Apache to try it out.

Hubert Chathi
added a comment - 18/Jan/11 6:14 AM The files uploaded by students have intentionally borked file headers (aka forcedownload), if anything fixes them we have a XSS problem.
For lighttpd the docs (well, blog post) says: "All the response headers from the backend are forwarded, just Content-Length is added.". And when I tested it, it maintained the "Content-Type: application/x-forcedownload" and "Content-Disposition: attachment; ..." headers.
From what I can tell, the Apache module should maintain those headers too (it looks like it just modifies ETag, and drops Content-Encoding), but I don't have Apache to try it out.

I did some informal benchmarking on lighttpd 1.4 using ApacheBench, and here are the results for 'ab -n 1000 -c 100 "http://localhost/xsendfile/pluginfile.php/14/mod_resource/content/1/elisdoc.tar.gz?forcedownload=1"' (1000 requests 100 concurrent, downloading a file uploaded to the front page):

Also, when using X-Sendfile, the lighttpd process was using about 20% CPU according to top (vs around 80% CPU without X-Sendfile), and the PHP processes stayed at their normal ~80-90MB memory usage (vs jumping up to ~100-200MB without X-Sendfile)

The difference between Hubert's and Gabriel's tests and mine is that I was downloading a file that has required user to be authorized - I made ab to pass a cookie to authenticate. In such a case the speed-up may be smaller as there is more PHP logic executed before the file starts downloading.

Anyway, it looks to me that it makes sense to add XSendfile support - the only problem I have is testing with other web servers than Apache.

Tomasz Muras
added a comment - 08/Feb/11 5:24 PM The difference between Hubert's and Gabriel's tests and mine is that I was downloading a file that has required user to be authorized - I made ab to pass a cookie to authenticate. In such a case the speed-up may be smaller as there is more PHP logic executed before the file starts downloading.
Anyway, it looks to me that it makes sense to add XSendfile support - the only problem I have is testing with other web servers than Apache.

Petr Škoda, we found that, for small files, file.php will performs better (on response time), but as the filesize grows X-Sendfile with optimizations starts to became very similar, and with files bigger then 10Mb it performs better (I have no idea why 100Mb benchmark stated it wrong but I can guess there was a problem collecting the data near that test).

This is some kind obvious, as it's faster, for small files, to just let the php handle it in one step, not two (like what is needed with X-Sendfile).

But the chief gain is that X-Sendfile can handle more "Hits per second" or better saying it can deliver more files at the same time then "php" version.

This can be seen by looking at "duration" graphs. (Fewer times are better)

Gabriel Mazetto
added a comment - 08/Feb/11 10:43 PM Petr Škoda, we found that, for small files, file.php will performs better (on response time), but as the filesize grows X-Sendfile with optimizations starts to became very similar, and with files bigger then 10Mb it performs better (I have no idea why 100Mb benchmark stated it wrong but I can guess there was a problem collecting the data near that test).
This is some kind obvious, as it's faster, for small files, to just let the php handle it in one step, not two (like what is needed with X-Sendfile).
But the chief gain is that X-Sendfile can handle more "Hits per second" or better saying it can deliver more files at the same time then "php" version.
This can be seen by looking at "duration" graphs. (Fewer times are better)

I can test with lighttpd. The other servers that I know of that support this are Nginx and Cherokee. (I may try out Cherokee at some point.) I don't know if IIS has any similar support (whether official or not). As far as what to test, we need to test that it serves the correct file contents (obviously), and that it doesn't munge important headers (especially the headers related to forcedownload). Is there anything else?

Hubert Chathi
added a comment - 10/Feb/11 10:16 PM I can test with lighttpd. The other servers that I know of that support this are Nginx and Cherokee. (I may try out Cherokee at some point.) I don't know if IIS has any similar support (whether official or not). As far as what to test, we need to test that it serves the correct file contents (obviously), and that it doesn't munge important headers (especially the headers related to forcedownload). Is there anything else?

Major benefit from sendfile support is ability to offload file sending (which may take significant amount of time because of large file size/slow client connection) to lightweight threaded webservers (lighttpd, nginx) instead of heavy php processes. It is very inefficient to use full-blown php process to simply serve a static file.

Currently I am investigating nginx + php-fpm setup for Moodle, and definitely can help with testing of this feature.

Alexey Dushechkin
added a comment - 22/Oct/11 5:50 PM Major benefit from sendfile support is ability to offload file sending (which may take significant amount of time because of large file size/slow client connection) to lightweight threaded webservers (lighttpd, nginx) instead of heavy php processes. It is very inefficient to use full-blown php process to simply serve a static file.
Currently I am investigating nginx + php-fpm setup for Moodle, and definitely can help with testing of this feature.

Just came back to this ticket to see if it's already accepted or not, and as Alexey said, if we consider nginx + php-fpm, apache2 + php-fpm, or even apache2 + php with fastcgi, offloading file transmission to the webserver means that your PHP process is free to serve other users while the webserver takes the responsability to sending the file.

Just imagine these two scenarios:

A not so big file but user with really slow connection

A big file (maybe a movie) and a user with a quite decent connection

In both situations, the PHP process will stay "blocked" throttling the download and not serving the other users. Just imagine that you spawned 10 process and you have 10 users downloading files with a really slow connection, your site became irresponsive until the users finish their downloads.

X-Sendfile is a major improvement and should really be considered as a major feature for next release.

If no one volunteers to code this feature, I will probably do it, but will need some help to not mess existing features.

Gabriel Mazetto
added a comment - 08/Mar/12 10:52 AM Just came back to this ticket to see if it's already accepted or not, and as Alexey said, if we consider nginx + php-fpm, apache2 + php-fpm, or even apache2 + php with fastcgi, offloading file transmission to the webserver means that your PHP process is free to serve other users while the webserver takes the responsability to sending the file.
Just imagine these two scenarios:
A not so big file but user with really slow connection
A big file (maybe a movie) and a user with a quite decent connection
In both situations, the PHP process will stay "blocked" throttling the download and not serving the other users. Just imagine that you spawned 10 process and you have 10 users downloading files with a really slow connection, your site became irresponsive until the users finish their downloads.
X-Sendfile is a major improvement and should really be considered as a major feature for next release.
If no one volunteers to code this feature, I will probably do it, but will need some help to not mess existing features.

I've lightly tested it in lighttpd, and for the things that I've tested, the security things were fine, but it needs to be tested more (and it needs to be documented what has been tested an how), and on different servers.

Hubert Chathi
added a comment - 09/Mar/12 2:19 AM Gabriel: it has already been coded. See http://vcs.uhoreg.ca/git/cgit/moodle/log/?h=MDL-26028 . It just needs to be tested to make sure that:
all file downloads are handled
downloads are processed correctly (sends the correct file contents)
Petr's security concerns are handled correctly
I've lightly tested it in lighttpd, and for the things that I've tested, the security things were fine, but it needs to be tested more (and it needs to be documented what has been tested an how), and on different servers.

Gabriel Mazetto
added a comment - 09/Mar/12 4:42 AM Well thank you for that, I didn't saw your first comment with the link to the repository.
I will make a test with nginx and paste configuration instructions.

Hubert Chathi
added a comment - 09/Mar/12 5:00 AM - edited Ah, good, nginx. I'm not sure that I completely understood its X-Accel-Redirect documentation, so it's good if you can test it (and fix it if it's incorrect).

I also found it difficult to apply your modification as your code is based on January 2011 codebase. Could you please update it to a more recent version? If possible also could you please do it on a public accessible git address (like on github)?

Gabriel Mazetto
added a comment - 11/Mar/12 6:19 AM - edited according to documentation:
Note that the following HTTP headers aren't modified by nginx:
Content-Type
Content-Disposition
Accept-Ranges
Set-Cookie
Cache-Control
Expires
source: http://wiki.nginx.org/XSendfile
I also found it difficult to apply your modification as your code is based on January 2011 codebase. Could you please update it to a more recent version? If possible also could you please do it on a public accessible git address (like on github)?

Sorry, I don't have time to update the patch right away. I'll see if I have time soon. (By the way, in case you didn't notice, there are two patches that need to be applied – one by Tomasz, and one by myself.)

Hubert Chathi
added a comment - 12/Mar/12 10:06 PM - edited Sorry, I don't have time to update the patch right away. I'll see if I have time soon. (By the way, in case you didn't notice, there are two patches that need to be applied – one by Tomasz, and one by myself.)
It is on a public git repository; you can "git clone http://vcs.uhoreg.ca/git/moodle.git ", and use the MDL-26028 . If you have a recent git, you can just do "git clone -b MDL-26028 http://vcs.uhoreg.ca/git/moodle.git ".

To make it work you will have to configure 2 internal locations in nginx, one for your root dir and one for moodleadta:
— snip —
location /moodledata/

{
internal;
alias /path/to/moodledata/;
}

location /directfile/

{
internal;
alias /path/to/http/root/directory/;}

— snip —

In the config file you will have to setup those locations:
$CFG->dataroot_xaccelredirect = '/moodledata/';
$CFG->dirroot_xaccelredirect = '/directfile/';

The implementation should be compatible with "non nginx setups", except for one buffer flushing I had to remove in prepare_file_content_sending() [lib/filelib.php]. I'm not sure about the implications of this modification, so if you are familiar with the subject please comment.

Loic Jeannin
added a comment - 19/Apr/12 5:34 AM I've implemented a more "spread" use of nginx's X-Accel-Redirect against v2.2.2. You can find my initial work here
http://http://git.digitalsk.com.br/moodle.git branch xaccel
To make it work you will have to configure 2 internal locations in nginx, one for your root dir and one for moodleadta:
— snip —
location /moodledata/
{
internal;
alias /path/to/moodledata/;
}
location /directfile/
{
internal;
alias /path/to/http/root/directory/;}
— snip —
In the config file you will have to setup those locations:
$CFG->dataroot_xaccelredirect = '/moodledata/';
$CFG->dirroot_xaccelredirect = '/directfile/';
The implementation should be compatible with "non nginx setups", except for one buffer flushing I had to remove in prepare_file_content_sending() [lib/filelib.php] . I'm not sure about the implications of this modification, so if you are familiar with the subject please comment.
Thanks,

Loic, I'm having problems fetching from your repository. When I try to do a git fetch, it just hangs.

Anyways, I don't think that you need the /directfile/ rule. As far as I can tell, you should be able to just do "X-Accel-Redirect: /path/to/file/relative/to/http/root" if you're sending a file that is in the Moodle web directory.

Hubert Chathi
added a comment - 22/Apr/12 10:35 AM Loic, I'm having problems fetching from your repository. When I try to do a git fetch, it just hangs.
Anyways, I don't think that you need the /directfile/ rule. As far as I can tell, you should be able to just do "X-Accel-Redirect: /path/to/file/relative/to/http/root" if you're sending a file that is in the Moodle web directory.

finally I have installed xsendfile support for my test server, I am going to work on it this week, hopefully to be submitted for integration for next week (master). Thanks for all the patches and information!

Petr Skoda
added a comment - 23/Apr/12 4:25 AM Hi,
finally I have installed xsendfile support for my test server, I am going to work on it this week, hopefully to be submitted for integration for next week (master). Thanks for all the patches and information!
Petr

Petr Skoda
added a comment - 23/Apr/12 4:34 AM - edited notes:
we must not give away the filepool file path, instead I guess the easiest may be to move this inside the stored_file
I agree we can not use xsendfile when dontdie or temp file served - because we can not delete files afterserving
I like the idea of readfile_accel() that does the selected serving
prepare_file_content_sending() should be optionally called from readfile_accel()
it might be better to allow configuration from config.php only because we need to have the setting available also before we load the CFG from database
I did not know there are so many different xsendfile ways - I will definitely need testers

Petr Skoda
added a comment - 24/Apr/12 4:31 AM Hmm, there are two major changes pending in master - image preview and repository linking, I am afraid this will have to wait a bit more, hopefully the feature freeze will not block this...

Hubert,
did you do : git clone http://git.digitalsk.com.br/moodle.git ? It should work.
For the /directfile/ you might be right but I have not tested that way. I assumed I had to declare the location "internal;" to work with X-Accel-Redirect.

Loic Jeannin
added a comment - 28/Apr/12 12:36 AM Hubert,
did you do : git clone http://git.digitalsk.com.br/moodle.git ? It should work.
For the /directfile/ you might be right but I have not tested that way. I assumed I had to declare the location "internal;" to work with X-Accel-Redirect.

Loic,
I tried cloning again, and it still hangs. I'm not sure what's wrong. Thanks for attaching the patch, though.
The "internal" is just a suggestion, to make sure that files that shouldn't be otherwise available aren't. So it isn't needed if nginx already serves that directory.
Regarding your patch, a few comments:

it would be better to put your readfile_accel in an existing library file instead of creating a new one

you should follow the Moodle coding style (the thing I noticed was your indenting)

there's no guarantee that the Moodle data directory will contain the word "moodledata", or that something in the main Moodle directory won't contain "moodledata" in its filename (e.g. if someone decides for whatever reason to create a plugin with "moodledata" in the name). So it would be best to compare against the actual contents of $CFG->dataroot. The "else" should also explicitly check against dirroot, in case someone, for whatever reason, calls that function on a file that isn't in either directory.

Comparing Loic's patch against Tomasz/my patch, I would say that Loic's patch is more thorough in that it also uses acceleration for theme files. Some benchmarks show that acceleration is less important for small files, so it's debatable how important it is for theme files. The downside to doing acceleration for theme files is that the theme PHP files use ABORT_AFTER_CONFIG, which means that it must be configured in config.php instead of in the database (or, at least, if it's configured in the database, the theme PHP files won't see the configuration). IMHO, I don't see a reason not to use acceleration in the theme files if it's possible, but we also want to allow users to configure those options within Moodle. So I would suggest that we add the options in the Moodle UI, but add a note saying that if they want theme files to be accelerated, then they need to set the options in the config.php file.

BTW, I also now have access to an nginx server, so I can test both Lighttpd and nginx.

Hubert Chathi
added a comment - 28/Apr/12 1:40 AM Loic,
I tried cloning again, and it still hangs. I'm not sure what's wrong. Thanks for attaching the patch, though.
The "internal" is just a suggestion, to make sure that files that shouldn't be otherwise available aren't. So it isn't needed if nginx already serves that directory.
Regarding your patch, a few comments:
it would be better to put your readfile_accel in an existing library file instead of creating a new one
you should follow the Moodle coding style (the thing I noticed was your indenting)
there's no guarantee that the Moodle data directory will contain the word "moodledata", or that something in the main Moodle directory won't contain "moodledata" in its filename (e.g. if someone decides for whatever reason to create a plugin with "moodledata" in the name). So it would be best to compare against the actual contents of $CFG->dataroot. The "else" should also explicitly check against dirroot, in case someone, for whatever reason, calls that function on a file that isn't in either directory.
Comparing Loic's patch against Tomasz/my patch, I would say that Loic's patch is more thorough in that it also uses acceleration for theme files. Some benchmarks show that acceleration is less important for small files, so it's debatable how important it is for theme files. The downside to doing acceleration for theme files is that the theme PHP files use ABORT_AFTER_CONFIG, which means that it must be configured in config.php instead of in the database (or, at least, if it's configured in the database, the theme PHP files won't see the configuration). IMHO, I don't see a reason not to use acceleration in the theme files if it's possible, but we also want to allow users to configure those options within Moodle. So I would suggest that we add the options in the Moodle UI, but add a note saying that if they want theme files to be accelerated, then they need to set the options in the config.php file.
BTW, I also now have access to an nginx server, so I can test both Lighttpd and nginx.

> it would be better to put your readfile_accel in an existing library file instead of creating a new one

which one do you suggest ?

> you should follow the Moodle coding style (the thing I noticed was your indenting)

Ok, my vim settings are ok now.

> there's no guarantee that the Moodle data directory will contain the word "moodledata", or that
>something in the main Moodle directory won't contain "moodledata" in its filename (e.g. if someone decides
>for whatever reason to create a plugin with "moodledata" in the name). So it would be best to compare
>against the actual contents of $CFG->dataroot. The "else" should also explicitly check against dirroot, in
>case someone, for whatever reason, calls that function on a file that isn't in either directory.

Right, I've already noticed that too.

I'm going to rewrite following the coding style (tabs/spaced are already fixed) and post it here.

Loic Jeannin
added a comment - 28/Apr/12 2:04 AM > it would be better to put your readfile_accel in an existing library file instead of creating a new one
which one do you suggest ?
> you should follow the Moodle coding style (the thing I noticed was your indenting)
Ok, my vim settings are ok now.
> there's no guarantee that the Moodle data directory will contain the word "moodledata", or that
>something in the main Moodle directory won't contain "moodledata" in its filename (e.g. if someone decides
>for whatever reason to create a plugin with "moodledata" in the name). So it would be best to compare
>against the actual contents of $CFG->dataroot. The "else" should also explicitly check against dirroot, in
>case someone, for whatever reason, calls that function on a file that isn't in either directory.
Right, I've already noticed that too.
I'm going to rewrite following the coding style (tabs/spaced are already fixed) and post it here.

> it would be better to put your readfile_accel in an existing library file instead of creating a new one
which one do you suggest ?

Sorry, I thought that I erased that comment.
Usually, I would suggest either moodlelib or weblib. But in this case, since you're calling the function from the theme PHP files, and it looks like there's a desire to keep those as light as possible, I don't know if that would work. So in this particular case, it might be fine the way you have it.

Hubert Chathi
added a comment - 28/Apr/12 2:11 AM
> it would be better to put your readfile_accel in an existing library file instead of creating a new one
which one do you suggest ?
Sorry, I thought that I erased that comment.
Usually, I would suggest either moodlelib or weblib. But in this case, since you're calling the function from the theme PHP files, and it looks like there's a desire to keep those as light as possible, I don't know if that would work. So in this particular case, it might be fine the way you have it.

Loic Jeannin
added a comment - 28/Apr/12 2:59 AM I've updated the git repository and the attached patch, now the coding style should be fixed.
I have 2 issues remaining:
the buffer flushing is still commented (prepare_file_content_sending())
the stored_file->readfile() is still unaware of the "dontdie" option

Thanks everybody for the patch, while I was refactoring the file apis a bit I realised we could use the contenthash of stored files as etag and significantly improve performance of force downloaded files such as forum attachments.

I have tested my patch with Apache X-Sendfile module only, hopefully it will work with Nginx and Lighttpd too.

Petr Skoda
added a comment - 30/Apr/12 2:08 AM Thanks everybody for the patch, while I was refactoring the file apis a bit I realised we could use the contenthash of stored files as etag and significantly improve performance of force downloaded files such as forum attachments.
I have tested my patch with Apache X-Sendfile module only, hopefully it will work with Nginx and Lighttpd too.
Please test and make some noise to help with getting this integrated.
Petr

Petr,
The nginx part in readaccel is missing a way to add the prefix configured in nginx (internal or not) for a direct access to the dataroot.
The way you wrote it should work OOTB with $dirroot but not with $dataroot.

Loic Jeannin
added a comment - 30/Apr/12 9:23 PM Petr,
The nginx part in readaccel is missing a way to add the prefix configured in nginx (internal or not) for a direct access to the dataroot.
The way you wrote it should work OOTB with $dirroot but not with $dataroot.
from your patch (line 61):
$filepath = substr($filepath, strlen($dataroot));
should be something like that:
$filepath = $CFG->dataroot_nginxprefix . substr($filepath, strlen($dataroot));
Thanks,

Sorry I don't understand what you mean with whitelist.
X-Accel-Redirect is a bit more complicated than the other solutions. It does not work with file paths, only with url, so you have to configure all the locations you want to reach in your nginx config. I had'nt thought of the cases where cachedir/tempdir are outside the dataroot; in this case you will have to configure each dir separately and check in xsendfilelib in which dir the file is in to add the proper url prefix.
I'm afraid there is no "easy way" to configure it.

Loic Jeannin
added a comment - 30/Apr/12 9:54 PM Sorry I don't understand what you mean with whitelist.
X-Accel-Redirect is a bit more complicated than the other solutions. It does not work with file paths, only with url, so you have to configure all the locations you want to reach in your nginx config. I had'nt thought of the cases where cachedir/tempdir are outside the dataroot; in this case you will have to configure each dir separately and check in xsendfilelib in which dir the file is in to add the proper url prefix.
I'm afraid there is no "easy way" to configure it.
Thanks,

Thanks for the explanation, I think I understood that correctly, my patch allows unlimited number of directory aliases which are selected automatically based on the real file path of the served path. Could somebody please verify it works? I do not have nginx install here. Or if somebody tells me how to configure nginx from macports on OSX I could do it myself...

Petr Skoda
added a comment - 30/Apr/12 10:11 PM Thanks for the explanation, I think I understood that correctly, my patch allows unlimited number of directory aliases which are selected automatically based on the real file path of the served path. Could somebody please verify it works? I do not have nginx install here. Or if somebody tells me how to configure nginx from macports on OSX I could do it myself...

arrgh, separators again, thanks!!! Why are you replacing the separators in your patch? For me the realpath() removes all doubles and trims the trailing separator. It seems it might break only if you had '/' or "d:\" in alias, oh, but that is correct value too (but I guess not recommended) - fixed too.

please retest, I have used a bit different logic that detects invalid dirs too.

Petr Skoda
added a comment - 01/May/12 12:10 AM - edited arrgh, separators again, thanks!!! Why are you replacing the separators in your patch? For me the realpath() removes all doubles and trims the trailing separator. It seems it might break only if you had '/' or "d:\" in alias, oh, but that is correct value too (but I guess not recommended) - fixed too.
please retest, I have used a bit different logic that detects invalid dirs too.

Loic Jeannin
added a comment - 01/May/12 1:54 AM Ok, test passed in linux env nginx + php_fpm. To be extensive we should find someone with a windows setup and nginx (good luck).
On the actual code : it's much cleaner to test only the last char.
Thanks,

Ankit Agarwal
added a comment - 03/May/12 1:15 PM Results of Etag test:-
-> Uploaded an image to forum.304 status is returned as expected on multiple page loads.
-> Forum attachment was set to force download.
Results of X-sendifle test:-
X-sendfile set to on
-> I got following strict standard errors all over the place( probably not related to above fix)
Strict Standards: Creating default object from empty value in /var/www/int/master/moodle/grade/lib.php on line 225
Strict Standards: Redefining already defined constructor for class grade_export_txt in /var/www/int/master/moodle/grade/export/txt/grade_export_txt.php on line 31
Strict Standards: Creating default object from empty value in /var/www/int/master/moodle/grade/lib.php on line 219
Strict Standards: Creating default object from empty value in /var/www/int/master/moodle/mod/scorm/locallib.php on line 538
Passing
Thanks