Moodle
  1. Moodle
  2. MDL-29712

The REST web service creates invalid XML by encoding HTML entities

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.6, 2.1, 2.2
    • Fix Version/s: 2.0.7, 2.1.4, 2.2.1
    • Component/s: Web Services
    • Labels:
      None
    • Testing Instructions:
      Hide

      Edit the user linked to the token and set the first name as Noël. Enable mobile web service and also the REST protocol.

      Use the Javascript-REST demo client (https://github.com/moodlehq/sample-ws-clients/tree/master/JAVASCRIPT-REST) and change these lines:

      var functionname = 'moodle_webservice_get_siteinfo';
      
      // ....
      
      var data = {
          wstoken: token,
          wsfunction: functionname,
          moodlewsrestformat: 'xml'
      }
      
      var response = $.ajax(
                            {   type: 'POST',
                                data: data,
                                dataType: 'text',
                                url: serverurl,
                                success: function(data) { 
                                     console.log('Parsed data');
                                     data = $.parseXML(data); 
                                     console.log(data);
                                }
                             }
                            );
      

      In firebug: no JS errors (mainly an xml parsing error) should appeared. You should be able to see the resulting XML in your JS logs.

      For 2.0 testing you'll have to create your own service and use moodle_user_get_users_by_id ws function. You'll also have to change these lines:

      var functionname = 'moodle_user_get_users_by_id';
      
      // ...
      
      var users = ['2'];
      
      var data = {
          wstoken: token,
          wsfunction: functionname,
                      userids: users
      }
      
      var response = $.ajax(
                            {   type: 'POST',
                                data: data,
                                dataType: 'text',
                                url: serverurl,
                                success: function(data) { 
                                     console.log('Parsed data');
                                     data = $.parseXML(data); 
                                     console.log(data);
                                }
                             }
                            );
      
      Show
      Edit the user linked to the token and set the first name as Noël. Enable mobile web service and also the REST protocol. Use the Javascript-REST demo client ( https://github.com/moodlehq/sample-ws-clients/tree/master/JAVASCRIPT-REST ) and change these lines: var functionname = 'moodle_webservice_get_siteinfo'; // .... var data = { wstoken: token, wsfunction: functionname, moodlewsrestformat: 'xml' } var response = $.ajax( { type: 'POST', data: data, dataType: 'text', url: serverurl, success: function(data) { console.log('Parsed data'); data = $.parseXML(data); console.log(data); } } ); In firebug: no JS errors (mainly an xml parsing error) should appeared. You should be able to see the resulting XML in your JS logs. For 2.0 testing you'll have to create your own service and use moodle_user_get_users_by_id ws function. You'll also have to change these lines: var functionname = 'moodle_user_get_users_by_id'; // ... var users = ['2']; var data = { wstoken: token, wsfunction: functionname, userids: users } var response = $.ajax( { type: 'POST', data: data, dataType: 'text', url: serverurl, success: function(data) { console.log('Parsed data'); data = $.parseXML(data); console.log(data); } } );
    • Workaround:
      Hide

      For javascript client using jquery:

      $.ajax({
      type: "POST",
      url: siteurl+"/webservice/rest/server.php?wstoken="+mytoken,
      data: data,
      dataType: 'text',
      dataFilter: function(data, dataType)

      { // XML returned by Moodle is not well parsed data = data.replace(/\<VALUE\>/gi,'<VALUE><![CDATA[').replace(/\<\/VALUE\>/gi,']]></VALUE>'); data = data.replace(/\<MESSAGE\>/gi,'<MESSAGE><![CDATA[').replace(/\<\/MESSAGE\>/gi,']]></MESSAGE>'); return data; }

      ,
      success: function(data)

      { ....data = $.parseXML(data); // }

      });

      Show
      For javascript client using jquery: $.ajax({ type: "POST", url: siteurl+"/webservice/rest/server.php?wstoken="+mytoken, data: data, dataType: 'text', dataFilter: function(data, dataType) { // XML returned by Moodle is not well parsed data = data.replace(/\<VALUE\>/gi,'<VALUE><![CDATA[').replace(/\<\/VALUE\>/gi,']]></VALUE>'); data = data.replace(/\<MESSAGE\>/gi,'<MESSAGE><![CDATA[').replace(/\<\/MESSAGE\>/gi,']]></MESSAGE>'); return data; } , success: function(data) { ....data = $.parseXML(data); // } });
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
    • Rank:
      19224

      Description

      The XML output produced by the REST web service converts all applicable characters to HTML entities. Most of these entities are not defined in XML and if they are in output of the REST web service function that output is not valid XML.

      In /webservice/rest/locallib.php in the xmlize_result() function all values in the output are filtered through the php htmlentities() function. Instead use the php htmlspecialchars function to only convert these characters – & < > " ' – to html entities.

      1. MDL-29712.patch
        2 kB
        Tom Cubanski
      2. rest_webservice_bad_xml.txt
        1 kB
        Tom Cubanski

        Issue Links

          Activity

          Hide
          Tom Cubanski added a comment -

          Its hard to discuss html entities through a web browser because the browser keeps changing them in the display. I've attached a text file that I have pasted in a web services call and response to better demonstrate this problem. In this example the XML generated by moodle_user_get_users_by_id that I pasted into the attached file is not valid XML.

          Show
          Tom Cubanski added a comment - Its hard to discuss html entities through a web browser because the browser keeps changing them in the display. I've attached a text file that I have pasted in a web services call and response to better demonstrate this problem. In this example the XML generated by moodle_user_get_users_by_id that I pasted into the attached file is not valid XML.
          Hide
          Juan Leyva added a comment -

          I think this bug is very important, is preventing using the REST + XML combination

          I have fixed it retrieving the document as it where plaintext and adding <![CDATA[ .. ]] to all the values:

          javascript code using jquery:

          $.ajax({
          type: "POST",
          url: siteurl+"/webservice/rest/server.php?wstoken="+mytoken,
          data: data,
          dataType: 'text',
          dataFilter: function(data, dataType)

          { // XML returned by Moodle is not well parsed data = data.replace(/\<VALUE\>/gi,'<VALUE><![CDATA[').replace(/\<\/VALUE\>/gi,']]></VALUE>'); data = data.replace(/\<MESSAGE\>/gi,'<MESSAGE><![CDATA[').replace(/\<\/MESSAGE\>/gi,']]></MESSAGE>'); return data; }

          ,
          success: function(data)

          { ....data = $.parseXML(data); // }

          });

          Regards

          Show
          Juan Leyva added a comment - I think this bug is very important, is preventing using the REST + XML combination I have fixed it retrieving the document as it where plaintext and adding <![CDATA[ .. ]] to all the values: javascript code using jquery: $.ajax({ type: "POST", url: siteurl+"/webservice/rest/server.php?wstoken="+mytoken, data: data, dataType: 'text', dataFilter: function(data, dataType) { // XML returned by Moodle is not well parsed data = data.replace(/\<VALUE\>/gi,'<VALUE><![CDATA[').replace(/\<\/VALUE\>/gi,']]></VALUE>'); data = data.replace(/\<MESSAGE\>/gi,'<MESSAGE><![CDATA[').replace(/\<\/MESSAGE\>/gi,']]></MESSAGE>'); return data; } , success: function(data) { ....data = $.parseXML(data); // } }); Regards
          Hide
          Tom Cubanski added a comment -

          I've attached a patch against the master branch of the fix that we a have been using to resolve this issue.

          Show
          Tom Cubanski added a comment - I've attached a patch against the master branch of the fix that we a have been using to resolve this issue.
          Hide
          Jérôme Mouneyrac added a comment -

          Thanks Tom and Juan. I'll have a look

          Show
          Jérôme Mouneyrac added a comment - Thanks Tom and Juan. I'll have a look
          Hide
          Patrick Pollet added a comment -
          Show
          Patrick Pollet added a comment - see also http://tracker.moodle.org/browse/MDL-28461 Cheers
          Hide
          Jérôme Mouneyrac added a comment -

          I'm testing Tom's patch. I don't have experience using REST-XML servers. As you (Juan, Patrick, and Tom) are using it, do you see any possible regression?

          If I change htmlentities for htmlspecialchars now, would it be possible that an existing REST-XML client expect all characters to be translated in HTML?

          At the end I could add a flag to the server to choose between htmlentities/htmlspecialchars. But if htmlentities makes no sens for any good client coding then it's overkill. And in this case I would prefer without a flag.

          Show
          Jérôme Mouneyrac added a comment - I'm testing Tom's patch. I don't have experience using REST-XML servers. As you (Juan, Patrick, and Tom) are using it, do you see any possible regression? If I change htmlentities for htmlspecialchars now, would it be possible that an existing REST-XML client expect all characters to be translated in HTML? At the end I could add a flag to the server to choose between htmlentities/htmlspecialchars. But if htmlentities makes no sens for any good client coding then it's overkill. And in this case I would prefer without a flag.
          Hide
          Tom Cubanski added a comment -

          I tested the xml I attached to this ticket in Java and with the php XML parser. Both parsers gave an error and failed to parse the bad xml.

          I don't think adding a flag to choose between htmlentities/htmlspecialchars is a good idea. It allows Moodle to produce output that standard xml parsers can't use.

          Show
          Tom Cubanski added a comment - I tested the xml I attached to this ticket in Java and with the php XML parser. Both parsers gave an error and failed to parse the bad xml. I don't think adding a flag to choose between htmlentities/htmlspecialchars is a good idea. It allows Moodle to produce output that standard xml parsers can't use.
          Hide
          Jérôme Mouneyrac added a comment -

          Thanks Tom.

          I now understand it breaks the xml valiation. With the patch, Juan "fixed" JS client will get:

          <![CDATA['Noël']]>
          

          instead of

          <![CDATA['No&euml;l']]>
          

          Juan, do you think it would break your client? It doesn't seem to me.

          I'm also wondering what would be the impact between htmlspecialchars and nothing as Patrick mentioned in MDL-28461.

          Show
          Jérôme Mouneyrac added a comment - Thanks Tom. I now understand it breaks the xml valiation. With the patch, Juan "fixed" JS client will get: <![CDATA['Noël']]> instead of <![CDATA['No&euml;l']]> Juan, do you think it would break your client? It doesn't seem to me. I'm also wondering what would be the impact between htmlspecialchars and nothing as Patrick mentioned in MDL-28461 .
          Hide
          Jérôme Mouneyrac added a comment -

          I pushed a commit as the patch seems good to me and it doesn't seem it would any clients.
          However before to backport and send to integration, I'll do more search about htmlspecialchars versus nothing.

          Show
          Jérôme Mouneyrac added a comment - I pushed a commit as the patch seems good to me and it doesn't seem it would any clients. However before to backport and send to integration, I'll do more search about htmlspecialchars versus nothing.
          Hide
          Tom Cubanski added a comment -

          Thanks for committing the patch Jerome. When choosing between htmlspecialchars and nothing you have make sure you convert the characters that define the XML structure: < > &. If those characters are in the document unconverted the document may not be able to be parsed. htmlspecialchars converts that characters that need to be converted for XML.

          Show
          Tom Cubanski added a comment - Thanks for committing the patch Jerome. When choosing between htmlspecialchars and nothing you have make sure you convert the characters that define the XML structure: < > &. If those characters are in the document unconverted the document may not be able to be parsed. htmlspecialchars converts that characters that need to be converted for XML.
          Hide
          Patrick Pollet added a comment -

          I second Tom's comment. Changing to htmlspecialchars is the way to go and it also fixed http://tracker.moodle.org/browse/MDL-28461 where errors messages were also messed when testing the REST mode in a browser.

          Show
          Patrick Pollet added a comment - I second Tom's comment. Changing to htmlspecialchars is the way to go and it also fixed http://tracker.moodle.org/browse/MDL-28461 where errors messages were also messed when testing the REST mode in a browser.
          Hide
          Jérôme Mouneyrac added a comment -

          Submitted for integration, thanks

          Show
          Jérôme Mouneyrac added a comment - Submitted for integration, thanks
          Hide
          Sam Hemelryk added a comment -

          Integrated thanks Jerome

          Show
          Sam Hemelryk added a comment - Integrated thanks Jerome
          Hide
          Rajesh Taneja added a comment -

          Thanks Tom and Jerome, for working on this.
          Works Great

          Show
          Rajesh Taneja added a comment - Thanks Tom and Jerome, for working on this. Works Great
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yes, you did it!

          Now your code is part of the best weeklies released ever, many thanks!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Yes, you did it! Now your code is part of the best weeklies released ever, many thanks! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: