Skip to content

Commit

Permalink
webdav: avoid throwing any exception when listing a directory for PRO…
Browse files Browse the repository at this point in the history
…PFIND

Motivation:

The PROPFIND request (optionally) lists items recursively up to some
specified depth.

Currently, if any children of the targeted directory is unreadable
(permission denied) then the entire PROPFIND response is discarded and
an HTML page is returned instead.

There is a similar response if there is some temporary glitch listing
a directory: the HTML page is shown instead.

This is bad because:

  1.  The successfully gathered information is not provided to the
      client.  The presence of any "problematic" subdirectory prevents
      *any* information being returned, rendering any navigation
      impossible (e.g., if the user is not allowed to list the /private
      directory then, depending of the client-supplied Depth value, the
      / directory cannot be listed).

  2.  The error is reported at the targeted request level, suggesting
      the problem is with the PROPFIND target, where the problem may
      be with one of the subdirectories.

  3.  The returned HTML does not conform to the expected response to a
      PROPFIND request.  This makes it impossible for any WebDAV client
      to process the request.

Modification:

Update the getChildren method to return `null` if there is a problem.
Although not ideal, this allows milton to collect information about
other directories.

Result:

A PROPFIND yields useful information, even if the user is not allowed to
list one of the targeted subdirectory.

Target: master
Request: 4.2
Request: 4.1
Request: 4.0
Request: 3.2
Patch: https://rb.dcache.org/r/11160/
Acked-by: Albert Rossi
  • Loading branch information
paulmillar committed Sep 6, 2018
1 parent a84d53f commit 49e4bf9
Showing 1 changed file with 11 additions and 2 deletions.
Expand Up @@ -113,9 +113,18 @@ public List<? extends Resource> getChildren()
} catch (FileNotFoundCacheException e) {
return Collections.emptyList();
} catch (PermissionDeniedCacheException e) {
throw WebDavExceptions.permissionDenied(e.getMessage(), e, this);
// Theoretically, we should throw NotAuthorizedException here. The
// problem is that Milton reacts badly to this, and aborts the whole
// PROPFIND request, even if the affected directory is not the primary
// one. Milton accepts a null response as equivalent to
// Collections.emptyList()
return null;
} catch (CacheException | InterruptedException e) {
throw new WebDavException(e.getMessage(), e, this);
// We currently have no way to indicate a temporary failure for this
// directory and throwing any kind of exception will abort the whole
// PROPFIND request; therefore, we return null. Milton accepts a
// null response as equivalent to Collections.emptyList()
return null;
}
}

Expand Down

0 comments on commit 49e4bf9

Please sign in to comment.