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
    • Rank:
      920

      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.

        Issue Links

          Activity

          Ashley Holman created issue -
          Jérôme Mouneyrac made changes -
          Field Original Value New Value
          Fix Version/s 2.0 [ 10122 ]
          Martin Dougiamas made changes -
          Fix Version/s 2.0.1 [ 10420 ]
          Fix Version/s 2.0 [ 10122 ]
          Jérôme Mouneyrac made changes -
          Fix Version/s STABLE backlog [ 10463 ]
          Fix Version/s 2.0.1 [ 10420 ]
          Eloy Lafuente (stronk7) made changes -
          Labels triaged
          Martin Dougiamas made changes -
          Workflow jira [ 40001 ] MDL Workflow [ 47009 ]
          Martin Dougiamas made changes -
          Workflow MDL Workflow [ 47009 ] MDL Full Workflow [ 75221 ]
          Ashley Holman made changes -
          Labels triaged netspot triaged
          Michael Blake made changes -
          Labels netspot triaged netspot partner triaged
          Jérôme Mouneyrac made changes -
          Priority Minor [ 4 ] Critical [ 2 ]
          Jérôme Mouneyrac made changes -
          Link This issue will help resolve MDL-29276 [ MDL-29276 ]
          Filter Manager made changes -
          Fix Version/s STABLE Sprint 14 [ 11050 ]
          Fix Version/s STABLE backlog [ 10463 ]
          Assignee Jerome Mouneyrac [ jerome ] Aparup Banerjee [ nebgor ]
          Jérôme Mouneyrac made changes -
          Link This issue has a non-specific relationship to MDL-29374 [ MDL-29374 ]
          Aparup Banerjee made changes -
          Status Open [ 1 ] Development in progress [ 3 ]
          Aparup Banerjee made changes -
          Link This issue has been marked as being related by MDL-29582 [ MDL-29582 ]
          Aparup Banerjee made changes -
          Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
          Peer reviewer rwijaya
          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.
          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 ]
          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.
          Aparup Banerjee made changes -
          Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
          Rossiani Wijaya made changes -
          Status Development in progress [ 3 ] Peer review in progress [ 10013 ]
          Rossiani Wijaya made changes -
          Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
          Rossiani Wijaya made changes -
          Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
          Eloy Lafuente (stronk7) made changes -
          Status Waiting for integration review [ 10010 ] Reopened [ 4 ]
          moodle.com made changes -
          Affects Version/s STABLE Sprint 15 [ 11158 ]
          Affects Version/s 2.0 [ 10122 ]
          moodle.com made changes -
          Fix Version/s STABLE Sprint 15 [ 11158 ]
          Fix Version/s STABLE Sprint 14 [ 11050 ]
          Aparup Banerjee made changes -
          Status Reopened [ 4 ] Development in progress [ 3 ]
          Aparup Banerjee made changes -
          Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
          Peer reviewer rwijaya 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 ]
          Aparup Banerjee made changes -
          Status Waiting for peer review [ 10012 ] Development in progress [ 3 ]
          moodle.com made changes -
          Fix Version/s STABLE Sprint 16 [ 11350 ]
          Fix Version/s STABLE Sprint 15 [ 11158 ]
          moodle.com made changes -
          Labels netspot partner triaged sprinting
          Michael de Raadt made changes -
          Labels sprinting netspot partner sprinting triaged
          Michael de Raadt made changes -
          Fix Version/s STABLE Sprint 17 [ 11550 ]
          Fix Version/s STABLE Sprint 16 [ 11350 ]
          Aparup Banerjee made changes -
          Link This issue development required addition/change MDL-31714 [ MDL-31714 ]
          Aparup Banerjee made changes -
          Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
          Michael de Raadt made changes -
          Fix Version/s STABLE Sprint 18 [ 11650 ]
          Fix Version/s STABLE Sprint 17 [ 11550 ]
          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
          Eloy Lafuente (stronk7) made changes -
          Currently in integration Yes [ 10041 ]
          Eloy Lafuente (stronk7) made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          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 ]
          Michael de Raadt made changes -
          Tester rajeshtaneja
          Rajesh Taneja made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Rajesh Taneja made changes -
          Status Testing in progress [ 10011 ] Problem during testing [ 10007 ]
          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.
          Eloy Lafuente (stronk7) made changes -
          Status Problem during testing [ 10007 ] Integration review in progress [ 10004 ]
          Eloy Lafuente (stronk7) made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Rajesh Taneja made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Rajesh Taneja made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          Eloy Lafuente (stronk7) made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes [ 10041 ]
          Integration date 01/Mar/12
          Eloy Lafuente (stronk7) made changes -
          Affects Version/s STABLE Sprint 15 [ 11158 ]
          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: