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
    • Rank:
      33640

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

        Activity

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

        Integrated, thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
        Hide
        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 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
        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
        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: