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

Broken links on confirmation email (or others)

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.5, 1.5.1, 1.5.2, 1.5.3, 1.5.4, 1.6.8, 1.7.6, 1.8.7, 1.9.7
    • Fix Version/s: 1.8.12, 1.9.8
    • Component/s: Libraries
    • Labels:
      None
    • Environment:
      All
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_15_STABLE, MOODLE_16_STABLE, MOODLE_17_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE

      Description

      Problem:
      At least with extended chars allowed in username the link is incorrectly done.
      e.g. username is 'john smith' the resulting link will be:

      <a href="http://...../john" target="_blank"> http://...../john </a> smith

      or simply:

      httpt://......./john smith

      Solution:
      urlencode($user->username)

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            paulo.matos Paulo Matos added a comment -

            Verified against moodle v1.6.4+ (2006050540), patch apply correctly.

            Show
            paulo.matos Paulo Matos added a comment - Verified against moodle v1.6.4+ (2006050540), patch apply correctly.
            Hide
            paulo.matos Paulo Matos added a comment -

            Is this hard to merge?

            Show
            paulo.matos Paulo Matos added a comment - Is this hard to merge?
            Hide
            skodak Petr Skoda added a comment -

            fixed in cvs, thanks for the patch

            Show
            skodak Petr Skoda added a comment - fixed in cvs, thanks for the patch
            Hide
            paulo.matos Paulo Matos added a comment - - edited

            Just discovered a side effect on login/confirm.php.

            Show
            paulo.matos Paulo Matos added a comment - - edited Just discovered a side effect on login/confirm.php.
            Hide
            paulo.matos Paulo Matos added a comment -

            When parameter 'data' is passed, formatted as: secret/username, then splitted by '/', username is not "urldecoded'. The patch fix this.

            This might affect 'p' parameter as well, however I could not find any reference in entire source to the use of it.

            The link generated with forget_password.php seems not to be affected.

            Show
            paulo.matos Paulo Matos added a comment - When parameter 'data' is passed, formatted as: secret/username, then splitted by '/', username is not "urldecoded'. The patch fix this. This might affect 'p' parameter as well, however I could not find any reference in entire source to the use of it. The link generated with forget_password.php seems not to be affected.
            Hide
            paulo.matos Paulo Matos added a comment -

            Hi Eloy! Please take a look to this. It's a simple fix.

            Show
            paulo.matos Paulo Matos added a comment - Hi Eloy! Please take a look to this. It's a simple fix.
            Hide
            paulo.matos Paulo Matos added a comment -

            Hi folks!
            Just reviewed this issue against latest 1.9.7+ and side problem on confirm.php is still there.
            Can you do this simple merge?


            Paulo Matos

            Show
            paulo.matos Paulo Matos added a comment - Hi folks! Just reviewed this issue against latest 1.9.7+ and side problem on confirm.php is still there. Can you do this simple merge? – Paulo Matos
            Hide
            keoghs Sean Keogh added a comment -

            Hi Guys,

            We are getting clients that we have upgraded to 1.9.7+ suffering from this problem. Could someone (Eloy perhaps?) take another look, and put the fix in if at all possible please?

            Show
            keoghs Sean Keogh added a comment - Hi Guys, We are getting clients that we have upgraded to 1.9.7+ suffering from this problem. Could someone (Eloy perhaps?) take another look, and put the fix in if at all possible please?
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Hi, thanks for pinging this up! (was lost deep in my bug lists).

            Just reproducing it now in order to test patch and apply it. Targets, 1.8, 1.9 and 2.0

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Hi, thanks for pinging this up! (was lost deep in my bug lists). Just reproducing it now in order to test patch and apply it. Targets, 1.8, 1.9 and 2.0 Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Oki, I was able to reproduce the problem.

            Anyway, the solution doesn't seem to be to urldecode() the information, as far as PHP does that for the whole GET/REQUEST automatically!

            The problem seemed to be the explode() exploding too much, because the string has > 1 slashes. So I've limited the explode to produce only 2 elements and now it seems to be working ok.

            Tested on 1.9 and applied to 1.8.12 / 1.9.8 / 2.0 (will be available next weekly)

            Resolving as fixed, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Oki, I was able to reproduce the problem. Anyway, the solution doesn't seem to be to urldecode() the information, as far as PHP does that for the whole GET/REQUEST automatically! The problem seemed to be the explode() exploding too much, because the string has > 1 slashes. So I've limited the explode to produce only 2 elements and now it seems to be working ok. Tested on 1.9 and applied to 1.8.12 / 1.9.8 / 2.0 (will be available next weekly) Resolving as fixed, ciao

              People

              • Votes:
                2 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  25/Mar/10