Moodle
  1. Moodle
  2. MDL-39832

Send strong ETags for partial content

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.3.9, 2.4.6, 2.5.2, 2.6
    • Component/s: Files API
    • Labels:
    • Testing Instructions:
      Hide

      Apply the same testing instruction described in MDL-39688 except for:

      1. 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.
      2. 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
      3. 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.:
      4. 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
      5. Before launching the URL above, please open Fiddler and press F12 to start "capturing" the network traffic
      6. Open the browser, Chrome, press F12 and select the Network tab
      7. 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)
      8. Cleanup Fiddler capturing
      9. Close Chrome and then reopen it, press F12, select the Network tab, paste the URL, press ENTER
      10. 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
      Show
      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
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
      m24_MDL-39832_Fix_Chrome_Issues_ETag_XSendfile_2ndRound
    • Pull 2.5 Branch:
      m25_MDL-39832_Fix_Chrome_Issues_ETag_XSendfile_2ndRound
    • Pull Master Branch:
      m26_MDL-39832_Fix_Chrome_Issues_ETag_XSendfile_2ndRound
    • Rank:
      50574

      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.

        Issue Links

          Activity

          Hide
          Petr Škoda added a comment -

          Please test...

          Show
          Petr Škoda added a comment - Please test...
          Hide
          Andrew Nicols added a comment -

          I'll look at this for peer review tomorrow. Had a quick glance now and everything looks good.

          Show
          Andrew Nicols added a comment - I'll look at this for peer review tomorrow. Had a quick glance now and everything looks good.
          Hide
          Andrew Nicols added a comment -

          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.

          Show
          Andrew Nicols added a comment - 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.
          Hide
          Petr Škoda added a comment -

          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?

          Show
          Petr Škoda added a comment - 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?
          Hide
          Andrew Nicols added a comment -

          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

          Show
          Andrew Nicols added a comment - 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
          Hide
          Andrew Nicols added a comment -

          Or was that a request for me to test and write instructions?

          Show
          Andrew Nicols added a comment - Or was that a request for me to test and write instructions?
          Hide
          Petr Škoda added a comment -

          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!

          Show
          Petr Škoda added a comment - 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!
          Hide
          Petr Škoda added a comment -

          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.

          Show
          Petr Škoda added a comment - 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.
          Hide
          Matteo Scaramuccia added a comment -

          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

          Show
          Matteo Scaramuccia added a comment - 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
          Hide
          Petr Škoda added a comment -

          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!

          Show
          Petr Škoda added a comment - 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!
          Hide
          Andrew Nicols added a comment -

          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.

          Show
          Andrew Nicols added a comment - 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.
          Hide
          Andrew Nicols added a comment -

          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.

          Show
          Andrew Nicols added a comment - 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.
          Hide
          Andrew Nicols added a comment -

          In fact, it's fine with nginx proxying to an apache server right up until you enable the X-Sendfile setting.

          Show
          Andrew Nicols added a comment - In fact, it's fine with nginx proxying to an apache server right up until you enable the X-Sendfile setting.
          Hide
          Andrew Nicols added a comment - - 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.

          Show
          Andrew Nicols added a comment - - 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.
          Hide
          Andrew Nicols added a comment -

          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 Škoda, what browsers + content are you testing this with?

          Show
          Andrew Nicols added a comment - 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 Škoda , what browsers + content are you testing this with?
          Hide
          Matteo Scaramuccia added a comment - - 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.

          Show
          Matteo Scaramuccia added a comment - - 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.
          Hide
          Petr Škoda added a comment - - 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?

          Show
          Petr Škoda added a comment - - 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?
          Hide
          Matteo Scaramuccia added a comment -

          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.

          Show
          Matteo Scaramuccia added a comment - 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.
          Hide
          Andrew Nicols added a comment -

          I can try and look into this further in about two weeks time.

          Show
          Andrew Nicols added a comment - I can try and look into this further in about two weeks time.
          Hide
          Petr Škoda added a comment -

          Here is my patch, anybody is free to work on this issue: https://github.com/skodak/moodle/compare/wip_MDL-39832_m26_etagranges

          Thanks everybody!

          Show
          Petr Škoda added a comment - Here is my patch, anybody is free to work on this issue: https://github.com/skodak/moodle/compare/wip_MDL-39832_m26_etagranges Thanks everybody!
          Hide
          Matteo Scaramuccia added a comment - - 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

          Show
          Matteo Scaramuccia added a comment - - 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
          Hide
          Andrew Nicols added a comment -

          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).

          Show
          Andrew Nicols added a comment - 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).
          Hide
          Matteo Scaramuccia added a comment -

          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.

          Show
          Matteo Scaramuccia added a comment - 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.
          Hide
          Petr Škoda added a comment -

          Wow, is it really only the missing double quotes? If yes should we fix the rest of etags? Are there any other headers that require double quotes?

          Show
          Petr Škoda added a comment - Wow, is it really only the missing double quotes? If yes should we fix the rest of etags? Are there any other headers that require double quotes?
          Hide
          Matteo Scaramuccia added a comment - - 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.

          Show
          Matteo Scaramuccia added a comment - - 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.
          Hide
          Andrew Nicols added a comment -

          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

          Show
          Andrew Nicols added a comment - 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
          Hide
          Petr Škoda added a comment -

          +1 for the backport

          Show
          Petr Škoda added a comment - +1 for the backport
          Hide
          Matteo Scaramuccia added a comment -

          OK Andrew&Petr, will do one of my evening (CEST).

          Show
          Matteo Scaramuccia added a comment - OK Andrew&Petr, will do one of my evening (CEST).
          Hide
          Matteo Scaramuccia added a comment -

          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.

          Show
          Matteo Scaramuccia added a comment - 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.
          Hide
          Petr Škoda added a comment -

          submitting, +1 for separate issue for the remaining quotes, thanks!!

          Show
          Petr Škoda added a comment - submitting, +1 for separate issue for the remaining quotes, thanks!!
          Hide
          Dan Poltawski added a comment -

          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

          Show
          Dan Poltawski added a comment - 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
          Hide
          Matteo Scaramuccia added a comment -

          Rebased.

          Show
          Matteo Scaramuccia added a comment - Rebased.
          Hide
          Dan Poltawski added a comment -

          Great detective work Matteo!

          I've integrated this to master, 25, 24 and 23 - thanks!

          Show
          Dan Poltawski added a comment - Great detective work Matteo! I've integrated this to master, 25, 24 and 23 - thanks!
          Hide
          Ankit Agarwal added a comment - - 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.

          Show
          Ankit Agarwal added a comment - - 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.
          Hide
          Dan Poltawski added a comment -

          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

          Show
          Dan Poltawski added a comment - 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
          Hide
          Matteo Scaramuccia added a comment -

          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:

          1. Before starting with tests, did you cleanup Chrome cache? It's unfortunately a missing point in the testing instructions
          2. Could you share the movie used for your tests?
          3. Could you confirm that tests has been performed using the pluginfile.php URL?
          4. 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?
          5. Did you use X-Sendfile configuration or just "plain" PHP module connection?
          6. 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 )
          Show
          Matteo Scaramuccia added a comment - 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 )
          Hide
          Dan Poltawski added a comment -

          > 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.

          Show
          Dan Poltawski added a comment - > 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.
          Hide
          Dan Poltawski added a comment -

          (i'm happy to sniff the traffic to work out whats going on, but I am not I can)

          Show
          Dan Poltawski added a comment - (i'm happy to sniff the traffic to work out whats going on, but I am not I can)
          Hide
          Ankit Agarwal added a comment - - edited

          Hi Matteo,
          Not sure am being of much help here, but anyway:-

          1. No, I didn't . But I just retried with clean cache, same issue.
          2. I used the one you mentioned http://samples.mplayerhq.hu/MPEG-4/vorbis-in-mp4/mi2_vorbis51.mp4 and the one here http://www.quirksmode.org/html5/tests/video.html
          3. Yes
          4. Yes
          5. Yes (I did see x send file tag in header)
          6. Am on linux, so used only the dev tools. I couldn't think of any other reason, why the contents will be empty. Anyways here are the headers, see if it helps
            GET /int/master/moodle/pluginfile.php/418/mod_resource/content/1/big_buck_bunny.mp4 HTTP/1.1
            Host: ankit.moodle.local
            Connection: keep-alive
            Cache-Control: max-age=0
            Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
            User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.110 Safari/537.36
            Referer: http://ankit.moodle.local/int/master/moodle/login/index.php
            Accept-Encoding: gzip,deflate,sdch
            Accept-Language: en-GB,en-US;q=0.8,en;q=0.6
            Cookie: MoodleSession=nk41ksu6f8qrggotnbnnbu3pd4; MOODLEID1_=%25DB%25CF%2592%25FBU%25F3%258C; MOODLEID1_=%2598%2529%25B2%25B0%25ED2%252B0
            HTTP/1.1 200 OK
            Date: Thu, 13 Jun 2013 02:17:29 GMT
            Server: Apache/2.2.22 (Ubuntu)
            X-Powered-By: PHP/5.3.10-1ubuntu3.6
            Expires: Fri, 14 Jun 2013 02:17:29 GMT
            Cache-Control: max-age=86400
            Pragma: 
            Content-Disposition: inline; filename="big_buck_bunny.mp4"
            Last-Modified: Wed, 12 Jun 2013 07:33:28 GMT
            Etag: "0181d7880acf0c443a57cd2510a5b81447fa577e"
            Accept-Ranges: bytes
            X-Sendfile: /var/moodledata/int/master/filedir/01/81/0181d7880acf0c443a57cd2510a5b81447fa577e
            Content-Length: 0
            Keep-Alive: timeout=5, max=98
            Connection: Keep-Alive
            Content-Type: video/mp4
            
            
            GET /int/master/moodle/pluginfile.php/418/mod_resource/content/1/big_buck_bunny.mp4 HTTP/1.1
            Host: ankit.moodle.local
            Connection: keep-alive
            Accept-Encoding: identity;q=1, *;q=0
            User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.110 Safari/537.36
            Accept: */*
            Referer: http://ankit.moodle.local/int/master/moodle/pluginfile.php/418/mod_resource/content/1/big_buck_bunny.mp4
            Accept-Language: en-GB,en-US;q=0.8,en;q=0.6
            Cookie: MoodleSession=nk41ksu6f8qrggotnbnnbu3pd4; MOODLEID1_=%25DB%25CF%2592%25FBU%25F3%258C; MOODLEID1_=%2598%2529%25B2%25B0%25ED2%252B0
            Range: bytes=0-
            
            HTTP/1.1 200 OK
            Date: Thu, 13 Jun 2013 02:17:29 GMT
            Server: Apache/2.2.22 (Ubuntu)
            X-Powered-By: PHP/5.3.10-1ubuntu3.6
            Expires: Fri, 14 Jun 2013 02:17:30 GMT
            Cache-Control: max-age=86400
            Pragma: 
            Content-Disposition: inline; filename="big_buck_bunny.mp4"
            Last-Modified: Wed, 12 Jun 2013 07:33:28 GMT
            Etag: "0181d7880acf0c443a57cd2510a5b81447fa577e"
            Accept-Ranges: bytes
            X-Sendfile: /var/moodledata/int/master/filedir/01/81/0181d7880acf0c443a57cd2510a5b81447fa577e
            Content-Length: 0
            Keep-Alive: timeout=5, max=97
            Connection: Keep-Alive
            Content-Type: video/mp4
            
            

            Thanks

          Show
          Ankit Agarwal added a comment - - edited Hi Matteo, Not sure am being of much help here, but anyway:- No, I didn't . But I just retried with clean cache, same issue. I used the one you mentioned http://samples.mplayerhq.hu/MPEG-4/vorbis-in-mp4/mi2_vorbis51.mp4 and the one here http://www.quirksmode.org/html5/tests/video.html Yes Yes Yes (I did see x send file tag in header) Am on linux, so used only the dev tools. I couldn't think of any other reason, why the contents will be empty. Anyways here are the headers, see if it helps GET / int /master/moodle/pluginfile.php/418/mod_resource/content/1/big_buck_bunny.mp4 HTTP/1.1 Host: ankit.moodle.local Connection: keep-alive Cache-Control: max-age=0 Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8 User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.110 Safari/537.36 Referer: http: //ankit.moodle.local/ int /master/moodle/login/index.php Accept-Encoding: gzip,deflate,sdch Accept-Language: en-GB,en-US;q=0.8,en;q=0.6 Cookie: MoodleSession=nk41ksu6f8qrggotnbnnbu3pd4; MOODLEID1_=%25DB%25CF%2592%25FBU%25F3%258C; MOODLEID1_=%2598%2529%25B2%25B0%25ED2%252B0 HTTP/1.1 200 OK Date: Thu, 13 Jun 2013 02:17:29 GMT Server: Apache/2.2.22 (Ubuntu) X-Powered-By: PHP/5.3.10-1ubuntu3.6 Expires: Fri, 14 Jun 2013 02:17:29 GMT Cache-Control: max-age=86400 Pragma: Content-Disposition: inline; filename= "big_buck_bunny.mp4" Last-Modified: Wed, 12 Jun 2013 07:33:28 GMT Etag: "0181d7880acf0c443a57cd2510a5b81447fa577e" Accept-Ranges: bytes X-Sendfile: / var /moodledata/ int /master/filedir/01/81/0181d7880acf0c443a57cd2510a5b81447fa577e Content-Length: 0 Keep-Alive: timeout=5, max=98 Connection: Keep-Alive Content-Type: video/mp4 GET / int /master/moodle/pluginfile.php/418/mod_resource/content/1/big_buck_bunny.mp4 HTTP/1.1 Host: ankit.moodle.local Connection: keep-alive Accept-Encoding: identity;q=1, *;q=0 User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.110 Safari/537.36 Accept: */* Referer: http: //ankit.moodle.local/ int /master/moodle/pluginfile.php/418/mod_resource/content/1/big_buck_bunny.mp4 Accept-Language: en-GB,en-US;q=0.8,en;q=0.6 Cookie: MoodleSession=nk41ksu6f8qrggotnbnnbu3pd4; MOODLEID1_=%25DB%25CF%2592%25FBU%25F3%258C; MOODLEID1_=%2598%2529%25B2%25B0%25ED2%252B0 Range: bytes=0- HTTP/1.1 200 OK Date: Thu, 13 Jun 2013 02:17:29 GMT Server: Apache/2.2.22 (Ubuntu) X-Powered-By: PHP/5.3.10-1ubuntu3.6 Expires: Fri, 14 Jun 2013 02:17:30 GMT Cache-Control: max-age=86400 Pragma: Content-Disposition: inline; filename= "big_buck_bunny.mp4" Last-Modified: Wed, 12 Jun 2013 07:33:28 GMT Etag: "0181d7880acf0c443a57cd2510a5b81447fa577e" Accept-Ranges: bytes X-Sendfile: / var /moodledata/ int /master/filedir/01/81/0181d7880acf0c443a57cd2510a5b81447fa577e Content-Length: 0 Keep-Alive: timeout=5, max=97 Connection: Keep-Alive Content-Type: video/mp4 Thanks
          Hide
          Matteo Scaramuccia added a comment -

          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

          Show
          Matteo Scaramuccia added a comment - 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
          Hide
          Dan Poltawski added a comment -

          Hi Matteo,

          Yep I know how to sniff the traffic, but I don't know what you want us to look for

          Show
          Dan Poltawski added a comment - Hi Matteo, Yep I know how to sniff the traffic, but I don't know what you want us to look for
          Hide
          Dan Poltawski added a comment -

          Anyway, i'm going to need to revert this, because from what I can see from our testing, it is continuing to cause a problem

          Show
          Dan Poltawski added a comment - Anyway, i'm going to need to revert this, because from what I can see from our testing, it is continuing to cause a problem
          Hide
          Dan Poltawski added a comment -

          I've reverted this change, sorry we have had to do that.

          Show
          Dan Poltawski added a comment - I've reverted this change, sorry we have had to do that.
          Hide
          Matteo Scaramuccia added a comment -

          Hi Dan,

          1. 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?
          2. Integration: no problem, that's the process. I hope to hear news from Ankit tests setup, too.
          Show
          Matteo Scaramuccia added a comment - 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.
          Hide
          Ankit Agarwal added a comment -

          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

          Show
          Ankit Agarwal added a comment - 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
          Hide
          Matteo Scaramuccia added a comment -

          Hi Ankit,

          1. 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.
          2. 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

          Show
          Matteo Scaramuccia added a comment - 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
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Matteo Scaramuccia added a comment -

          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.

          Show
          Matteo Scaramuccia added a comment - 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.
          Hide
          Matteo Scaramuccia added a comment -

          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.

          Show
          Matteo Scaramuccia added a comment - 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.
          Hide
          Matteo Scaramuccia added a comment - - 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
          Show
          Matteo Scaramuccia added a comment - - 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
          Hide
          Andrew Nicols added a comment -

          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.

          Show
          Andrew Nicols added a comment - 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.
          Hide
          Andrew Nicols added a comment -

          Yup - you can add a double-tick for me too. All works as expected wit Chrome on Mac OS. Tested with one commit reverted just to prove the issue too.

          Show
          Andrew Nicols added a comment - Yup - you can add a double-tick for me too. All works as expected wit Chrome on Mac OS. Tested with one commit reverted just to prove the issue too.
          Hide
          Matteo Scaramuccia added a comment -

          TNX Andrew!
          It seems we need to wait for replies from Dan/Ankit to identify the next step.

          Show
          Matteo Scaramuccia added a comment - TNX Andrew! It seems we need to wait for replies from Dan/Ankit to identify the next step.
          Hide
          Dan Poltawski added a comment -

          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

          Show
          Dan Poltawski added a comment - 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
          Hide
          Matteo Scaramuccia added a comment -

          @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

          Show
          Matteo Scaramuccia added a comment - @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
          Hide
          Dan Poltawski added a comment -

          Thank you for your time!

          Show
          Dan Poltawski added a comment - Thank you for your time!
          Hide
          Andrew Nicols added a comment -

          Sending back for integration after much (more) testing

          Show
          Andrew Nicols added a comment - Sending back for integration after much (more) testing
          Hide
          Andrew Nicols added a comment -

          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.

          Show
          Andrew Nicols added a comment - 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.
          Hide
          Dan Poltawski added a comment -

          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

          Show
          Dan Poltawski added a comment - 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
          Hide
          Matteo Scaramuccia added a comment -

          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:

          commit a151eb403eae727634050f58f389183eefca4630
          Author: Dan Poltawski <dan@moodle.com>
          Date:   Thu Jun 13 13:56:11 2013 +0800
          
              Revert "MDL-39832 Files: Revert MDL-39688 commit, f111746140fe80a821ae69e3bf518d950bf188a1"
          
              This reverts commit 3960c00e3cf129c60b01fecdfe2851ba9ef7ba59.
          
          commit db74804ff1245df4bdc0c8de187cfbe43a3ec0d7
          Author: Dan Poltawski <dan@moodle.com>
          Date:   Thu Jun 13 13:56:09 2013 +0800
          
              Revert "MDL-39832 Files: Fixed ETag format according with RFC2616."
          
              This reverts commit 9ab37ae3e07f54bc08644f8462d94bed68ff6c9b.
          
          commit 0a006517e5620f32e8a22fddcb43103992e5b134
          Merge: f4c824c 9ab37ae
          Author: Dan Poltawski <dan@moodle.com>
          Date:   Mon Jun 10 15:42:45 2013 +0800
          
              Merge branch 'm26_MDL-39832_Fix_Chrome_Issues_ETag_XSendfile' of https://github.com/scara/moodle
          
          commit 9ab37ae3e07f54bc08644f8462d94bed68ff6c9b
          Author: Matteo Scaramuccia <moodle@matteoscaramuccia.com>
          Date:   Sat Jun 1 12:34:40 2013 +0200
          
              MDL-39832 Files: Fixed ETag format according with RFC2616.
          
              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
          
          commit 3960c00e3cf129c60b01fecdfe2851ba9ef7ba59
          Author: Matteo Scaramuccia <moodle@matteoscaramuccia.com>
          Date:   Sat Jun 1 12:21:29 2013 +0200
          
              MDL-39832 Files: Revert MDL-39688 commit, f111746140fe80a821ae69e3bf518d950bf188a1
          
              MDL-39688 has been successful in providing a solution for the issue but further
              investigations done in MDL-39832 have got the evidence that the given solution
              is more likely a workaround.
          

          I doubt if I need to push almost a new branch (e.g. m26_MDL-39832_Fix_Chrome_Issues_ETag_XSendfile_Reverting), to apply the correct reverting: any advice will be greatly appreciated.

          TIA,
          Matteo

          Show
          Matteo Scaramuccia added a comment - 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: commit a151eb403eae727634050f58f389183eefca4630 Author: Dan Poltawski <dan@moodle.com> Date: Thu Jun 13 13:56:11 2013 +0800 Revert "MDL-39832 Files: Revert MDL-39688 commit, f111746140fe80a821ae69e3bf518d950bf188a1" This reverts commit 3960c00e3cf129c60b01fecdfe2851ba9ef7ba59. commit db74804ff1245df4bdc0c8de187cfbe43a3ec0d7 Author: Dan Poltawski <dan@moodle.com> Date: Thu Jun 13 13:56:09 2013 +0800 Revert "MDL-39832 Files: Fixed ETag format according with RFC2616." This reverts commit 9ab37ae3e07f54bc08644f8462d94bed68ff6c9b. commit 0a006517e5620f32e8a22fddcb43103992e5b134 Merge: f4c824c 9ab37ae Author: Dan Poltawski <dan@moodle.com> Date: Mon Jun 10 15:42:45 2013 +0800 Merge branch 'm26_MDL-39832_Fix_Chrome_Issues_ETag_XSendfile' of https://github.com/scara/moodle commit 9ab37ae3e07f54bc08644f8462d94bed68ff6c9b Author: Matteo Scaramuccia <moodle@matteoscaramuccia.com> Date: Sat Jun 1 12:34:40 2013 +0200 MDL-39832 Files: Fixed ETag format according with RFC2616. 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 commit 3960c00e3cf129c60b01fecdfe2851ba9ef7ba59 Author: Matteo Scaramuccia <moodle@matteoscaramuccia.com> Date: Sat Jun 1 12:21:29 2013 +0200 MDL-39832 Files: Revert MDL-39688 commit, f111746140fe80a821ae69e3bf518d950bf188a1 MDL-39688 has been successful in providing a solution for the issue but further investigations done in MDL-39832 have got the evidence that the given solution is more likely a workaround. I doubt if I need to push almost a new branch (e.g. m26_ MDL-39832 _Fix_Chrome_Issues_ETag_XSendfile_Reverting ), to apply the correct reverting: any advice will be greatly appreciated. TIA, Matteo
          Hide
          Eloy Lafuente (stronk7) added a comment -

          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

          Show
          Eloy Lafuente (stronk7) added a comment - 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
          Hide
          Matteo Scaramuccia added a comment -

          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:

          1. 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)
          2. 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.

          Show
          Matteo Scaramuccia added a comment - 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.
          Show
          Matteo Scaramuccia added a comment - - edited Hi Eloy, back again, I've made a mistake in my previous comment; all the branches have been already "fixed" and "reverted", e.g.: https://github.com/moodle/moodle/commits/master/lib/filelib.php https://github.com/moodle/moodle/commits/MOODLE_25_STABLE/lib/filelib.php https://github.com/moodle/moodle/commits/MOODLE_24_STABLE/lib/filelib.php https://github.com/moodle/moodle/commits/MOODLE_23_STABLE/lib/filelib.php
          Hide
          Eloy Lafuente (stronk7) added a comment -

          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

          Show
          Eloy Lafuente (stronk7) added a comment - 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
          Hide
          Matteo Scaramuccia added a comment -

          OK,
          will do all the required changes to move from now (A) to what required (B).

          Matteo

          Show
          Matteo Scaramuccia added a comment - OK, will do all the required changes to move from now (A) to what required (B). Matteo
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Many thanks Matteo!

          I'm sending this back to development, so will be picked next week (or when available, no pressure!).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Many thanks Matteo! I'm sending this back to development, so will be picked next week (or when available, no pressure!). Ciao
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Matteo Scaramuccia added a comment -
          Show
          Matteo Scaramuccia added a comment - For the/my record, interesting reading: https://www.kernel.org/pub/software/scm/git/docs/howto/revert-a-faulty-merge.txt
          Hide
          Matteo Scaramuccia added a comment -

          Hi Andrew,
          would you be so kind and patient to perform another review and propose the fix back again to the integration review?

          TIA!
          Matteo

          Show
          Matteo Scaramuccia added a comment - Hi Andrew, would you be so kind and patient to perform another review and propose the fix back again to the integration review? TIA! Matteo
          Hide
          Dan Poltawski added a comment -

          (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.

          Show
          Dan Poltawski added a comment - (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.
          Hide
          Matteo Scaramuccia added a comment -

          NP Dan!
          Already done and learned something new which is fine .

          Show
          Matteo Scaramuccia added a comment - NP Dan! Already done and learned something new which is fine .
          Hide
          Matteo Scaramuccia added a comment -

          Rebased, just in case it will be integrated the next week .

          Show
          Matteo Scaramuccia added a comment - Rebased, just in case it will be integrated the next week .
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Thanks Matteo, those commits look pretty clear now!

          Show
          Eloy Lafuente (stronk7) added a comment - Thanks Matteo, those commits look pretty clear now!
          Hide
          Andrew Nicols added a comment -

          Peer review looks good to me still (no change). Submitting for integration now.

          Show
          Andrew Nicols added a comment - Peer review looks good to me still (no change). Submitting for integration now.
          Hide
          Matteo Scaramuccia added a comment -

          Rebased.

          Show
          Matteo Scaramuccia added a comment - Rebased.
          Hide
          Marina Glancy added a comment -

          Thanks guys, this has been integrated in 2.3, 2.4, 2.5 and master.
          Dan volunteered to be a tester

          Show
          Marina Glancy added a comment - Thanks guys, this has been integrated in 2.3, 2.4, 2.5 and master. Dan volunteered to be a tester
          Hide
          Dan Poltawski added a comment -

          Hurray, passed. Thanks Matteo.

          I noticed a few things while testing:

          1. 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.
          2. 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.

          Show
          Dan Poltawski added a comment - 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.
          Hide
          Dan Poltawski added a comment -

          ps. Thanks Andrew for the xsendfile note. It was suprisingly easy and that symlink tip helped me avoid messing around to fix it.

          Show
          Dan Poltawski added a comment - ps. Thanks Andrew for the xsendfile note. It was suprisingly easy and that symlink tip helped me avoid messing around to fix it.
          Hide
          Matteo Scaramuccia added a comment -

          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.

          Show
          Matteo Scaramuccia added a comment - 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.
          Hide
          Dan Poltawski added a comment -

          Surely not - are we not restricting xsendfile served files to moodledata? I didn't think it was a solution for serving dirroot files.

          Show
          Dan Poltawski added a comment - Surely not - are we not restricting xsendfile served files to moodledata? I didn't think it was a solution for serving dirroot files.
          Hide
          Matteo Scaramuccia added a comment -

          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.

          Show
          Matteo Scaramuccia added a comment - 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 .
          Hide
          Damyon Wiese added a comment -

          a single bug fix
          a drop in a waterfall
          hear the mighty roar

          Thanks for your contribution!

          Show
          Damyon Wiese added a comment - a single bug fix a drop in a waterfall hear the mighty roar Thanks for your contribution!
          Hide
          Eloy Lafuente (stronk7) added a comment -
          Show
          Eloy Lafuente (stronk7) added a comment - 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

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: