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 Bug
    • Status: Closed
    • Priority: Minor 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
    • Rank:
      30995

      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.

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

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          Petr Škoda added a comment -

          +1

          Show
          Petr Škoda added a comment - +1
          Hide
          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
          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
          Petr Škoda added a comment -

          thanks!

          Show
          Petr Škoda added a comment - thanks!
          Hide
          Dan Poltawski added a comment -

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

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

          Reopening to request a patch for 1.8+

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

          Backported. Ciao

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

          Thanks, Eloy

          Show
          Dongsheng Cai added a comment - Thanks, Eloy

            People

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

              Dates

              • Created:
                Updated:
                Resolved: