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

Mediaplugin filter returns blank $text when bad UTF-8 is found

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.7.2
    • Fix Version/s: 1.7.3, 1.8.3, 1.9
    • Component/s: Filters, Resource, Unicode
    • Labels:
      None
    • Environment:
      PHP5-only issue.
    • Affected Branches:
      MOODLE_17_STABLE
    • Fixed Branches:
      MOODLE_17_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE

      Description

      I've been hours trying to find out why, under certain circumstances, a link like "/file.php/263/onefile.htm" would return a blank page while "/file.php/263/otherfile.htm" does not. Tracing took me through file.php->filelib.php (send_file function) -> filter_text function and finally to mediaplugin_filter function, where I found that preg_replace() was the one returning a blank $text.

      The problem arose when upgraded the server to PHP5, so I suspect preg_replace() is returning an empty $text when the original $text is not well-formed UTF-8. This is very probable, since the files in moodledata not necessarily are in UTF-8 (this particular installation was a 1.4, then 1.5, and now 1.7.2).

        Gliffy Diagrams

          Activity

          Hide
          petcheverry Pablo Etcheverry added a comment -

          I've made an ugly hack to solve this problem. I just check the result of preg_replace and if it is null, I simply ignore it and replace it with the original text.

          filter/mediaplugin/filter.php:

          -$text = preg_replace($search, $replace, $text);

          +$newtext = preg_replace($search, $replace, $text);
          +$text = $newtext ? $newtext : $text;

          Show
          petcheverry Pablo Etcheverry added a comment - I've made an ugly hack to solve this problem. I just check the result of preg_replace and if it is null, I simply ignore it and replace it with the original text. filter/mediaplugin/filter.php: -$text = preg_replace($search, $replace, $text); +$newtext = preg_replace($search, $replace, $text); +$text = $newtext ? $newtext : $text;
          Hide
          dougiamas Martin Dougiamas added a comment -

          Nick, can you please check this in to 1.7, 1.8 and head?

          Show
          dougiamas Martin Dougiamas added a comment - Nick, can you please check this in to 1.7, 1.8 and head?
          Hide
          rezeau Joseph Rézeau added a comment - - edited

          Nicolas,
          You shouldn't have applied Pablo's lovely/ugly hack... Now, the mutlimedia filter is broken: it won't filter HTML uploaded files or Composed Web pages or other resources etc.for MP3 links to display the MP3 player. GRRRR!

          The problem appears when more than one filter is enabled (which is the usual case). Then, the $text passed as parameter is lost because of the $newtext variable you introduced...

          Joseph

          Show
          rezeau Joseph Rézeau added a comment - - edited Nicolas, You shouldn't have applied Pablo's lovely/ugly hack... Now, the mutlimedia filter is broken: it won't filter HTML uploaded files or Composed Web pages or other resources etc.for MP3 links to display the MP3 player. GRRRR! The problem appears when more than one filter is enabled (which is the usual case). Then, the $text passed as parameter is lost because of the $newtext variable you introduced... Joseph
          Hide
          skodak Petr Skoda added a comment -

          Oh - I did not notice this bug, seems to be similar to MDL-10155 - I guess there should be an is_null() check instead, see:
          http://moodle.cvs.sourceforge.net/moodle/moodle/filter/multilang/filter.php?r1=1.19&r2=1.20

          Show
          skodak Petr Skoda added a comment - Oh - I did not notice this bug, seems to be similar to MDL-10155 - I guess there should be an is_null() check instead, see: http://moodle.cvs.sourceforge.net/moodle/moodle/filter/multilang/filter.php?r1=1.19&r2=1.20
          Hide
          nicolasconnault Nicolas Connault added a comment -

          Sorry Joseph, this is fixed now. I must not have been thinking straight

          Show
          nicolasconnault Nicolas Connault added a comment - Sorry Joseph, this is fixed now. I must not have been thinking straight
          Hide
          nicolasconnault Nicolas Connault added a comment -

          I haven't tested this, but I haven't heard any complaints since I fixed it and committed it over a week ago, so I'm now closing the bug.

          Show
          nicolasconnault Nicolas Connault added a comment - I haven't tested this, but I haven't heard any complaints since I fixed it and committed it over a week ago, so I'm now closing the bug.

            People

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

              Dates

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