Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-28481

Webservices cause header already sent warnings to fill up error_log

    Details

      Description

      Web services attempt to send headers in instances where they've already been sent. Doesn't hurt functionally, but it does cause 4 warnings to goto the error_log each time a web service is called.

      Moodle should make a headers_sent() check before sending off the headers.

      Replication instructions:
      1. Turn up debugging to include PHP warnings.
      2. Make a webservice call
      3. Notice the warnings that appear in apaches error log.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            aolley Adam Olley added a comment -

            Added link to github with a simple fix for this.

            Show
            aolley Adam Olley added a comment - Added link to github with a simple fix for this.
            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for reporting this and providing a solution.

            Show
            salvetore Michael de Raadt added a comment - Thanks for reporting this and providing a solution.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            I didn't test, but I know about this issue with SOAP server. The patch seems good to me, Mike, you can add to the next sprint and keep it assigned to me.

            Thank you Adam.

            Show
            jerome Jérôme Mouneyrac added a comment - I didn't test, but I know about this issue with SOAP server. The patch seems good to me, Mike, you can add to the next sprint and keep it assigned to me. Thank you Adam.
            Hide
            skodak Petr Skoda added a comment -

            Hello,
            I do not like this change much because it only hides the fact that some code is doing something wrong. I looked at the code and it seems that the headers are sent only once in all cases. Could you get me a stack trace of the first and second sending of headers?

            Petr

            Show
            skodak Petr Skoda added a comment - Hello, I do not like this change much because it only hides the fact that some code is doing something wrong. I looked at the code and it seems that the headers are sent only once in all cases. Could you get me a stack trace of the first and second sending of headers? Petr
            Hide
            aolley Adam Olley added a comment -

            Thanks for the comment Petr, it got me to look in the right place.

            I believe the issue is that the soap server is outputting content before the headers. Naturally as soon as you dump out content, headers are in effect sent.
            In webservice/lib.php's run(), $response = $this->zend_server->handle(); is called before $this->send_headers();. handle() will call dump(), which just echoes out data.

            I've changed my commit to what makes more sense given this. It moves the send_headers call before any output is possibly sent.

            Show
            aolley Adam Olley added a comment - Thanks for the comment Petr, it got me to look in the right place. I believe the issue is that the soap server is outputting content before the headers. Naturally as soon as you dump out content, headers are in effect sent. In webservice/lib.php's run(), $response = $this->zend_server->handle(); is called before $this->send_headers();. handle() will call dump(), which just echoes out data. I've changed my commit to what makes more sense given this. It moves the send_headers call before any output is possibly sent.
            Hide
            skodak Petr Skoda added a comment -

            makes sense, thanks!

            Show
            skodak Petr Skoda added a comment - makes sense, thanks!
            Hide
            skodak Petr Skoda added a comment -

            To integrators: please cherry pick to all 2.x branches

            Show
            skodak Petr Skoda added a comment - To integrators: please cherry pick to all 2.x branches
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            (fixing repository and branch)

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - (fixing repository and branch)
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated, thanks! (backported to 20 and 21 stable)

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks! (backported to 20 and 21 stable)
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Works Great
            Thanks for fixing this Adam and Petr.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Works Great Thanks for fixing this Adam and Petr.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Many thanks for the hard work, this has been sent upstream and is available in all the git and cvs repositories.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work, this has been sent upstream and is available in all the git and cvs repositories.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Oct/11