Now ETag header must be always present, in the form of Etag: "<content hash>". Please note that the case Moodle adopted for the ETag header is with T being in lower case.

Chrome must be used first, especially to check the combination with $CFG->xsendfile: the issue has been found when playing MP4 files in Chrome under some combinations of web deployments where X-SendFile was used for performance reasons. The most simple combination is: Apache/2 + mod_xsendfile + $CFG->xsendfile = 'X-Sendfile'; in config.php

In order to verify that caching capabilities work as expected you could use one of the MP4 files used to investigate the issue e.g.:

Before launching the URL above, please open Fiddler and press F12 to start "capturing" the network traffic

Open the browser, Chrome, press F12 and select the Network tab

Copy the URL above to let Chrome play the content 'till the end: you'll see in both Fiddler and Network tab the full and partial request according with your eventual playing with the slider, Content-Length will change according with Range requested in case of HTTP 206. In case of testing a local instance, the bigger (> 10MB) the movie the higher the chance to see many different HTTP partial requests when moving the slider forward, over the buffered content and then back again, untill the file will be fully buffered (then cached)

Move the slider up and down to see the partial request being performed and the content being shown from that point of the slider. When the content will be cached, subsequent request will be served from browser cache instead of making requests to the server i.e. Fiddler will register nothing and the Network tab in Chrome will report request being served from the cache

