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

        1. setwantsurltome.patch
          4 kB
          Eloy Lafuente (stronk7)

          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