Moodle
  1. Moodle
  2. MDL-37014

Webdav repositories not working for some servers

    Details

    • Testing Instructions:
      Hide
      1. set up a webdav repository
      2. create a forum post
      3. add an attachment to the forum, from the webdav repository
        1. confirm you can navigate to subfolders and list all files in those subfolders
        2. confirm that any files selected download successfully
      Show
      set up a webdav repository create a forum post add an attachment to the forum, from the webdav repository confirm you can navigate to subfolders and list all files in those subfolders confirm that any files selected download successfully
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull Master Branch:
      MDL-37014_webdav_folders

      Description

      I have a client whose webdav server returns the full server url in the 'href' element of each file & folder when using the 'PROPFIND' (ls) call.

      This seems to cause all sorts of problems with the webdav repository:

      Stripping the server URL from the 'href' attribute, before returning it to the filepicker seems to solve all of these problems (and won't cause problems on servers that were previously working, as they will not have a URL to strip out).

      I will attach a patch in a moment.

        Gliffy Diagrams

          Activity

          Hide
          Davo Smith added a comment -

          Note: I cannot include the details of the webdav server that is not working with the webdav repository, but it is running on Ajaxexporer ( http://ajaxplorer.info/ ) and was accessed via https, basic authentication and port 443.

          Show
          Davo Smith added a comment - Note: I cannot include the details of the webdav server that is not working with the webdav repository, but it is running on Ajaxexporer ( http://ajaxplorer.info/ ) and was accessed via https, basic authentication and port 443.
          Hide
          Frédéric Massart added a comment -

          Hi Davo,

          I think your patch is good, and would probably solve the case you are experiencing without damaging other users' experience. Is there a specific setting in the WebDAV server that we could you use to replicate this behaviour? That'd be handy for testing.

          Also, before you submit for integration, could you consider the following:

          • Perhaps adding a comment explaining why you're stripping out the URL of the WebDAV server would help understanding this line in the future.
          • Could you make use of preg_quote() when adding the host in the regex pattern?
          • This should ideally be backported, can I ask you to provide 2.2, 2.3, 2.4 and master branches? The cherry-pick would probably be fine but I think integrators prefer when you have created separated branch for each.

          Thanks!

          Show
          Frédéric Massart added a comment - Hi Davo, I think your patch is good, and would probably solve the case you are experiencing without damaging other users' experience. Is there a specific setting in the WebDAV server that we could you use to replicate this behaviour? That'd be handy for testing. Also, before you submit for integration, could you consider the following: Perhaps adding a comment explaining why you're stripping out the URL of the WebDAV server would help understanding this line in the future. Could you make use of preg_quote() when adding the host in the regex pattern? This should ideally be backported, can I ask you to provide 2.2, 2.3, 2.4 and master branches? The cherry-pick would probably be fine but I think integrators prefer when you have created separated branch for each. Thanks!
          Hide
          Michael de Raadt added a comment -

          I've triaged this issue to the Stable backlog, but I'll leave this with you, Davo.

          Show
          Michael de Raadt added a comment - I've triaged this issue to the Stable backlog, but I'll leave this with you, Davo.
          Hide
          Davo Smith added a comment -
          • I've added a comment to explain the change I've made
          • I've added preg_quote
          • I've cherry-picked to different branches (and fixed the merge conflict on the 22 branch)

          I don't know exactly how the particular webdav repository is configured, other than it is running on Ajaxexplorer (as mentioned above).

          Show
          Davo Smith added a comment - I've added a comment to explain the change I've made I've added preg_quote I've cherry-picked to different branches (and fixed the merge conflict on the 22 branch) I don't know exactly how the particular webdav repository is configured, other than it is running on Ajaxexplorer (as mentioned above).
          Hide
          Dan Poltawski added a comment -

          Integrated to master, 24, 23 and 22. Thanks Davo.

          Show
          Dan Poltawski added a comment - Integrated to master, 24, 23 and 22. Thanks Davo.
          Hide
          Rajesh Taneja added a comment -

          Thanks for fixing this Davo.

          I used dropdav for testing this and was able to navigate though files in sub-folders and was able to attach and download them.

          Show
          Rajesh Taneja added a comment - Thanks for fixing this Davo. I used dropdav for testing this and was able to navigate though files in sub-folders and was able to attach and download them.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Changes are now upstream, thanks for your collaboration!

          If you are going to have any celebration next days, enjoy with your gang, if not, too!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Changes are now upstream, thanks for your collaboration! If you are going to have any celebration next days, enjoy with your gang, if not, too! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: