Moodle

From Petr Skoda (skodak at centrum.cz) Tuesday, 18 April 2006, 05:22 AM:

Hi!

I am not using flash or mediaplugin myself, you will have to assign it to somebody else

I guess you might try Jamie Pratt...

From Joseph R?zeau (joseph.rezeau at uhb.fr) Tuesday, 18 April 2006, 05:39 AM:

Martin, could you re-assign this bug please?

Joseph

From Martin Dougiamas (martin at moodle.com) Thursday, 20 April 2006, 04:26 PM:

It might fix your problem, but does it cause more problems with different input?

What is the exact case you are using there?

I've checked this into CVS on the assumption you're right, but this might need more testing.

From Joseph R?zeau (joseph.rezeau at uhb.fr) Friday, 21 April 2006, 12:35 AM:

Hi Martin!

>It might fix your problem, but does it cause more problems with different input?

Well, I don't think there is another input going through there...

>What is the exact case you are using there?

Sorry, what do you mean by case ? UPPERCASE vs lower case?

Anyway, I have tested this bug again, and I can confirm that it has been there since at least version 1.5. In fact, NO CHANGES take place at all in the colors of the Flash player!!! This is because the faulty call to htmlentities changes the correct string, e.g.

bgColour=000000&btnColour=ffffff&btnBorderColour=cccccc&iconColour=000000&iconOverColour=FF0000&trackColour=cccccc&handleColour=ffffff&loaderColour=ffffff&waitForPlay=yes&

into this garbage:

flashvars=bgColour=000000&btnColour=ffffff&btnBorderColour=cccccc&iconColour=000000&iconOverColour=FF0000&trackColour=cccccc&handleColour=ffffff&loaderColour=ffffff&waitForPlay=yes&

And I suspect that this bug has gone un-noticed all those years simply because no-one has ever taken the trouble to change the colors of the Flash player! I've noticed that ALL the themes which are currently distributed with Moodle simply copy exactly the same parameters as those from \filter\mediaplugin\filter.php. And it also goes un-noticed because the colors coded in filter.php are the same as those hard-coded in the Flash file! (i.e. black and green).

So I recommend deleting the faulty line

$c = htmlentities($c);

so everything works as expected.

Joseph

From Martin Dougiamas (martin at moodle.com) Friday, 21 April 2006, 12:26 PM:

OK, great, thanks for the thining on this, fixed in 1.5.x and HEAD

assign to a valid user

The htmlentities line has crept back in, and is breaking the flash vars as the original author stated.

I came across this because the waitForPlay option was doing nothing. Removing the htmlentities() call fixes this.

I guess the real solution is to apply htmlentities() to the each of the parameter values but keep the & separators literal.

> I guess the real solution is to apply htmlentities() to the each of the parameter values
> but keep the & separators literal.

Good idea. Here is a suggested fix for "filter/mediaplugin/filter.php":

OLD: $c = htmlentities($c);
NEW: $c = implode('&', array_map('htmlentities', explode('&', $c)));

Now fixed in the latest 19_STABLE using the fix that the original reporter suggested.

Gordon, I did try your patch however it looks like htmlentities() is not appropriate, we actually want urlencode() to escape chars in URL format. The problem is, the splitting on '&' is not enough since urlencode will also escape the '=' sign as well. Ideally we would have a proper array containing flash vars which then get escaped through urlencode(), however this change would impact all theme config.php files so not something we should introduce into 19_STABLE.

Petr could you just quickly check this for injections/XSS ?

looks ok, the data is not coming from user, so the worst alternative is that it would not validate as xhtml strict

Also just checked this into HEAD.

Note to theme designers: you should now be able to customise mp3 player settings/colours as documented in: http://docs.moodle.org/en/MP3_player

If you need to use special characters in flash vars, use appropriate URL encoding when setting $THEME->resource_mp3player_colors and $THEME->filter_mediaplugin_colors

See http://www.w3schools.com/TAGS/ref_urlencode.asp

Just specify the first version it was fixed in. Thanks

thanks everyone, closing