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

html_to_text is pathologically slow with certain input involving <pre>

    Details

    • Database:
      Any
    • Testing Instructions:
      Hide
      1. Place the attached script into the root of your Moodle site
      2. Run the attached script via a web browser
      3. The output should be:
        • without the patched code – the page will run for however long your PHP timeout is set to, and then display the conversion result for the first string and the second string without a converted result
        • with the patches code – the page will load and both strings will be converted from their HTML format into text and displayed on the page very quickly
      Show
      Place the attached script into the root of your Moodle site Run the attached script via a web browser The output should be: without the patched code – the page will run for however long your PHP timeout is set to, and then display the conversion result for the first string and the second string without a converted result with the patches code – the page will load and both strings will be converted from their HTML format into text and displayed on the page very quickly
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-30741_master

      Description

      The following string value from a quiz question was causing the convert_pre() method in _/lib/html2text.php to get stuck and never finish converting the string to a text representation:

      Consider the following function:<br /><pre style="color: rgb(153, 51, 102);">void FillMeUp(char* in_string) {<br />  int i = 0;<br />  while (in_string[i] != '\0') {<br />    in_string[i] = 'X';<br />    i++;<br />  }<br />}</pre>What would happen if a non-terminated string were input to this function?<br /><br />

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            jfilip Justin Filip added a comment -

            I've incorporated the updated code from the upstream html2text.php file dealing with parsing of PRE tags and written a new unit test in lib/simpletest/testweblib.php for testing the PRE parsing.

            Show
            jfilip Justin Filip added a comment - I've incorporated the updated code from the upstream html2text.php file dealing with parsing of PRE tags and written a new unit test in lib/simpletest/testweblib.php for testing the PRE parsing.
            Hide
            jfilip Justin Filip added a comment -
            Show
            jfilip Justin Filip added a comment - The updated code for html2text.php can be found here: http://trac.roundcube.net/browser/trunk/roundcubemail/program/lib/html2text.php?rev=5497
            Hide
            jfilip Justin Filip added a comment -

            The earliest test.php upload needs to be removed but I do not have permission to do that myself.

            Show
            jfilip Justin Filip added a comment - The earliest test.php upload needs to be removed but I do not have permission to do that myself.
            Hide
            timhunt Tim Hunt added a comment -

            I guess this needs to be cherry-picked to all later branches, but that does not seem to be a problem. This looks like a good fix to me, so submitting for integration. +1

            Show
            timhunt Tim Hunt added a comment - I guess this needs to be cherry-picked to all later branches, but that does not seem to be a problem. This looks like a good fix to me, so submitting for integration. +1
            Hide
            tsala Helen Foster added a comment -

            Justin, thanks for your report with fix. I've just added you to the developers group in the tracker and am assigning this issue to you as suggested by Tim.

            Show
            tsala Helen Foster added a comment - Justin, thanks for your report with fix. I've just added you to the developers group in the tracker and am assigning this issue to you as suggested by Tim.
            Hide
            jfilip Justin Filip added a comment -

            Cherry-picked the commits into all of the remaining 2.x branches and updated the pull and diff information.

            Is there anything else that I need to do with this one?

            Show
            jfilip Justin Filip added a comment - Cherry-picked the commits into all of the remaining 2.x branches and updated the pull and diff information. Is there anything else that I need to do with this one?
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Hi,
            Works as expected.
            Tough just noticed this is the out put that I get for first string

            The output of the following is "Segmentation Fault", i.e., the program crashes: int main() { int **p = new int*; int *q = new int; *q = 23; *p = q; cout 

            As you can see the last part after cout (<< **p;
            }) is not present, anyway that seems expected behavior to me considering '<' is present.
            passing
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Hi, Works as expected. Tough just noticed this is the out put that I get for first string The output of the following is "Segmentation Fault", i.e., the program crashes: int main() { int **p = new int*; int *q = new int; *q = 23; *p = q; cout As you can see the last part after cout (<< **p; }) is not present, anyway that seems expected behavior to me considering '<' is present. passing Thanks
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

            Now... disconnect, relax and enjoy the next days, yay!

            Closing...ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks! Now... disconnect, relax and enjoy the next days, yay! Closing...ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  9/Jan/12