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

create_user_record requires ipaddress, does not work from scripts

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.9.5
    • Fix Version/s: 1.9.10, 2.0
    • Component/s: Libraries
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE

      Description

      If you call create_user_record from a script or anywhere where $_SERVER['REMOTE_ADDR'] and such are not defined. It will try to insert a user record where lastip is null. This doesn't work because the user table has a non-null constraint on lastip. As a result, things like ldap authentication are broken (see last few comments on MDL-17682).

      Suggestions:

      • Remove non-null constraint. This makes the most sense to me because a user that has just be created by a script shouldn't have a lastip.
      • default to '127.0.0.1' in case getremoteaddr() returns null

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              trick Alan Trick added a comment -

              This is also an issue with create_course and enrolment modules.

              Show
              trick Alan Trick added a comment - This is also an issue with create_course and enrolment modules.
              Hide
              iarenaza Iñaki Arenaza added a comment -

              I think a better/safer fix is to change create_user_record() to check for a null return value from getremoteaddr and change it to an empty string, which is a valid database value. After all, the only use of getremoteaddr that has trouble with a null return value is create_user_record.

              Something like the attached patch (for 1.8.x, 1.9.x and HEAD) could do it.

              Saludos.
              Iñaki.

              Show
              iarenaza Iñaki Arenaza added a comment - I think a better/safer fix is to change create_user_record() to check for a null return value from getremoteaddr and change it to an empty string, which is a valid database value. After all, the only use of getremoteaddr that has trouble with a null return value is create_user_record. Something like the attached patch (for 1.8.x, 1.9.x and HEAD) could do it. Saludos. Iñaki.
              Hide
              trick Alan Trick added a comment -

              Does any of the Moodle code expect it to be a well-formed IPv4 address?

              Show
              trick Alan Trick added a comment - Does any of the Moodle code expect it to be a well-formed IPv4 address?
              Hide
              iarenaza Iñaki Arenaza added a comment -

              I don't think so (but I may be wrong).

              Saludos. Iñaki.

              Show
              iarenaza Iñaki Arenaza added a comment - I don't think so (but I may be wrong). Saludos. Iñaki.
              Hide
              pholden Paul Holden added a comment -

              The problem in this bug is related to the one I've linked in that the function getremoteaddr() doesn't return a value when called from the CLI.

              Show
              pholden Paul Holden added a comment - The problem in this bug is related to the one I've linked in that the function getremoteaddr() doesn't return a value when called from the CLI.
              Hide
              dougiamas Martin Dougiamas added a comment -

              I'm changing getremoteaddr() to return "0.0.0.0" instead of null by default, and checking all the calls (it was a bit of a mess, quite a few things would get upset with the null). 127.0.0.1 is not really safe, because in some checks that use this function the code will assume the client is the local machine. The default can be changed via a new parameter in case you really want a null.

              eg getremoteaddr(null)

              Show
              dougiamas Martin Dougiamas added a comment - I'm changing getremoteaddr() to return "0.0.0.0" instead of null by default, and checking all the calls (it was a bit of a mess, quite a few things would get upset with the null). 127.0.0.1 is not really safe, because in some checks that use this function the code will assume the client is the local machine. The default can be changed via a new parameter in case you really want a null. eg getremoteaddr(null)

                People

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

                  Dates

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