Moodle
  1. Moodle
  2. MDL-35669

Gravatar integration appears to have stopped working

    Details

    • Testing Instructions:
      Hide

      Note: This requires testing under 22 & 23 (master is 100% equivalent to 23).

      On a server with enablegravatar switched on:

      Test 1 (default setting)

      1. Leave the gravatardefaulturl setting at the default
      2. View the profile of a user whose email address is not linked to a gravatar profile
      3. You should see Gravatar's "mystery man" image as the profile picture

      Test 2 (custom image)

      1. Set the gravatardefaulturl setting to the URL of an image
      2. View the profile of a user whose email address is not linked to a gravatar profile
      3. You should see the image from step 1

      Test 3 (empty setting)

      1. Set the gravatardefaulturl setting to the empty string
      2. View the profile of a user whose email address is not linked to a gravatar profile
      3. You should see the default image for the current page's theme
      Show
      Note: This requires testing under 22 & 23 (master is 100% equivalent to 23). On a server with enablegravatar switched on: Test 1 (default setting) Leave the gravatardefaulturl setting at the default View the profile of a user whose email address is not linked to a gravatar profile You should see Gravatar's "mystery man" image as the profile picture Test 2 (custom image) Set the gravatardefaulturl setting to the URL of an image View the profile of a user whose email address is not linked to a gravatar profile You should see the image from step 1 Test 3 (empty setting) Set the gravatardefaulturl setting to the empty string View the profile of a user whose email address is not linked to a gravatar profile You should see the default image for the current page's theme
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-35669-master-2
    • Rank:
      44406

      Description

      This morning we've noticed that Gravatar integration has stopped working on our server (it was working yesterday). This can also be seen on the moodle.org server (e.g. http://moodle.org/user/view.php?id=1504560&course=5)

      It looks as if Gravatar may have introduced checking for a file extension for default images - the image URL generated for the above example user is http://www.gravatar.com/avatar/6df0d771964ef4d1f10cab72149d8460?s=100&d=http%3A%2F%2Fmoodle.org%2Ftheme%2Fimage.php%2Fmoodleofficial%2Fcore%2F1348555103%2Fu%2Ff1 which is now giving a page saying "The type of image you are trying to process is not allowed".

      I have checked this on various themes, including standard, and it isn't working on any of them so I don't think it is caused by a corrupt image file or anything like that. I've also checked the Gravatar site to see if this is documented but can't find it anywhere (so all this is just speculation!)

        Issue Links

          Activity

          Hide
          Charles Fulton added a comment - - edited

          We're seeing this as well--same symptoms. Bumping priority to Major.

          One workaround would be to manually append an extension (any, doesn't matter) to the URL builder in lib/outputcomponents.php and then strip that extension back out in theme/image.php. That doesn't feel like a stable solution, though.

          Show
          Charles Fulton added a comment - - edited We're seeing this as well--same symptoms. Bumping priority to Major. One workaround would be to manually append an extension (any, doesn't matter) to the URL builder in lib/outputcomponents.php and then strip that extension back out in theme/image.php. That doesn't feel like a stable solution, though.
          Hide
          matt crider added a comment -

          Me as well. The URL I'm using is http://gravatar.com/avatar/46016ddc91706a62e508455b5dbdf9df?rating=X&size=80&default=http://gravatar.com/avatar.php (based off of a fake email address – simplekidder@mailinator.com) .

          Show
          matt crider added a comment - Me as well. The URL I'm using is http://gravatar.com/avatar/46016ddc91706a62e508455b5dbdf9df?rating=X&size=80&default=http://gravatar.com/avatar.php (based off of a fake email address – simplekidder@mailinator.com) .
          Hide
          Helen Foster added a comment -

          Just noting that I have temporarily disabled the use of gravatars on moodle.org until this issue is resolved.

          Show
          Helen Foster added a comment - Just noting that I have temporarily disabled the use of gravatars on moodle.org until this issue is resolved.
          Hide
          Michael Aherne added a comment - - edited

          I've had a good look at this and I can't see any way to fix it sensibly that'll work with both the following configurations:

          1. $CFG->themedir used to include themes that are outside of dirroot
          2. $CFG->slasharguments off

          The options I thought of:

          1. Change the gravatar URL generating code to give an absolute URL to a specific image file. This could be done fairly easily but would only work for themes under dirroot, not with ones in themedir (assuming it's outside dirroot)
          2. [Charles's workaround above] Find out the extension of the file (using resolve_image_location()) and append it to the URL, then tweak theme/image.php to first see if $image is a full filename before trying to append the standard extensions. This could probably be done, but it wouldn't solve the Gravatar issue where slasharguments is off as Gravatar would see .php as the file extension.

          The only possible solution I can see is always to send the absolute URL to Moodle's default "no profile picture" images (e.g. $CFG->wwwroot.'/pix/u/f1.png'), which would be less than ideal.

          Can anyone see any other possibilities?

          Show
          Michael Aherne added a comment - - edited I've had a good look at this and I can't see any way to fix it sensibly that'll work with both the following configurations: $CFG->themedir used to include themes that are outside of dirroot $CFG->slasharguments off The options I thought of: Change the gravatar URL generating code to give an absolute URL to a specific image file. This could be done fairly easily but would only work for themes under dirroot, not with ones in themedir (assuming it's outside dirroot) [Charles's workaround above] Find out the extension of the file (using resolve_image_location()) and append it to the URL, then tweak theme/image.php to first see if $image is a full filename before trying to append the standard extensions. This could probably be done, but it wouldn't solve the Gravatar issue where slasharguments is off as Gravatar would see .php as the file extension. The only possible solution I can see is always to send the absolute URL to Moodle's default "no profile picture" images (e.g. $CFG->wwwroot.'/pix/u/f1.png'), which would be less than ideal. Can anyone see any other possibilities?
          Hide
          Michael Aherne added a comment - - edited

          I've put a patch at https://github.com/micaherne/moodle/commit/e09c142001bba50d1ca00927636dd20d8179f2e1 which basically does option 1 above unless the image has to be served through image.php, in which case it just uses the default images from /pix.

          It's not intended as a fix as-is, just as an illustration of what I was talking about in the previous comment!

          Show
          Michael Aherne added a comment - - edited I've put a patch at https://github.com/micaherne/moodle/commit/e09c142001bba50d1ca00927636dd20d8179f2e1 which basically does option 1 above unless the image has to be served through image.php, in which case it just uses the default images from /pix. It's not intended as a fix as-is, just as an illustration of what I was talking about in the previous comment!
          Hide
          Petr Škoda added a comment -

          Hmm, there could be also a new admin setting for default gravatar image (full URL) and by default we could default to their mystery-man (https://en.gravatar.com/site/implement/images/).

          Show
          Petr Škoda added a comment - Hmm, there could be also a new admin setting for default gravatar image (full URL) and by default we could default to their mystery-man ( https://en.gravatar.com/site/implement/images/ ).
          Hide
          Michael Aherne added a comment - - edited

          Good idea, Petr! I've incorporated this into the patch and added it as a pull request, as it seems like a usable solution now.

          I guess the downsides of having a URL defined in a setting are:

          1. it doesn't change depending on which theme the page is using
          2. there are no resized versions of it so it will rely on the browser for resizing - not always too good, particularly in IE

          so I've left my original process for finding the correct image from the theme (where possible) available.

          Show
          Michael Aherne added a comment - - edited Good idea, Petr! I've incorporated this into the patch and added it as a pull request, as it seems like a usable solution now. I guess the downsides of having a URL defined in a setting are: it doesn't change depending on which theme the page is using there are no resized versions of it so it will rely on the browser for resizing - not always too good, particularly in IE so I've left my original process for finding the correct image from the theme (where possible) available.
          Hide
          Petr Škoda added a comment -

          Yes, the combination of these two approaches could be the best, maybe it would be better to do the resolve_image_location('u/'.$filename, 'core'); only when necessary after the first if for performance reasons.

          Show
          Petr Škoda added a comment - Yes, the combination of these two approaches could be the best, maybe it would be better to do the resolve_image_location('u/'.$filename, 'core'); only when necessary after the first if for performance reasons.
          Hide
          Michael Aherne added a comment -

          Good point! I'll get it updated.

          Show
          Michael Aherne added a comment - Good point! I'll get it updated.
          Hide
          Michael Aherne added a comment -

          OK, that's it updated so it doesn't do any further processing if the setting is set.

          Show
          Michael Aherne added a comment - OK, that's it updated so it doesn't do any further processing if the setting is set.
          Hide
          Petr Škoda added a comment -

          this "$gravatardefault = $CFG->gravatardefaulturl;" would throw a warning before the upgrade, it is recommended to do:

          if (empty($CFG->gravatardefaulturl)) {
            ....
          } else {
            $gravatardefault = $CFG->gravatardefaulturl;
          }
          

          the rest looks ok.

          Show
          Petr Škoda added a comment - this "$gravatardefault = $CFG->gravatardefaulturl;" would throw a warning before the upgrade, it is recommended to do: if (empty($CFG->gravatardefaulturl)) { .... } else { $gravatardefault = $CFG->gravatardefaulturl; } the rest looks ok.
          Hide
          Charles Fulton added a comment -

          If anyone's curious I finally got answer back from Gravatar "support":

          We have recently implemented some changes that wouldn't change our current documentation so much as an enforce that default images must actually be images. They should also be publicly accessible.

          I noticed in the case you mentioned that you're using a php file to serve up the image. For security reasons, that will no longer work.

          I think that's nonsense myself but there we are.

          Show
          Charles Fulton added a comment - If anyone's curious I finally got answer back from Gravatar "support": We have recently implemented some changes that wouldn't change our current documentation so much as an enforce that default images must actually be images. They should also be publicly accessible. I noticed in the case you mentioned that you're using a php file to serve up the image. For security reasons, that will no longer work. I think that's nonsense myself but there we are.
          Hide
          Petr Škoda added a comment -

          I suppose they are afraid of CSRF attacks, thanks for the info, I guess that confirms the latest patch is the safest way to guarantee compatibility.

          Show
          Petr Škoda added a comment - I suppose they are afraid of CSRF attacks, thanks for the info, I guess that confirms the latest patch is the safest way to guarantee compatibility.
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Note that this change also breaks the use of gravatar from any non-public site. Crazy change!

          https://bugs.launchpad.net/libravatar/+bug/1057832

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Note that this change also breaks the use of gravatar from any non-public site. Crazy change! https://bugs.launchpad.net/libravatar/+bug/1057832
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          I really don't think the submitted patch is the best way.

          1) It shows one notice before upgrading.
          2) After upgrading, all sites will default to d=mm
          3) It never will show a image from a moodle profile.
          4) Line #395 is missing "/pix" path ?

          Really we cannot continue using the default image (d=url) anymore, because the gravatar guys have broken it for all moodle (and other products) installations. Not sure why are fetching those files and later saying they are incorrect. The old "blindy redirect" was far better (for our interests).

          Anyway, right now... A) no *.php is allowed anymore and B) no intranets are allowed anymore.

          So, given A) and B)... I only can imagine 2 solutions, both changing Moodle's approach:

          S1) avoid relying on gravatar default images and try to use internal image first, only calling to gravatar, with d=mm, if local is missing). This is a change in current behavior where gravatar ones have precedence).
          S2) change/support the images serving scripts to be "non-php". Could be an alternative in some sites, not for all.

          Bah, gravatar, grrr

          Show
          Eloy Lafuente (stronk7) added a comment - - edited I really don't think the submitted patch is the best way. 1) It shows one notice before upgrading. 2) After upgrading, all sites will default to d=mm 3) It never will show a image from a moodle profile. 4) Line #395 is missing "/pix" path ? Really we cannot continue using the default image (d=url) anymore, because the gravatar guys have broken it for all moodle (and other products) installations. Not sure why are fetching those files and later saying they are incorrect. The old "blindy redirect" was far better (for our interests). Anyway, right now... A) no *.php is allowed anymore and B) no intranets are allowed anymore. So, given A) and B)... I only can imagine 2 solutions, both changing Moodle's approach: S1) avoid relying on gravatar default images and try to use internal image first, only calling to gravatar, with d=mm, if local is missing). This is a change in current behavior where gravatar ones have precedence). S2) change/support the images serving scripts to be "non-php". Could be an alternative in some sites, not for all. Bah, gravatar, grrr
          Hide
          Michael Aherne added a comment -

          Hi Petr & Eloy, thanks for the feedback. I've incorporated the fix for the pre-upgrade notice and fixed the missing "pix" link. On Eloy's points:

          1. is fixed
          2. this was by design as suggested by Petr - is there a better default value that could be used?
          3. I don't see this behaviour - if I upload an image it is shown even if there is a gravatar profile linked to the user's email address. It sounds from your description of solution 1 that this is not expected behaviour, though. Can you give some more detail?
          4. is fixed

          I've assigned the issue back to moodle.com since it's not clear whether this is the right approach to fixing the problem, and I'm not really in a position to decide one way or another!

          Show
          Michael Aherne added a comment - Hi Petr & Eloy, thanks for the feedback. I've incorporated the fix for the pre-upgrade notice and fixed the missing "pix" link. On Eloy's points: is fixed this was by design as suggested by Petr - is there a better default value that could be used? I don't see this behaviour - if I upload an image it is shown even if there is a gravatar profile linked to the user's email address. It sounds from your description of solution 1 that this is not expected behaviour, though. Can you give some more detail? is fixed I've assigned the issue back to moodle.com since it's not clear whether this is the right approach to fixing the problem, and I'm not really in a position to decide one way or another!
          Hide
          Petr Škoda added a comment -

          Hi and thanks, I believe the current patch is correct, I have discussed it with Eloy a bit today.

          Eloy: it is up to you, my +1 to get it integrated this week, anything seems better than current code...

          Show
          Petr Škoda added a comment - Hi and thanks, I believe the current patch is correct, I have discussed it with Eloy a bit today. Eloy: it is up to you, my +1 to get it integrated this week, anything seems better than current code...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Michael,

          yes, yes, I was wrong, yesterday I did not read the patch in context and that leaded here to incorrect conclusions (#2 and #3 in my list). Today, thanks to Petr, I've seen the "light"!

          Integrating this...

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Michael, yes, yes, I was wrong, yesterday I did not read the patch in context and that leaded here to incorrect conclusions (#2 and #3 in my list). Today, thanks to Petr, I've seen the "light"! Integrating this...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (22, 23 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (22, 23 & master), thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Added one commit to fix images when serving under https.

          Show
          Eloy Lafuente (stronk7) added a comment - Added one commit to fix images when serving under https.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Added one more commit to amend existing tests and completed to cover some missing combinations.

          Show
          Eloy Lafuente (stronk7) added a comment - Added one more commit to amend existing tests and completed to cover some missing combinations.
          Hide
          Jason Fowler added a comment -

          Works as described. Thanks Michael

          Show
          Jason Fowler added a comment - Works as described. Thanks Michael
          Hide
          Michael Aherne added a comment -

          Thanks for fixing the secure image URL, Eloy - I'd completely missed that one!

          Show
          Michael Aherne added a comment - Thanks for fixing the secure image URL, Eloy - I'd completely missed that one!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Heh, me too (when reviewing it). Say thanks to unit tests, yay!

          Show
          Eloy Lafuente (stronk7) added a comment - Heh, me too (when reviewing it). Say thanks to unit tests, yay!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Closing as fixed, many thanks for your awesome collaboration.

          Show
          Eloy Lafuente (stronk7) added a comment - Closing as fixed, many thanks for your awesome collaboration.
          Hide
          Helen Foster added a comment -

          New Gravatar default image URL setting documented in http://docs.moodle.org/23/en/Roles_settings and http://docs.moodle.org/22/en/Gravatars

          Show
          Helen Foster added a comment - New Gravatar default image URL setting documented in http://docs.moodle.org/23/en/Roles_settings and http://docs.moodle.org/22/en/Gravatars

            People

            • Votes:
              6 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: