Moodle
  1. Moodle
  2. MDL-28439

Portfolio Google Docs export does not work with http 1.0 proxies (Squid)

    Details

    • Type: Bug Bug
    • Status: Reopened
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.1
    • Fix Version/s: STABLE backlog
    • Component/s: Portfolio API
    • Labels:
    • Environment:
      Squid proxy server
    • Testing Instructions:
      Hide
      • Only occurs on sites behind Squid proxy.
        1. Enable 'portfolios' and Google Docs portfolio plugin at site level.
        2. Create a post in any forum.
        3. Make sure it has a fair amount of text in it e.g. 20 lines.
        4. Select Export link and select Google Docs (if choice presented)
        5. Success message should be displayed
      Show
      Only occurs on sites behind Squid proxy. 1. Enable 'portfolios' and Google Docs portfolio plugin at site level. 2. Create a post in any forum. 3. Make sure it has a fair amount of text in it e.g. 20 lines. 4. Select Export link and select Google Docs (if choice presented) 5. Success message should be displayed
    • Workaround:
      Hide

      Set Expect header to empty in the request.

      FIX:

      In lib/googleapi.php (ln 345):

      re-work the google docs send_file function to support request failure in these circumstances.

      Suggested code for review (new bit is the If with comment):
      (note code spacing between brackets and braces is wrong - the rest of the file is like this so I followed)

      <pre>
      /**

      • Sends a file object to google documents
        *
      • @param object $file File object
      • @return boolean True on success
        */
        public function send_file($file){
        $this->google_curl->resetopt();
        $this->google_curl->setHeader("Content-Length: ". $file->get_filesize());
        $this->google_curl->setHeader("Content-Type: ". $file->get_mimetype());
        $this->google_curl->setHeader("Slug: ". $file->get_filename());

      $this->google_curl->post(google_docs::DOCUMENTFEED_URL, $file->get_content());

      if($this->google_curl->info['http_code'] === 201)

      { return true; }

      else if($this->google_curl->info['http_code'] === 417){
      //MDL-? Support http 1.0 proxies.
      $this->google_curl->setHeader("Content-Type: ". $file->get_mimetype());
      $this->google_curl->setHeader('Expect:');
      $this->google_curl->post(google_docs::DOCUMENTFEED_URL, $file->get_content());
      if($this->google_curl->info['http_code'] === 201)

      { return true; }

      else

      { return false; }

      //End MDL-?
      }else

      { return false; }

      }
      </pre>

      Show
      Set Expect header to empty in the request. FIX: In lib/googleapi.php (ln 345): re-work the google docs send_file function to support request failure in these circumstances. Suggested code for review (new bit is the If with comment): (note code spacing between brackets and braces is wrong - the rest of the file is like this so I followed) <pre> /** Sends a file object to google documents * @param object $file File object @return boolean True on success */ public function send_file($file){ $this->google_curl->resetopt(); $this->google_curl->setHeader("Content-Length: ". $file->get_filesize()); $this->google_curl->setHeader("Content-Type: ". $file->get_mimetype()); $this->google_curl->setHeader("Slug: ". $file->get_filename()); $this->google_curl->post(google_docs::DOCUMENTFEED_URL, $file->get_content()); if($this->google_curl->info ['http_code'] === 201) { return true; } else if($this->google_curl->info ['http_code'] === 417){ //MDL-? Support http 1.0 proxies. $this->google_curl->setHeader("Content-Type: ". $file->get_mimetype()); $this->google_curl->setHeader('Expect:'); $this->google_curl->post(google_docs::DOCUMENTFEED_URL, $file->get_content()); if($this->google_curl->info ['http_code'] === 201) { return true; } else { return false; } //End MDL-? }else { return false; } } </pre>
    • Affected Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
      git@github.com:jason-platts/moodle.git
    • Pull Master Branch:
      wip-MDL_28439_MASTER
    • Rank:
      18086

      Description

      Exporting text from modules using the Portfolio API to Google Docs often fails.

      This seems to be when the content to be exported is more than just a few lines of text.

      Debugging shows an http 417 error being returned from our institutions proxy server (Squid).

      There seems to be issues with a default header (Expect) php curl uses and Squid.

      See:
      http://serverfault.com/questions/107813/why-does-squid-reject-this-multipart-form-data-post-from-curl
      http://www.squid-cache.org/mail-archive/squid-users/201002/0714.html

      Reproduction instructions (only occurs on sites behind Squid proxy):

      1. Enable 'portfolios' and Google Docs portfolio plugin at site level.
      2. Create a post in any forum.
      3. Make sure it has a fair amount of text in it e.g. 20 lines.
      4. Select Export link and select Google Docs (if choice presented)
      5. Instead of getting success message you get:

      Failed to send your data to the selected system: original error was portfolio_gdocs/sendfailed

      Stack trace:
      line 516 of /lib/portfolio/exporter.php: portfolio_export_exception thrown
      line 237 of /lib/portfolio/exporter.php: call to portfolio_exporter->process_stage_send()
      line 243 of /lib/portfolio/exporter.php: call to portfolio_exporter->process_stage()
      line 243 of /lib/portfolio/exporter.php: call to portfolio_exporter->process_stage()
      line 243 of /lib/portfolio/exporter.php: call to portfolio_exporter->process_stage()
      line 243 of /lib/portfolio/exporter.php: call to portfolio_exporter->process_stage()
      line 270 of /portfolio/add.php: call to portfolio_exporter->process_stage()

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting this and reporting a solution.

          I've put it on our backlog and we'll try to get to it as soon as we can.

          Show
          Michael de Raadt added a comment - Thanks for reporting this and reporting a solution. I've put it on our backlog and we'll try to get to it as soon as we can.
          Hide
          Penny Leach added a comment -

          dan wrote the googledoc plugin

          Show
          Penny Leach added a comment - dan wrote the googledoc plugin
          Hide
          Jason Platts added a comment -

          Here is the fix submitted for integration.

          Hopefully I filled in everything right as it's been a while since I last submitted something...

          I'm not sure whether there is supposed to be a fix for 2.0 also or just master and current stable version?

          Jason

          Show
          Jason Platts added a comment - Here is the fix submitted for integration. Hopefully I filled in everything right as it's been a while since I last submitted something... I'm not sure whether there is supposed to be a fix for 2.0 also or just master and current stable version? Jason
          Hide
          Petr Škoda added a comment -

          Reassigning to Dan for peer review, thanks for the patch.

          Show
          Petr Škoda added a comment - Reassigning to Dan for peer review, thanks for the patch.
          Hide
          Dan Poltawski added a comment -

          Hmm. Thanks for the analysis and patch. I'm intrigued by this as i'm sure i've tested this behind a squid proxy!

          Show
          Dan Poltawski added a comment - Hmm. Thanks for the analysis and patch. I'm intrigued by this as i'm sure i've tested this behind a squid proxy!
          Hide
          Dan Poltawski added a comment -

          Hi Jason,

          My first thoughts on this are that it would actually probably be better placed to work correctly from the start - and also fixed in the base curl class (so that any other parts of Moodle which send big requests behave correctly with a proxy).

          It'd be good if we could come up with a simple example to demonstrate this problem

          Show
          Dan Poltawski added a comment - Hi Jason, My first thoughts on this are that it would actually probably be better placed to work correctly from the start - and also fixed in the base curl class (so that any other parts of Moodle which send big requests behave correctly with a proxy). It'd be good if we could come up with a simple example to demonstrate this problem
          Hide
          Jason Platts added a comment -

          Hi Dan,

          I agree with putting a fix in the curl class in file lib, that would certainly make more sense as this issue will also affect any other bits of Moodle that call it and make post requests.

          I'm not quite sure how one would go about a simple demonstration as presumably this is affected by the version of squid that is used etc.
          (For info we are using squid/2.6.STABLE21)

          I don't think it's a configuration issue as the info I found on squid suggests it just doesn't support this, so this might make it easier to test. http://www.squid-cache.org/mail-archive/squid-users/201002/0714.html

          I can give access to a external facing development site here to verify fixes (though there wouldn't be external file access, so I would have to apply any changes etc at this end).

          Show
          Jason Platts added a comment - Hi Dan, I agree with putting a fix in the curl class in file lib, that would certainly make more sense as this issue will also affect any other bits of Moodle that call it and make post requests. I'm not quite sure how one would go about a simple demonstration as presumably this is affected by the version of squid that is used etc. (For info we are using squid/2.6.STABLE21) I don't think it's a configuration issue as the info I found on squid suggests it just doesn't support this, so this might make it easier to test. http://www.squid-cache.org/mail-archive/squid-users/201002/0714.html I can give access to a external facing development site here to verify fixes (though there wouldn't be external file access, so I would have to apply any changes etc at this end).
          Hide
          Jason Platts added a comment -

          I just had a go a adding this to the curl lib rather than google lib.

          https://github.com/jason-platts/moodle/commit/dc92fffaebc9b002a99952bd8e1c08c0dc15d56e

          Show
          Jason Platts added a comment - I just had a go a adding this to the curl lib rather than google lib. https://github.com/jason-platts/moodle/commit/dc92fffaebc9b002a99952bd8e1c08c0dc15d56e
          Hide
          Dan Poltawski added a comment -

          Hi Jason,

          I don't think that is the right way to solve this problem - Looking briefly at the php curl manual it looks like we can set CURLOPT_HTTP_VERSION to CURL_HTTP_VERSION_1_0 to force http 1.0 which I believe will solve this issue?

          Show
          Dan Poltawski added a comment - Hi Jason, I don't think that is the right way to solve this problem - Looking briefly at the php curl manual it looks like we can set CURLOPT_HTTP_VERSION to CURL_HTTP_VERSION_1_0 to force http 1.0 which I believe will solve this issue?
          Hide
          Jason Platts added a comment -

          You're right Dan, setting this option does remove the expect header that was causing the issue.

          Is there a sensible way to implement this?

          I'd be a bit concerned that setting the version by default might cause other issues, so could it be controlled by a new admin setting?

          Presumably this http version option would then be set in the curl constructor method (also with other proxy stuff)?

          I'm away on holiday for a week now so am a bit short on time, but if you want I can code up a patch for this the week after as it should be quite straightforward.

          Show
          Jason Platts added a comment - You're right Dan, setting this option does remove the expect header that was causing the issue. Is there a sensible way to implement this? I'd be a bit concerned that setting the version by default might cause other issues, so could it be controlled by a new admin setting? Presumably this http version option would then be set in the curl constructor method (also with other proxy stuff)? I'm away on holiday for a week now so am a bit short on time, but if you want I can code up a patch for this the week after as it should be quite straightforward.
          Hide
          Jason Platts added a comment -

          I've had a go at fixing this with a config setting.

          https://github.com/jason-platts/moodle/commit/b37b9beaf300d75be6e2a22018c79c65279b7f69
          (note this doesn't have any version.php updates as I'm assuming this would need to occur at the time of integration)

          Unfortunately I'm now unable to test this as our IT department have managed to resolve the issue at the proxy end. I'm not sure others would be able to do this as it wasn't through standard configuration of Squid - so this fix might still be useful.

          Show
          Jason Platts added a comment - I've had a go at fixing this with a config setting. https://github.com/jason-platts/moodle/commit/b37b9beaf300d75be6e2a22018c79c65279b7f69 (note this doesn't have any version.php updates as I'm assuming this would need to occur at the time of integration) Unfortunately I'm now unable to test this as our IT department have managed to resolve the issue at the proxy end. I'm not sure others would be able to do this as it wasn't through standard configuration of Squid - so this fix might still be useful.
          Hide
          Loughborough College added a comment -

          had similar errors with my installation, even through we got the site to run without going through a proxy. had to alter the lib\googleapi.php file

          line: 245 - change the google url to https

          then it started to work fine, don't know if this is connected, but was the only similar bug on the tracker

          Show
          Loughborough College added a comment - had similar errors with my installation, even through we got the site to run without going through a proxy. had to alter the lib\googleapi.php file line: 245 - change the google url to https then it started to work fine, don't know if this is connected, but was the only similar bug on the tracker
          Hide
          Jason Platts added a comment -

          Hi Loughborough,

          This is probably because of MDL-26845.

          Which I raised ages ago and then was ignored. Now Google exporting doesn't work any more.

          Show
          Jason Platts added a comment - Hi Loughborough, This is probably because of MDL-26845 . Which I raised ages ago and then was ignored. Now Google exporting doesn't work any more.
          Hide
          Dan Poltawski added a comment -

          Hi Loughborough College,

          No that was an unrelated issue addressed in: MDL-29661

          Show
          Dan Poltawski added a comment - Hi Loughborough College, No that was an unrelated issue addressed in: MDL-29661
          Hide
          Dan Poltawski added a comment -

          I have a feeling that this issue can be solved by the same solution as MDL-38170

          Show
          Dan Poltawski added a comment - I have a feeling that this issue can be solved by the same solution as MDL-38170

            People

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

              Dates

              • Created:
                Updated: