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

Web sevices: core_get_course_contents errors with sketchy URLs

    XMLWordPrintable

Details

    • MOODLE_310_STABLE, MOODLE_38_STABLE, MOODLE_39_STABLE, MOODLE_400_STABLE
    • MOODLE_38_STABLE, MOODLE_39_STABLE
    • MDL-69758-m39
    • MDL-69758-master
    • Hide
      • The easiest way to call this service is using the Moodle mobile app. So, ensure your website is set up to allow the app.
      • Choose a test user who will be able to use the app.

      BEFORE patch:

      1. Logged in as admin, go to a course that your test user is enrolled in, or enrol them in a new course if required.
      2. Add a new URL to the course with the following (probably valid, and definitely working, but stupid) URL: http://www.open.ac.uk/libraryservices//////beingdigital/
      3. In the Moodle app, log in as the test user and go to the course. Pull down from the top of the screen to force it to reload the course (just in case you already had it cached).
      4. Note: At time of writing, the URL still does not work if you click on it from the app. This is expected.
      5. Look at the web server error log (with Apache web server this is called error.log by default and is in the 'logs' directory, but it depends on configuration).
      6. You should see error from the time of your Moodle app request with text similar to the following:
        • PHP Warning: count(): Parameter must be an array or an object that implements Countable in [FILESYSTEM PATH] externallib.php
        • PHP Warning: Invalid argument supplied for foreach() in [FILESYSTEM PATH]/externallib.php on line 329

      AFTER patch:

      1. Repeat steps 1-5
      2. Look at the web server error log (with Apache web server this is called error.log by default and is in the 'logs' directory, but it depends on configuration).
      3. You should NOT see the error that was being shown before the patch.
      Show
      The easiest way to call this service is using the Moodle mobile app. So, ensure your website is set up to allow the app. Choose a test user who will be able to use the app. BEFORE patch: Logged in as admin, go to a course that your test user is enrolled in, or enrol them in a new course if required. Add a new URL to the course with the following (probably valid, and definitely working, but stupid) URL: http://www.open.ac.uk/libraryservices//////beingdigital/ In the Moodle app, log in as the test user and go to the course. Pull down from the top of the screen to force it to reload the course (just in case you already had it cached). Note: At time of writing, the URL still does not work if you click on it from the app. This is expected. Look at the web server error log (with Apache web server this is called error.log by default and is in the 'logs' directory, but it depends on configuration). You should see error from the time of your Moodle app request with text similar to the following: PHP Warning: count(): Parameter must be an array or an object that implements Countable in [FILESYSTEM PATH] externallib.php PHP Warning: Invalid argument supplied for foreach() in [FILESYSTEM PATH] /externallib.php on line 329 AFTER patch: Repeat steps 1-5 Look at the web server error log (with Apache web server this is called error.log by default and is in the 'logs' directory, but it depends on configuration). You should NOT see the error that was being shown before the patch.

    Description

      We saw errors like this in the server logs:

      PHP Warning: count(): Parameter must be an array or an object that implements Countable in /srv/learn2.open.ac.uk/www/html/course/externallib.php on line 324

      This is after the following code:

                                  $getcontentfunction = $cm->modname.'_export_contents';
                                  if (function_exists($getcontentfunction)) {
                                      $contents = $getcontentfunction($cm, $baseurl);
      

      The _export_contents function is expected to return an array. If it does not, you get these warnings.

      The cause turned out to be in url_export_contents:

          $isurl = clean_param($fullurl, PARAM_URL);
          if (empty($isurl)) {
              return null;
          }
      

      And this was happening because the URL was set to:

      http://www.open.ac.uk/libraryservices//beingdigital/

      Note the double slash - sketchy, but it works. (The URL also works with only a single slash there.)

      From the mobile app (where this webservice is called), clicking on the link doesn't work - it does nothing.

      If you change the URL to a single slash then it doesn't cause this error.

      There is clearly (imo) a bug in url_export_contents - instead of returning null, it should return an empty array if there's an error. I looked at the other _export_contents functions and didn't find any other examples of the same problem.

      However, is it correct that clean_param returns empty in this case? I had a quick look at the URI RFC and don't think it forbids empty path segments... I'm going to leave that for a separate issue if we need to fix it... Likewise, I wonder if mod_url should validate the URL against the same function on the settings form? Maybe hard because of placeholders or something?

      Attachments

        Activity

          People

            quen Sam Marshall
            quen Sam Marshall
            Tim Hunt Tim Hunt
            Eloy Lafuente (stronk7) Eloy Lafuente (stronk7)
            Anna Carissa Sadia Anna Carissa Sadia
            David Woloszyn, Huong Nguyen, Jake Dallimore, Michael Hawkins, Stevani Andolo, Juan Leyva, David Woloszyn, Huong Nguyen, Jake Dallimore, Michael Hawkins, Stevani Andolo
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:
              9/Nov/20

              Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 2 hours, 5 minutes
                2h 5m