Apply the same testing instruction described in MDL-39688 except for:
Now ETag header must be always present, in the form of Etag: "<content hash>" . Please note that the case Moodle adopted for the ETag header is with T being in lower case.
Chrome must be used first, especially to check the combination with $CFG->xsendfile : the issue has been found when playing MP4 files in Chrome under some combinations of web deployments where X-SendFile was used for performance reasons. The most simple combination is: Apache/2 + mod_xsendfile + $CFG->xsendfile = 'X-Sendfile'; in config.php
In order to verify that caching capabilities work as expected you could use one of the MP4 files used to investigate the issue e.g.:
http://samples.mplayerhq.hu/MPEG-4/vorbis-in-mp4/mi2_vorbis51.mp4
http://www.tools4movies.com/dvd_catalyst_profile_samples/The%20Amazing%20Spiderman%20bionic%20hq.mp4
Download them e.g. using wget and upload each of them in Moodle as a File resource .
Do not launch them directly to perform the tests by means of using the File resource link (e.g.: http://vm-centos5/moodle-master/mod/resource/view.php?id=51 ) but discover the file that will be served by the Files API looking for this URL http://hostname/path/to/moodle/pluginfile.php/ <id>/mod_resource/content/<courseid>/<filename> in the media plugin HTML code block (look at the HTML source), e.g.: http://vm-centos5/moodle-master/pluginfile.php/78/mod_resource/content/1/mi2_vorbis51.mp4
Before launching the URL above, please open Fiddler and press F12 to start "capturing" the network traffic
Open the browser, Chrome, press F12 and select the Network tab
Copy the URL above to let Chrome play the content 'till the end: you'll see in both Fiddler and Network tab the full and partial request according with your eventual playing with the slider, Content-Length will change according with Range requested in case of HTTP 206 . In case of testing a local instance, the bigger (> 10MB) the movie the higher the chance to see many different HTTP partial requests when moving the slider forward, over the buffered content and then back again, untill the file will be fully buffered (then cached)
Cleanup Fiddler capturing
Close Chrome and then reopen it, press F12, select the Network tab, paste the URL, press ENTER
Move the slider up and down to see the partial request being performed and the content being shown from that point of the slider. When the content will be cached, subsequent request will be served from browser cache instead of making requests to the server i.e. Fiddler will register nothing and the Network tab in Chrome will report request being served from the cache

Description

See MDL-39688, this patch started to implement ETag headers based on HTTP ranges.

Further investigations have driven to another solution which should be applied to the branches where MDL-39688 has been applied because of the ETag header added by Moodle is content driven, helping browser caching mechanisms to work as expected even in cluster deployments where e.g. ETag being added by Apache+X-Sendifle could be built on top of file inodes, probably dependent on the web server serving the request even if files are shared ($CFG->dataroot) someway between all the nodes.

Andrew Nicols
added a comment - 25/May/13 11:48 AM Oh, one comment. Is it worth making the $etag parameter optional and defaulting to null or ''? This will break compatibility for any plugins out there serving files themselves for some reason.

Hi, yes, I was considering that too, but because we can not print any debug messages and the error output must be already disabled in these scripts via "define('NO_DEBUG_DISPLAY', true);" this should not create any problems, it is just a notice that will go to error_log, right?

Petr Skoda
added a comment - 25/May/13 11:53 AM Hi, yes, I was considering that too, but because we can not print any debug messages and the error output must be already disabled in these scripts via "define('NO_DEBUG_DISPLAY', true);" this should not create any problems, it is just a notice that will go to error_log, right?

Testing

Needs some testing instructions

Sanity check

General:
Just wondering why you're dropping both ETag and Etag? According to RFC 2616 Section 4.2, "Field names are case-insensitive.". The documentation on header_remove also states that the parameter name is case insensitive.

lib/xsendfile.php:
Also I think it would make more sense to move your new header_etag function out of xsendfilelub.php and into somewhere more appropriate - perhaps lib/weblib.php. Sending etags is not related to XSendFile really.

I guess it makes sense to use header rather than header_etag() when you know that there won't be any byte serving in use.

Other than the points above, this looks good and a much welcomed addition

Andrew Nicols
added a comment - 27/May/13 2:04 AM I agree with you Re the default parameter value - that makes some sense, though because we do set NO_DEBUG_DISPLAY, it may mean that plugin developer may never discover the bug :\
[Y] Syntax
[Y] Output
[Y] Whitespace
[-] Language
[-] Databases
[N] Testing
[Y] Security
[-] Documentation
[Y] Git
[N] Sanity check
Testing
Needs some testing instructions
Sanity check
General:
Just wondering why you're dropping both ETag and Etag? According to RFC 2616 Section 4.2, "Field names are case-insensitive.". The documentation on header_remove also states that the parameter name is case insensitive.
lib/xsendfile.php:
Also I think it would make more sense to move your new header_etag function out of xsendfilelub.php and into somewhere more appropriate - perhaps lib/weblib.php. Sending etags is not related to XSendFile really.
I guess it makes sense to use header rather than header_etag() when you know that there won't be any byte serving in use.
Other than the points above, this looks good and a much welcomed addition

Petr Skoda
added a comment - 27/May/13 9:13 AM Grrr - I read header_remove() was case-sensitive, fixing...
header_etag() can not be in core libs because it is in pages that do not have any core libs available, all they have is CFG from config.php
I think about the hader_etag() once more, but so far I did not find better place for it.
Thanks!

Petr Skoda
added a comment - 27/May/13 1:18 PM Repo updated, I have used the name "_header_etag" to mark it private and I have also added some more inline docs. The tag removing is now fixed too. Thanks a lot.

I must admit that I'm somewhat confused, sad for my ignorance: right now I'm thinking the opposite compared to my own comment linked above that is to say I'm thinking the same way as Andrew does when he quotes http://tools.ietf.org/html/rfc2616#section-13.5.4:ETag must be always related to the full content representing the URL being requested, served partially or not.

To enforce Andrew's point http://tools.ietf.org/html/rfc2616#section-14.27 is more explicit about the ETag to be related with the entire content and not to the partial response, indeed If-Range should do the trick for an outdated partial response.
But... we've evidence that ETag for partial content breaks things as per Andrew's use case: I'm totally lost... unless there is an issue in the way that MP4 client is able to cache partial responses.

ETag in Moodle will always be strong being equal to the content hash and when the content change ETag will change too. Should Moodle implement the support for $_SERVER['HTTP_IF_RANGE'] too (limited or not to X-Sendfile), re-sending the whole file in case of mismatching?
Maybe sniffing the traffic to reconstruct HTTP Requests and HTTP Responses between the two parts while generating the issue, as described in MDL-39688, will help.

Matteo Scaramuccia
added a comment - 27/May/13 9:29 PM Hi All,
I'm keep on reading RFCs and some code: my last comment has been written in https://tracker.moodle.org/browse/MDL-39688?focusedCommentId=224831&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-224831 .
I must admit that I'm somewhat confused, sad for my ignorance: right now I'm thinking the opposite compared to my own comment linked above that is to say I'm thinking the same way as Andrew does when he quotes http://tools.ietf.org/html/rfc2616#section-13.5.4: ETag must be always related to the full content representing the URL being requested, served partially or not.
To enforce Andrew's point http://tools.ietf.org/html/rfc2616#section-14.27 is more explicit about the ETag to be related with the entire content and not to the partial response, indeed If-Range should do the trick for an outdated partial response.
But... we've evidence that ETag for partial content breaks things as per Andrew's use case: I'm totally lost... unless there is an issue in the way that MP4 client is able to cache partial responses.
ETag in Moodle will always be strong being equal to the content hash and when the content change ETag will change too. Should Moodle implement the support for $_SERVER ['HTTP_IF_RANGE'] too (limited or not to X-Sendfile ), re-sending the whole file in case of mismatching?
Maybe sniffing the traffic to reconstruct HTTP Requests and HTTP Responses between the two parts while generating the issue, as described in MDL-39688 , will help.
My apologise for the noise I've created 'till now.
Truly,
Matteo

Petr Skoda
added a comment - 27/May/13 9:33 PM I am also confused a bit after reading the spec, the reality in proxies and client might be different unfortunately.
I am not sure my proposed patch is the correct solution, this was the main reason why I created this issue, I do not think we should rush this through integration now.
Thanks everybody!

Andrew Nicols
added a comment - 28/May/13 5:36 AM I'm glad that I'm not the only one confused then Matteo Scaramuccia !
I've just tried your patch Petr, and it still doesn't work as expected - MP4 only plays within Chrome when we don't return any ETag headers with the byte-served content it seems.
Still investigating.

Andrew Nicols
added a comment - 28/May/13 5:48 AM This only seems to affect when using X-Accel-Redirect (and possibly other X-Sendfile implementations). It does not seem to affect the stock Moodle byte-serving.

Andrew Nicols
added a comment - 28/May/13 6:26 AM - edited Just to confirm, this is also broken with mod_xsendfile on Apache with ETags present both with, and without the range. That's just straight Apache2 -> mod_php5 so no proxying going on.

The only browser I've been able to test in so far is Chrome. It seems that neither Firefox, nor Opera seem to request partial content for mp4 content (grr) so they don't seem to exhibit this bug (yay). They may of course request partial content for other content types but I've not tried that.

Andrew Nicols
added a comment - 28/May/13 6:50 AM The only browser I've been able to test in so far is Chrome. It seems that neither Firefox, nor Opera seem to request partial content for mp4 content (grr) so they don't seem to exhibit this bug (yay). They may of course request partial content for other content types but I've not tried that.
Matteo Scaramuccia and Petr Skoda , what browsers + content are you testing this with?

Hi Andrew,
'till now I've just read specs and code, nothing more: I was waiting for any feedback about what would be the client showing that strange and blocking issue.
I'll play with CR and MP4 plus I'd create some tests using curl to verify Moodle supporting byte serving - which is by far working "on average" - even with If-Range - currently missing, I'm thinking to propose its support for 2.6 - and I'll look at the HTTP Headers exchanged using CR, to identify what could be the issue in terms of HTTP specs compliance or CR specific issue. Unfortunately this work will be done on spare time, typically evening.

Matteo Scaramuccia
added a comment - 28/May/13 7:14 AM - edited Hi Andrew,
'till now I've just read specs and code, nothing more: I was waiting for any feedback about what would be the client showing that strange and blocking issue.
I'll play with CR and MP4 plus I'd create some tests using curl to verify Moodle supporting byte serving - which is by far working "on average" - even with If-Range - currently missing, I'm thinking to propose its support for 2.6 - and I'll look at the HTTP Headers exchanged using CR, to identify what could be the issue in terms of HTTP specs compliance or CR specific issue. Unfortunately this work will be done on spare time, typically evening.
EDIT @ 20130528T0925+02:00 : have you already checked that the byte range provided by CR is a real partial response request i.e. not what described here, http://stackoverflow.com/questions/12801192/client-closes-connection-when-streaming-m4v-from-apache-to-chrome-with-jplayer#answer-12979414? It could be where Moodle byte serving could be improved in replying to a "fake Partial Response" request.

Petr Skoda
added a comment - 28/May/13 9:04 AM - edited Matteo: Do you want this to be assigned to you? I am not sure I would find enough time to create a solution based on reverse engineering of proxies and clients.
Or Andrew, do you want this?

Matteo Scaramuccia
added a comment - 28/May/13 10:12 AM Hi Petr,
my time is unfortunately limited, maybe Andrew's time for this issue could be more than mine: I'll be happy to help in any case as per my capabilities and available time.

Hi Andrew,
this weekend I'll do some investigations&coding, could you provide me a link to an MP4 sample file "big" enough to replicate the issue using Chrome on master running under Apache 2 with mod_xsendfile?
One other potential issue is that ETag must be double quoted, http://tools.ietf.org/html/rfc2616#section-3.11:

entity-tag = [ weak ] opaque-tag

weak = "W/"

opaque-tag = quoted-string

Even if I'll be unsuccessful I'll provide a WIP branch to show how the things should be IMHO changed according with RFC2616.

Matteo Scaramuccia
added a comment - 31/May/13 6:01 PM - edited Hi Andrew,
this weekend I'll do some investigations&coding, could you provide me a link to an MP4 sample file "big" enough to replicate the issue using Chrome on master running under Apache 2 with mod_xsendfile ?
One other potential issue is that ETag must be double quoted, http://tools.ietf.org/html/rfc2616#section-3.11:
entity-tag = [ weak ] opaque-tag
weak = "W/"
opaque-tag = quoted-string
Even if I'll be unsuccessful I'll provide a WIP branch to show how the things should be IMHO changed according with RFC2616.
EDIT: in the mean time I've found some MP4 files here, http://www.thriveforums.org/forum/toshiba-thrive-video/3418-sample-mp4-videos-thrive.html , converted from MOV files.
TIA,
Matteo

I'm not using anything special - I'm just using a free sample MP4 I found somewhere. It doesn't have to be anything long. I uploaded it to a section description, then viewed it in Chrome using the link it displays as to entirely rule out an issue with the Media plugin. I.e. I'm opening pluginfile.php directly. Make sure you've not disabled the browser cache (in Chrome Dev tools).

Andrew Nicols
added a comment - 01/Jun/13 7:45 AM Hi Matteo,
I'm not using anything special - I'm just using a free sample MP4 I found somewhere. It doesn't have to be anything long. I uploaded it to a section description, then viewed it in Chrome using the link it displays as to entirely rule out an issue with the Media plugin. I.e. I'm opening pluginfile.php directly. Make sure you've not disabled the browser cache (in Chrome Dev tools).

Matteo Scaramuccia
added a comment - 01/Jun/13 11:21 AM Hi All,
I've done several experiments using Apache/2.2.3 , mod_xsendfile/0.12 and Chrome with the provided MP4 files ( http://samples.mplayerhq.hu/MPEG-4/vorbis-in-mp4/mi2_vorbis51.mp4 included) changing things as per my previous comments ending with adding double quotes: this last change did the trick.
@ Andrew Nicols : could you test it?
My suggestion is to apply this change in all the branches where MDL-39688 has been applied: I'll be happy to provide them including test instructions.

Hi Petr,
it looks like that: would you like to test it too, reverting first MDL-36888 to see that Chrome will not be happy and then applying my proposal? In my environment works as described, but you know, I could be wrong
I'll double check RFC2616, while waiting for comments from Andrew, for other potential syntax issues.

Quick searched for ((^|\s)header([^LH]+); - No redirect (Location) and HTTP/1.0 statuses -, here the comments on the matches:

Accept-Ranges: no quotes

Cache-Control: no quotes, just for extensions. Not applicable in Moodle

If Andrew and you will confirm my current proposal to work as expected in your environments, I'd suggest to fix all the other ETag headers, e.g. in lib/csslib.php, as part of a separate improvement for 2.6, not sure for the previous versions.
Note: FirePHP::setHeader doesn't apply quotes for headers BUT it is not used to set ETag. Similar comment for XHProf wrapper.

Matteo Scaramuccia
added a comment - 01/Jun/13 1:22 PM - edited Hi Petr,
it looks like that: would you like to test it too, reverting first MDL-36888 to see that Chrome will not be happy and then applying my proposal? In my environment works as described, but you know, I could be wrong
I'll double check RFC2616 , while waiting for comments from Andrew, for other potential syntax issues.
Quick searched for ((^|\s)header( [^LH] +); - No redirect ( Location ) and HTTP/1.0 statuses -, here the comments on the matches:
Accept-Ranges : no quotes
Cache-Control : no quotes, just for extensions. Not applicable in Moodle
Content-Disposition , filename must be quoted: not always in Moodle e.g. grade/edit/outcome/export.php
Content-Encoding : no quotes
Content-Length : no quotes
Content-Type : no quotes. Almost OK except for:
LF in lib/tablelib.php . Same in mod/survey/download.php
charset should not be quoted e.g. not Content-Type: text/xml; charset="utf-8"' : few files affected
Etag : quotes required. Missing quotes in: lib/csslib.php , theme/image.php , theme/yui_image.php , ...
Expires : no quotes
Pragma : no quotes except for extensions. Not applicable in Moodle
Status : no quotes
Last-Modified : no quotes ( HTTP-date )
WWW-Authenticate : quotes for Realm (RFC2617)
If Andrew and you will confirm my current proposal to work as expected in your environments, I'd suggest to fix all the other ETag headers, e.g. in lib/csslib.php , as part of a separate improvement for 2.6, not sure for the previous versions.
Note: FirePHP::setHeader doesn't apply quotes for headers BUT it is not used to set ETag . Similar comment for XHProf wrapper.

Thanks Matteo - I can't believe it was this simple. I've spent a couple of hours trying to track it down and re-read the RFC numerous times. Kudos

I've just tried wrapping in quotes and it does indeed work as expected. Your patch looks good to me.

I think that we should correct the quotes in the other ETag headers you've identified as part of this patch. The Byte Range issues are just a symptom of the real issue. This should also be backported to all stable branches IMO.

I think we should file a new issue for the other header quoting issues you've identified rather than as part of this one.

Andrew Nicols
added a comment - 02/Jun/13 10:20 PM Thanks Matteo - I can't believe it was this simple. I've spent a couple of hours trying to track it down and re-read the RFC numerous times. Kudos
I've just tried wrapping in quotes and it does indeed work as expected. Your patch looks good to me.
I think that we should correct the quotes in the other ETag headers you've identified as part of this patch. The Byte Range issues are just a symptom of the real issue. This should also be backported to all stable branches IMO.
I think we should file a new issue for the other header quoting issues you've identified rather than as part of this one.
Andrew

Do you agree that fixing the other missing quotes could be done in a separate issue? For clarity sake, this issue should IMHO cover just what discussed and done in MDL-39688 but I'll wait for your thoughts.

Matteo Scaramuccia
added a comment - 03/Jun/13 7:51 PM Hi All,
here are the backports of the current proposal.
Do you agree that fixing the other missing quotes could be done in a separate issue? For clarity sake, this issue should IMHO cover just what discussed and done in MDL-39688 but I'll wait for your thoughts.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

Dan Poltawski
added a comment - 06/Jun/13 1:21 PM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.
TIA and ciao

Ankit Agarwal
added a comment - 12/Jun/13 7:52 AM - edited The file doesn't play any more on chrome. The content of file is empty always. This works perfectly on stable.
Let me know if I can provide more info on anything.
Dan has confirmed the issue.
Failing.

To try and add some detail, there are definite scenarios where chrome seems to not load the file when requested directly, but I HAVE seen it load when requested by quicktime. Either way, I have to say it does look to be something fishy with the etag

Dan Poltawski
added a comment - 12/Jun/13 8:06 AM To try and add some detail, there are definite scenarios where chrome seems to not load the file when requested directly, but I HAVE seen it load when requested by quicktime. Either way, I have to say it does look to be something fishy with the etag

Hi Ankit&Dan,
could you give me some details about your tests to let me replicate it in my environment? Here are the questions about what I'm supposing to be required to identify the scope of your tests:

Before starting with tests, did you cleanup Chrome cache? It's unfortunately a missing point in the testing instructions

Could you share the movie used for your tests?

Could you confirm that tests has been performed using the pluginfile.php URL?

Could you confirm that the connection between your browser and the Moodle instance is direct e.g. a local installation i.e. no proxy in between?

Did you use X-Sendfile configuration or just "plain" PHP module connection?

Did you check what both Fiddler and Chrome Dev Tools show during your tests to identify any eventual other reason? (I know, this is the part I must work on... when I'll replicate it )

Matteo Scaramuccia
added a comment - 12/Jun/13 10:16 AM Hi Ankit&Dan,
could you give me some details about your tests to let me replicate it in my environment? Here are the questions about what I'm supposing to be required to identify the scope of your tests:
Before starting with tests, did you cleanup Chrome cache? It's unfortunately a missing point in the testing instructions
Could you share the movie used for your tests?
Could you confirm that tests has been performed using the pluginfile.php URL?
Could you confirm that the connection between your browser and the Moodle instance is direct e.g. a local installation i.e. no proxy in between?
Did you use X-Sendfile configuration or just "plain" PHP module connection?
Did you check what both Fiddler and Chrome Dev Tools show during your tests to identify any eventual other reason? (I know, this is the part I must work on... when I'll replicate it )

Dan Poltawski
added a comment - 13/Jun/13 2:10 AM > 1. Before starting with tests, did you cleanup Chrome cache? It's unfortunately a missing point in the testing instructions
Yes - well I tried in incognito window
> Could you share the movie used for your tests?
We used the mp4 in the testing instructions, and I also used http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4
> Could you confirm that tests has been performed using the pluginfile.php URL?
Yep
> Could you confirm that the connection between your browser and the Moodle instance is direct e.g. a local installation i.e. no proxy in between?
Yep
> Did you use X-Sendfile configuration or just "plain" PHP module connection?
No X-Sendfile
> Did you check what both Fiddler and Chrome Dev Tools show during your tests to identify any eventual other reason? (I know, this is the part I must work on... when I'll replicate it )
No fiddler here as I don't use windows.

TNX for your replies!
About sniffing and options out of a Windows env: WireShark is The Network Sniffer, regardless the OS. You can inspect everything, please filter the traffic limiting it at least to HTTP otherwise you'll be lost in a sea of packets . You'll find several hints in the network e.g. http://www.eriky.com/2009/01/sniffing-http-headers-with-wireshark.
In *nix systems, especially when troubleshooting on the server, tcpdump is a great (and powerful as well) CLI.Fiddler is a really nice and powerful tool for Windows, focused on HTTP requests, easy to use almost for any user: that's the reason why I suggested it, my fault not to consider other options.

I'll perform tests&investigations against your video one of the next evening, unfortunately out of this integration week.

@Ankit Agarwal: when you configure X-Sendfile in a Moodle deployment you need first to install its support in your web deploy: here, you need to install mod_xsendfile since you're running Moodle under Apache. The goal of that extension is, shortly, to:

look for the X-Sendfile header

pickup the path to your local system

REMOVE it from your HTTP Headers otherwise, big disclosure here!

serve your file directly, offloading the PHP module i.e. Moodle will not send anything when running under that context, expecting&trusting that the Web Server will do it as per X-Sendfile specs.

Matteo Scaramuccia
added a comment - 13/Jun/13 5:25 AM TNX for your replies!
About sniffing and options out of a Windows env: WireShark is The Network Sniffer, regardless the OS. You can inspect everything, please filter the traffic limiting it at least to HTTP otherwise you'll be lost in a sea of packets . You'll find several hints in the network e.g. http://www.eriky.com/2009/01/sniffing-http-headers-with-wireshark .
In *nix systems, especially when troubleshooting on the server, tcpdump is a great (and powerful as well) CLI.
Fiddler is a really nice and powerful tool for Windows, focused on HTTP requests, easy to use almost for any user: that's the reason why I suggested it, my fault not to consider other options.
I'll perform tests&investigations against your video one of the next evening, unfortunately out of this integration week.
@ Ankit Agarwal : when you configure X-Sendfile in a Moodle deployment you need first to install its support in your web deploy: here, you need to install mod_xsendfile since you're running Moodle under Apache. The goal of that extension is, shortly, to:
look for the X-Sendfile header
pickup the path to your local system
REMOVE it from your HTTP Headers otherwise, big disclosure here!
serve your file directly, offloading the PHP module i.e. Moodle will not send anything when running under that context, expecting&trusting that the Web Server will do it as per X-Sendfile specs.
That's why you receive nothing: please, install the module in your Ubuntu e.g. http://blog.brightbox.co.uk/posts/apache-x-sendfile-module-for-ubuntu-hardy

Sniffing: I apologize for my misunderstanding your initial point !
BTW, what should be looked for is the correct match between Request type and given Response. If the Request contains an Accept header, the Response should be exactly related (e.g. right Content-Length, before looking at the body itself) and you need to compare the Chrome Dev Tools network information with the ones sniffed to be sure that, at the end, a Request shown in Chrome Dev Tools as served by the internal cache means no hit to the server unless the previous (partial) requests didn't cover the all content length and Chrome is correctly required to ask the server for the missing parts.
Have you already try to cleanup your Chrome cache, perform the same test without the PR to see if your behavior is due to the PR i.e. to the usage of ETag?

Integration: no problem, that's the process. I hope to hear news from Ankit tests setup, too.

Matteo Scaramuccia
added a comment - 13/Jun/13 7:15 AM Hi Dan,
Sniffing: I apologize for my misunderstanding your initial point !
BTW, what should be looked for is the correct match between Request type and given Response. If the Request contains an Accept header, the Response should be exactly related (e.g. right Content-Length , before looking at the body itself) and you need to compare the Chrome Dev Tools network information with the ones sniffed to be sure that, at the end, a Request shown in Chrome Dev Tools as served by the internal cache means no hit to the server unless the previous (partial) requests didn't cover the all content length and Chrome is correctly required to ask the server for the missing parts.
Have you already try to cleanup your Chrome cache, perform the same test without the PR to see if your behavior is due to the PR i.e. to the usage of ETag ?
Integration: no problem, that's the process. I hope to hear news from Ankit tests setup, too.

Hi Matteo,
I already had x-send file installed and enabled, but am not sure if it is working correctly, as I just tested without the x-sendfile config and the vid was working fine. Perhaps some configuration setup is missing, I will investigate a little more and get back to you.
Cheers

Ankit Agarwal
added a comment - 13/Jun/13 8:15 AM Hi Matteo,
I already had x-send file installed and enabled, but am not sure if it is working correctly, as I just tested without the x-sendfile config and the vid was working fine. Perhaps some configuration setup is missing, I will investigate a little more and get back to you.
Cheers

Setup: please check the Apache error log and the way the module has been packaged/configured for Ubuntu i.e. you should find at least these parameters in your configuration: XSendFile on and XSendFilePath /var/moodledata/int/master, the second being important because it gives the request acceptance scope. Please note that the scope should be built using multiple XSendFilePath lines, at least two in order to map both your WWWROOT and DATAROOT.

Tests: could you talk offline with Dan to search for differences between both setups (when using Chrome with no X-Sendfile in Moodle) to eventually return me any feedback for my investigations?

Matteo Scaramuccia
added a comment - 13/Jun/13 8:36 AM Hi Ankit,
Setup: please check the Apache error log and the way the module has been packaged/configured for Ubuntu i.e. you should find at least these parameters in your configuration: XSendFile on and XSendFilePath /var/moodledata/int/master , the second being important because it gives the request acceptance scope. Please note that the scope should be built using multiple XSendFilePath lines, at least two in order to map both your WWWROOT and DATAROOT .
Tests: could you talk offline with Dan to search for differences between both setups (when using Chrome with no X-Sendfile in Moodle) to eventually return me any feedback for my investigations?
TIA

Hi Dan,
just finished a first test run using all the permutations of the test configurations under Chrome/27.0.1453.110 Windows/Vista SP2 i.e. including incognito windows and running the PR with and without $CFG->xsendfile = 'X-Sendfile'; against the big movie I proposed and your video: all of them successfully.
I'll try to use Chrome on a VM with Windows XP SP3, to start excluding issues with my dev/test environment.

Matteo Scaramuccia
added a comment - 14/Jun/13 8:59 PM Hi Dan,
just finished a first test run using all the permutations of the test configurations under Chrome/27.0.1453.110 Windows/Vista SP2 i.e. including incognito windows and running the PR with and without $CFG->xsendfile = 'X-Sendfile'; against the big movie I proposed and your video: all of them successfully.
I'll try to use Chrome on a VM with Windows XP SP3 , to start excluding issues with my dev/test environment.

Matteo Scaramuccia
added a comment - 15/Jun/13 12:04 PM Same successful result under Chrome/27.0.1453.110 Windows/XP SP3 using all the permutations of the test fixtures combinations above.
Stopping here waiting for some feedback from Ankit.

Hi Andrew,
I guess that the next step should be asking for a new peer review to step into another test session since everything is working on my side even under the worst - as per MDL-39688 - case, Chrome + X-Sendfile.

Wondering if you'd have the time to repeat your tests too. Since MDL-40002 has been integrated, IMHO we should at least add quotes to the current ETag related code, if we are not confident that this patch does the job in helping bandwidth saving in case of Partial Responses (HTTP 206).

Matteo Scaramuccia
added a comment - 17/Jun/13 7:35 PM - edited Hi Andrew,
I guess that the next step should be asking for a new peer review to step into another test session since everything is working on my side even under the worst - as per MDL-39688 - case, Chrome + X-Sendfile .
Wondering if you'd have the time to repeat your tests too. Since MDL-40002 has been integrated, IMHO we should at least add quotes to the current ETag related code, if we are not confident that this patch does the job in helping bandwidth saving in case of Partial Responses ( HTTP 206 ).
Test summary :
Tester
w/o X-Sendfile
w/ X-Sendfile
Andrew/1° run
Andrew/2° run
Ankit/1° run
-
Dan/1° run
-
Matteo/1° Run
Matteo/2° Run

I've been trying to get around to testing this myself too for the past few days but haven't had a chance. The last time that I checked, it was all working correctly both with, and without X-SendFile. I also tried with nginx and X-Accel-Redirect just for completeness.

Andrew Nicols
added a comment - 17/Jun/13 8:04 PM I've been trying to get around to testing this myself too for the past few days but haven't had a chance. The last time that I checked, it was all working correctly both with, and without X-SendFile. I also tried with nginx and X-Accel-Redirect just for completeness.
I'll have another go testing shortly.

I'm not going to have time to test this with x-sendfile in the near future (i'm running stock version of mac apache so need to compile it myself, and a bit pressed for time at the moment), but retesting without it and it seems it must've been cached or something and I am not seeing the problem when I tested. Originally I had not seen that Ankit was testing this with x-sendfile, which has lead to quite some confusion on this issue! Sorry about that.

So, I think this is good to go and was ok in the first place. I can confirm that Ankit didn't have x-sendfile properly configured (just had the module enabled), so that is likely the cause of ankits problem.

Dan Poltawski
added a comment - 18/Jun/13 2:03 AM Hi Matteo,
I'm not going to have time to test this with x-sendfile in the near future (i'm running stock version of mac apache so need to compile it myself, and a bit pressed for time at the moment), but retesting without it and it seems it must've been cached or something and I am not seeing the problem when I tested. Originally I had not seen that Ankit was testing this with x-sendfile, which has lead to quite some confusion on this issue! Sorry about that.
So, I think this is good to go and was ok in the first place. I can confirm that Ankit didn't have x-sendfile properly configured (just had the module enabled), so that is likely the cause of ankits problem.
cheers,
dan

Matteo Scaramuccia
added a comment - 18/Jun/13 5:10 AM @Dan: TNX for your time and efforts spent for this issue too, appreciated !
@Andrew: if you agree, could you close the peer review and send it to integration? TIA.
Matteo

Dan Poltawski FYI it was surprisingly easy to compile mod_xsendfile for Ma. The only issue I encountered is that httpd on OS X (Mountain Lion at least) uses a path to cc which doesn't exist on a normal install. I had to symlink /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.8.xctoolchain to /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/ before I could run the apxs -c command.

Andrew Nicols
added a comment - 18/Jun/13 8:12 AM Dan Poltawski FYI it was surprisingly easy to compile mod_xsendfile for Ma. The only issue I encountered is that httpd on OS X (Mountain Lion at least) uses a path to cc which doesn't exist on a normal install. I had to symlink /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.8.xctoolchain to /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/ before I could run the apxs -c command.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

Dan Poltawski
added a comment - 21/Jun/13 7:19 AM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.
TIA and ciao

Hi Dan,
I'm in difficulty with git vs process.
Your rebase request is part of the process (almost an automatic message) but in this issue things have been already integrated and then reverted (as per the discussion above) so rebasing can do nothing.

Here two reverts (reverting the reverts ) are IMHO required to integrate the issue again:

I think you should ignore the reverts performed before, just imagine they never happened. Just provide clean branches with the changes you want to be applied to every current upstream branch. I must confess I'm a bit disoriented about what is remaining to be integrated here, please clarify.

Also, I'm keeping this on-hold for this week because practically everybody is out @ the AU Moot these days and it seems that the most important part of this issue is to test/verify nothing is borked. And there is already people experimented to test this that surely will perform the task far better than Sam or me (the only two available this week).

Eloy Lafuente (stronk7)
added a comment - 24/Jun/13 5:22 PM Hi Matteo,
I think you should ignore the reverts performed before, just imagine they never happened. Just provide clean branches with the changes you want to be applied to every current upstream branch. I must confess I'm a bit disoriented about what is remaining to be integrated here, please clarify.
Also, I'm keeping this on-hold for this week because practically everybody is out @ the AU Moot these days and it seems that the most important part of this issue is to test/verify nothing is borked. And there is already people experimented to test this that surely will perform the task far better than Sam or me (the only two available this week).
Thanks for the big effort, ciao

If you look at the 2.6 branch you'll see that the two commits have been already applied from my branch and then reverted due to the bad testing result, which has been found to be inconsistent in the next integration round.

Shortly, for 2.3, 2.4, 2.5 you already have the branches (I'll rebase them) while for 2.6 we need to decide if:
a. you revert the two reverts done by Dan w/ no need for rebasing on my side
b. I re-create the two commits reverting Dan's reverts, in a new branch since my proposal for 2.6 has been already applied into master
c. I apply the required fix, maybe not with the same commits history used in the other branches, in a new branch without explicitly reverting Dan's reverts

The goal should IMHO be how to keep the history consistent between branches and I'm wondering if (a) could be the right option.

Matteo Scaramuccia
added a comment - 24/Jun/13 8:37 PM Hi Eloy,
taking 2.5 (m25_ MDL-39832 _Fix_Chrome_Issues_ETag_XSendfile) as an example, here are the two commits to be applied for this issue, MDL-39832 :
https://github.com/scara/moodle/commit/d618e3c07d36272809b84d72df51067f1daa51c7: reverting the work done by Petr in MDL-39688 . It works but it removed the possibility to nicely cache, browser side, the Partial Responses ( HTTP 206 )
https://github.com/scara/moodle/commit/52286ac358bbd81e8291cf0a85054ba63603732e: adding the missing double quotes as it has been identified to be the proper fix for MDL-39688 .
If you look at the 2.6 branch you'll see that the two commits have been already applied from my branch and then reverted due to the bad testing result, which has been found to be inconsistent in the next integration round.
Shortly, for 2.3, 2.4, 2.5 you already have the branches (I'll rebase them) while for 2.6 we need to decide if:
a. you revert the two reverts done by Dan w/ no need for rebasing on my side
b. I re-create the two commits reverting Dan's reverts, in a new branch since my proposal for 2.6 has been already applied into master
c. I apply the required fix, maybe not with the same commits history used in the other branches, in a new branch without explicitly reverting Dan's reverts
The goal should IMHO be how to keep the history consistent between branches and I'm wondering if (a) could be the right option.

I don't mind what has happened until now in any of the branches. If something was sent and later reverted it 100% the same than assuming it never landed in first term.

So, right now, the master branch is into A status and you want to move it to B status, so all we need is a commit performing the changes from A to B.

I really don't get the point about reverting the reverts not what has been partially or totally added modified by other issue. Just add as many commits you need to go successfully to B from current status. if for master you need 2 or more commits, one undoing what was done in another issue and another for the fixes originally related to this issue, np here at all.

But I think we must not be introducing re-re-re-reverts. Just imagine it never happened, really.

If that is alternative (c) in your list above... then go for it, IMO. Erase those 2 commits from your mind (the original and its revert, they sum, together, 0 changes).

Eloy Lafuente (stronk7)
added a comment - 25/Jun/13 12:49 AM lol, I'm getting crazy with this, hahaha.
I don't mind what has happened until now in any of the branches. If something was sent and later reverted it 100% the same than assuming it never landed in first term.
So, right now, the master branch is into A status and you want to move it to B status, so all we need is a commit performing the changes from A to B.
I really don't get the point about reverting the reverts not what has been partially or totally added modified by other issue. Just add as many commits you need to go successfully to B from current status. if for master you need 2 or more commits, one undoing what was done in another issue and another for the fixes originally related to this issue, np here at all.
But I think we must not be introducing re-re-re-reverts. Just imagine it never happened, really.
If that is alternative (c) in your list above... then go for it, IMO. Erase those 2 commits from your mind (the original and its revert, they sum, together, 0 changes).
Not sure if I'm missing something or no... sorry, thanks and ciao

Dan Poltawski
added a comment - 27/Jun/13 3:12 AM (Sorry I didn't see this message earlier Matteo).
Basically, you need to re-apply the commits, which is kind of ugly I agree - and rebase doesn't do the right thing, because it thinks its already been commited.

The quicktime plugin seems screwed on my machine on first load. It seems to work ok one cached. This was unrelated to this patch and also to xsendfile on or off. I think this was the problem I saw when testing earlier.

Dan Poltawski
added a comment - 10/Jul/13 4:36 AM Hurray, passed. Thanks Matteo.
I noticed a few things while testing:
The quicktime plugin seems screwed on my machine on first load. It seems to work ok one cached. This was unrelated to this patch and also to xsendfile on or off. I think this was the problem I saw when testing earlier.
We seem to be serving a YUI file mistakenly using x-sendfile:
(20023)The given path was above the root path: xsendfile: unable to find file: /Users/danp/git/integration/lib/yuilib/3.9.1/build/assets/skins/sam/sprite_icons.png, referer: https://dan.moodle.local/integration/mod/forum/post.php?forum=1
However, I couldn't reproduce this when I tried agian.

Regarding with the xsendfile error, let me share some of my notes: XSendfilePath should be defined e.g. as /Users/danp/git, without the trailing slash and using forward slashes for both *nix and Windows... if everything worked as expected there was no issue with AllowOverride FileInfo too and I've no clear reasons for that error in your log file.

Matteo Scaramuccia
added a comment - 10/Jul/13 5:18 AM
Regarding with the xsendfile error, let me share some of my notes: XSendfilePath should be defined e.g. as /Users/danp/git , without the trailing slash and using forward slashes for both *nix and Windows... if everything worked as expected there was no issue with AllowOverride FileInfo too and I've no clear reasons for that error in your log file.

Good point!
This morning I didn't explicitly focus on being an error related to what should be supposed to be served as a static file, being under dirroot because I'm supposing this behaviour to be by-design: those IMG files are referenced in CSS files as relative to the CSS themselves, the CSS files being served through a PHP helper - I know you know it , just for the record - for managing also the theme versioning and performing other required stuff like rewriting the path of the imgs too (ref.: yui_compo.php) i.e. that is the reason why X-Sendfile is running even on dirroot, raw details: theme/yui_image.php.

Matteo Scaramuccia
added a comment - 10/Jul/13 7:57 AM Good point!
This morning I didn't explicitly focus on being an error related to what should be supposed to be served as a static file, being under dirroot because I'm supposing this behaviour to be by-design: those IMG files are referenced in CSS files as relative to the CSS themselves, the CSS files being served through a PHP helper - I know you know it , just for the record - for managing also the theme versioning and performing other required stuff like rewriting the path of the imgs too (ref.: yui_compo.php ) i.e. that is the reason why X-Sendfile is running even on dirroot , raw details: theme/yui_image.php .

Eloy Lafuente (stronk7)
added a comment - 15/Oct/13 8:43 PM Just to comment that his causes a regression on etag verifications that are being commented @ https://tracker.moodle.org/browse/MDL-38743?focusedCommentId=250106&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-250106
Ciao

We have encountered the problem with ETags recently, so I will write here in case someone find it useful. Basically, about month ago all of a sudden Chrome stopped playing videos on Moodle (presumably after some Chrome or Nginx update, as there were no configuration changes or anything that could affect it). We are using Nginx on the frontends and $CFG->xsendfile settings to make Nginx deliver static data directly from Moodle datastore. The video is normally retrieved using byte-range requests, which Nginx handled properly.

Clearing the cache in Chrome helped and made video work, but only once, the next page refresh or visiting page containing the video the second time resulted in video error. The Nginx we were using is 1.2 on Debian Wheezy. Investigation revealed that disabling ETags header propogation in Nginx for Moodle data static content solved the issue (it used add_header Etag $upstream_http_etag; to propagate ETag coming from Moodle response on background to the static file output it generate), all video started working fine and all partial requests were handled properly.

Further investigation and tests revealed that upgrading Nginx to 1.6 (from wheezy-backports) solves the problem even with the same ETags configuration in Nginx - without any changes to anything, it just started working fine in Chrome with newer version of Nginx. An interesting thing I spotted: if I remove ETag coming from Moodle, Nginx will now generate own "strong" ETag by default, and it will be the same for all parts of the same file, no matter which range is requested, so this confirms the earlier discussion that same ETag is used for all parts of the same file as per RFC. Otherwise, there are no differences in headers, so the issue is somewhat mysterious. I am going to report the bug to Nginx maintainers in Debian.

Ruslan Kabalin
added a comment - 13/Jun/14 6:47 PM We have encountered the problem with ETags recently, so I will write here in case someone find it useful. Basically, about month ago all of a sudden Chrome stopped playing videos on Moodle (presumably after some Chrome or Nginx update, as there were no configuration changes or anything that could affect it). We are using Nginx on the frontends and $CFG->xsendfile settings to make Nginx deliver static data directly from Moodle datastore. The video is normally retrieved using byte-range requests, which Nginx handled properly.
Clearing the cache in Chrome helped and made video work, but only once, the next page refresh or visiting page containing the video the second time resulted in video error. The Nginx we were using is 1.2 on Debian Wheezy. Investigation revealed that disabling ETags header propogation in Nginx for Moodle data static content solved the issue (it used add_header Etag $upstream_http_etag; to propagate ETag coming from Moodle response on background to the static file output it generate), all video started working fine and all partial requests were handled properly.
Further investigation and tests revealed that upgrading Nginx to 1.6 (from wheezy-backports) solves the problem even with the same ETags configuration in Nginx - without any changes to anything, it just started working fine in Chrome with newer version of Nginx. An interesting thing I spotted: if I remove ETag coming from Moodle, Nginx will now generate own "strong" ETag by default, and it will be the same for all parts of the same file, no matter which range is requested, so this confirms the earlier discussion that same ETag is used for all parts of the same file as per RFC. Otherwise, there are no differences in headers, so the issue is somewhat mysterious. I am going to report the bug to Nginx maintainers in Debian.

Ruslan Kabalin
added a comment - 13/Jun/14 8:51 PM Dan Poltawski Yep I have seen them, but the change has been integrated a while ago, and there were no problems a long after till something changed within last two month.