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

Web Service moodle_user_create_users() does not accept leading/trailing spaces in firstname / lastname

    Details

    • Testing Instructions:
      Hide

      1) turn on web services under advanced features.
      2) plugins->web services -> external services , create a web service
      3) define in the GUI, a web services function that creates moodle users using 'moodle_user_create_users'
      4) call the web service with a web services test client (some can be cloned from here : git@github.com:moodlehq/sample-ws-clients.git )
      5) check that it can create firstname and lastnames with spaces in it.

      Please test MDL-30878 also. Part of this patch was removed for testing MDL-30878.

      Show
      1) turn on web services under advanced features. 2) plugins->web services -> external services , create a web service 3) define in the GUI, a web services function that creates moodle users using 'moodle_user_create_users' 4) call the web service with a web services test client (some can be cloned from here : git@github.com:moodlehq/sample-ws-clients.git ) 5) check that it can create firstname and lastnames with spaces in it. Please test MDL-30878 also. Part of this patch was removed for testing MDL-30878.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-25027_stable_consistency_fix

      Description

      Users can be added with leading/trailing spaces in firstname and lastname via the GUI form. However, the same values submitted via web services will cause an exception to be thrown.

      This appears to be because the create_users service runs the data through truncate_userinfo() and throws an exception if it is returned modified in anyway. truncate_userinfo() has a call to trim() which strips the spaces. The fields are of type PARAM_NOTAGS which allows the spaces.

        Gliffy Diagrams

          Issue Links

            Activity

            ashleyholman Ashley Holman created issue -
            jerome Jérôme Mouneyrac made changes -
            Field Original Value New Value
            Fix Version/s 2.0 [ 10122 ]
            dougiamas Martin Dougiamas made changes -
            Fix Version/s 2.0.1 [ 10420 ]
            Fix Version/s 2.0 [ 10122 ]
            jerome Jérôme Mouneyrac made changes -
            Fix Version/s STABLE backlog [ 10463 ]
            Fix Version/s 2.0.1 [ 10420 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Labels triaged
            dougiamas Martin Dougiamas made changes -
            Workflow jira [ 40001 ] MDL Workflow [ 47009 ]
            dougiamas Martin Dougiamas made changes -
            Workflow MDL Workflow [ 47009 ] MDL Full Workflow [ 75221 ]
            ashleyholman Ashley Holman made changes -
            Labels triaged netspot triaged
            mblake Michael Blake made changes -
            Labels netspot triaged netspot partner triaged
            jerome Jérôme Mouneyrac made changes -
            Priority Minor [ 4 ] Critical [ 2 ]
            jerome Jérôme Mouneyrac made changes -
            Link This issue will help resolve MDL-29276 [ MDL-29276 ]
            filter manager Filter Manager made changes -
            Fix Version/s STABLE Sprint 14 [ 11050 ]
            Fix Version/s STABLE backlog [ 10463 ]
            Assignee Jerome Mouneyrac [ jerome ] Aparup Banerjee [ nebgor ]
            jerome Jérôme Mouneyrac made changes -
            Link This issue has a non-specific relationship to MDL-29374 [ MDL-29374 ]
            nebgor Aparup Banerjee made changes -
            Status Open [ 1 ] Development in progress [ 3 ]
            nebgor Aparup Banerjee made changes -
            Link This issue has been marked as being related by MDL-29582 [ MDL-29582 ]
            nebgor Aparup Banerjee made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            Peer reviewer rwijaya
            nebgor Aparup Banerjee made changes -
            Testing Instructions 1) turn on web services under advanced features.
            2) plugins->web services -> external services , create a web service
            3) define in the GUI, a web services function that creates moodle users using 'moodle_user_create_users'
            4) call the web service with a web services test client
            5) check that it can create firstname and lastnames with spaces in it.
            rwijaya Rossiani Wijaya made changes -
            Original Estimate 0 minutes [ 0 ]
            Remaining Estimate 0 minutes [ 0 ]
            Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
            nebgor Aparup Banerjee made changes -
            Pull Master Diff URL https://github.com/nebgor/moodle/compare/master...MDL-25027 https://github.com/nebgor/moodle/compare/mistress...MDL-25027
            Testing Instructions 1) turn on web services under advanced features.
            2) plugins->web services -> external services , create a web service
            3) define in the GUI, a web services function that creates moodle users using 'moodle_user_create_users'
            4) call the web service with a web services test client
            5) check that it can create firstname and lastnames with spaces in it.
            1) turn on web services under advanced features.
            2) plugins->web services -> external services , create a web service
            3) define in the GUI, a web services function that creates moodle users using 'moodle_user_create_users'
            4) call the web service with a web services test client (some can be cloned from here : git@github.com:moodlehq/sample-ws-clients.git )
            5) check that it can create firstname and lastnames with spaces in it.
            nebgor Aparup Banerjee made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            rwijaya Rossiani Wijaya made changes -
            Status Development in progress [ 3 ] Peer review in progress [ 10013 ]
            rwijaya Rossiani Wijaya made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            rwijaya Rossiani Wijaya made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Waiting for integration review [ 10010 ] Reopened [ 4 ]
            moodle.com moodle.com made changes -
            Affects Version/s STABLE Sprint 15 [ 11158 ]
            Affects Version/s 2.0 [ 10122 ]
            moodle.com moodle.com made changes -
            Fix Version/s STABLE Sprint 15 [ 11158 ]
            Fix Version/s STABLE Sprint 14 [ 11050 ]
            nebgor Aparup Banerjee made changes -
            Status Reopened [ 4 ] Development in progress [ 3 ]
            nebgor Aparup Banerjee made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            Peer reviewer rwijaya samhemelryk
            samhemelryk Sam Hemelryk made changes -
            Comment [ Hi :)

            I've been looking at this issue and MDL-29582 this morning and have to admit I'm a little confused.

            # The modified method _truncate_userinfo_ is taking a portion of the string and then trimming which is a weird order to have those two operations in. The only reason I can think of for the trim is to remove trailing whitespace that is perhaps left over from the substr operation... department "Department of communication & technology" would be trimmed to "Department of communication & " (note the now trailing whitespace). This would of course mean that
            # Only some fields are safe to substring; username, idnumber, email, country for sure are not. Where as institution, department, etc wouldn't really matter. At a glance with username's being trimmed should we in the future increase the length of the username field there would be a risk of authentication plugins adding users that already exist because of username concatenation.

            As for the actual issue of accepting leading and trailing whitespace - I lack to see why we would want to accept leading/trailing whitespace but of course that is inconsequential - really this issue is about consistency right?

            My personal opinion at this point is that Moodle code should not be truncating user_info, and should not be trimming it either. I think core should be validating the incoming data to make sure that it meets the requirements we have, and any failure to meet those requirements (field length etc) should result in error/exception.
            Any trimming or concatenation should be done as an entirely separate operation before that validation, on a field by field basis, and really only if we know that incoming data is going not going to meet our requirements and only if we know it is safe to complete such operations (so really only on information only fields).

            Perhaps I've missed something?

            Cheers
            Sam ]
            nebgor Aparup Banerjee made changes -
            Status Waiting for peer review [ 10012 ] Development in progress [ 3 ]
            moodle.com moodle.com made changes -
            Fix Version/s STABLE Sprint 16 [ 11350 ]
            Fix Version/s STABLE Sprint 15 [ 11158 ]
            moodle.com moodle.com made changes -
            Labels netspot partner triaged sprinting
            salvetore Michael de Raadt made changes -
            Labels sprinting netspot partner sprinting triaged
            salvetore Michael de Raadt made changes -
            Fix Version/s STABLE Sprint 17 [ 11550 ]
            Fix Version/s STABLE Sprint 16 [ 11350 ]
            nebgor Aparup Banerjee made changes -
            Link This issue development required addition/change MDL-31714 [ MDL-31714 ]
            nebgor Aparup Banerjee made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            salvetore Michael de Raadt made changes -
            Fix Version/s STABLE Sprint 18 [ 11650 ]
            Fix Version/s STABLE Sprint 17 [ 11550 ]
            nebgor Aparup Banerjee made changes -
            Status Waiting for peer review [ 10012 ] Waiting for integration review [ 10010 ]
            Pull from Repository git://github.com/nebgor/moodle.git
            Integrator stronk7
            stronk7 Eloy Lafuente (stronk7) made changes -
            Currently in integration Yes [ 10041 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Affects Version/s 2.2.1 [ 11456 ]
            Affects Version/s 2.1.4 [ 11452 ]
            Fix Version/s 2.1.5 [ 11553 ]
            Fix Version/s 2.2.2 [ 11552 ]
            salvetore Michael de Raadt made changes -
            Tester rajeshtaneja
            rajeshtaneja Rajesh Taneja made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            rajeshtaneja Rajesh Taneja made changes -
            Status Testing in progress [ 10011 ] Problem during testing [ 10007 ]
            rajeshtaneja Rajesh Taneja made changes -
            Testing Instructions 1) turn on web services under advanced features.
            2) plugins->web services -> external services , create a web service
            3) define in the GUI, a web services function that creates moodle users using 'moodle_user_create_users'
            4) call the web service with a web services test client (some can be cloned from here : git@github.com:moodlehq/sample-ws-clients.git )
            5) check that it can create firstname and lastnames with spaces in it.
            1) turn on web services under advanced features.
            2) plugins->web services -> external services , create a web service
            3) define in the GUI, a web services function that creates moodle users using 'moodle_user_create_users'
            4) call the web service with a web services test client (some can be cloned from here : git@github.com:moodlehq/sample-ws-clients.git )
            5) check that it can create firstname and lastnames with spaces in it.

            Please test MDL-30878 also. Part of this patch was removed for testing MDL-30878.
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Problem during testing [ 10007 ] Integration review in progress [ 10004 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            rajeshtaneja Rajesh Taneja made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            rajeshtaneja Rajesh Taneja made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 01/Mar/12
            stronk7 Eloy Lafuente (stronk7) made changes -
            Affects Version/s STABLE Sprint 15 [ 11158 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Fix Version/s STABLE Sprint 18 [ 11650 ]

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Mar/12