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

linking to an image file in a course from the front page can mess up $SESSION->wantsurl

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 1.8.6, 1.9.1
    • Component/s: Authentication
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE

      Description

      To reproduce...

      1. Upload an image to a course's file area and copy the files's url
      2. Place an img tag link to the image on the site front page
      3. Confirm that the image is not displayed when you are logged out (at least)
      4. Log in normally, you are sent to the image (in the course) - which will not be what a user expected.

      What's happening is that the img tag calls file.php, which calles require_login() which in turn goes to the login page - except you see none of this as it's an image. Unfortunately, $SESSION->wantsurl has been set to the location of the image. When you next log in (properly) it picks up the wantsurl setting and sends you to that location immediately after login.

      Of course, you shouldn't have that img there in the first place, but I spent days diagnosing this on a customer's site.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Thanks for research, Howard. In fact we had this reported since some days ago at MDL-14235... I'm going to see if we can leave file.php out from setting $SESSION->wantsurl in some way...

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Thanks for research, Howard. In fact we had this reported since some days ago at MDL-14235 ... I'm going to see if we can leave file.php out from setting $SESSION->wantsurl in some way... Ciao
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Adding patch that seems to fix the problem.

              1) It adds one new optional parameter to require_login() and require_course_login(): $setwantsurltome = true
              2) Default (true) respects old behaviour, defining $SESSION->wantsurl. If set to false, that session variable isn't overwritten.
              3) Updated file.php to use that parameter to avoid messing redirects.

              Patch is over 19_STABLE, please test. TIA!

              Note: Perhpas there are some other like-file.php scripts needing to be patched too, although I think file.php is the most obvious.

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Adding patch that seems to fix the problem. 1) It adds one new optional parameter to require_login() and require_course_login(): $setwantsurltome = true 2) Default (true) respects old behaviour, defining $SESSION->wantsurl. If set to false, that session variable isn't overwritten. 3) Updated file.php to use that parameter to avoid messing redirects. Patch is over 19_STABLE, please test. TIA! Note: Perhpas there are some other like-file.php scripts needing to be patched too, although I think file.php is the most obvious. Ciao
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment - - edited

              Adding Martin and Petr here to evaluate the change. I think it's safe, respecting old behaviour and allowing to have control over that session variable wherver necessary.

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - - edited Adding Martin and Petr here to evaluate the change. I think it's safe, respecting old behaviour and allowing to have control over that session variable wherver necessary. Ciao
              Hide
              skodak Petr Skoda added a comment -

              +1

              Show
              skodak Petr Skoda added a comment - +1
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Hi,

              this has been fixed under 19_STABLE and HEAD. Changes are available from CVS and will be included in next WEEKLY packages.

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Hi, this has been fixed under 19_STABLE and HEAD. Changes are available from CVS and will be included in next WEEKLY packages. Ciao
              Hide
              skodak Petr Skoda added a comment -

              thanks!

              Show
              skodak Petr Skoda added a comment - thanks!
              Hide
              poltawski Dan Poltawski added a comment -

              Wow, thanks for this.. i've seen this many times and never thought to file a bug! doh.

              Show
              poltawski Dan Poltawski added a comment - Wow, thanks for this.. i've seen this many times and never thought to file a bug! doh.
              Hide
              mblake Michael Blake added a comment -

              Reopening to request a patch for 1.8+

              Show
              mblake Michael Blake added a comment - Reopening to request a patch for 1.8+
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Backported. Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Backported. Ciao
              Hide
              dongsheng Dongsheng Cai added a comment -

              Thanks, Eloy

              Show
              dongsheng Dongsheng Cai added a comment - Thanks, Eloy

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    15/May/08