Skip to content

Commit

Permalink
webdav: work-around Milton racy API for creating collections
Browse files Browse the repository at this point in the history
Motivation:

When processing a PUT request, milton checks whether parent paths exist
and creates them if not.  Unfortunately, this process is racy:
concurrent PUTs that target the same missing directory risk concurrent
createCollection method calls with the same target.  If this happens all
but one createCollection call will fail, resulting in the corresponding
PUT requests failing.

The problem is described here:

    miltonio/milton2#114

Modification:

Add a work-around where dCache pretends the createCollection call is
successful, allowing the PUT request to succeed.

Result:

Fix a problem where all but one requests fail, if multiple concurrent
PUT requests have directories in the path that do not already exist.

Target: master
Requires-notes: yes
Requires-book: no
Request: 5.0
Request: 4.2
Request: 4.1
Request: 4.0
Request: 3.2
Patch: https://rb.dcache.org/r/11480/
Acked-by: Olufemi Adeyemi
  • Loading branch information
paulmillar committed Jan 23, 2019
1 parent 517279f commit e8e5aa4
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,46 @@ public void delete()

@Override
public CollectionResource createCollection(String newName)
throws NotAuthorizedException, ConflictException
throws NotAuthorizedException, ConflictException, BadRequestException
{
try {
return _factory.makeDirectory(_attributes, _path.child(newName));
} catch (FileExistsCacheException e) {
/* Milton tries to prevent this from happening by checking if the
* desired child collection already exists; however, this process
* is racy. See:
*
* https://github.com/miltonio/milton2/issues/114
*
* The work-around is somewhat complicated.
*
* If this method is called as part of a MKCOL request then we must
* fail the request with 405 Method Not Allowed. This is not
* supported by Milton API, so we use a work-around.
*
* If this method is called as part of a PUT request then we pretend
* the DcacheResourceFactory#makeDirectory call was successful, if
* there is a directory with that name, otherwise indicate a
* conflict.
*/
String httpMethod = ServletRequest.getRequest().getMethod();
switch (httpMethod) {
case "MKCOL":
throw new MethodNotAllowedException("collection already exists", e, this);

case "PUT":
Resource child = child(newName);
if (!(child instanceof CollectionResource)) {
// This thread lost the race (in Milton), and the winning
// thread created something other than a directory.
throw new ConflictException(this);
}
return (CollectionResource)child;

default:
LOG.error("createCollection called processing unexpected HTTP method: {}", httpMethod);
throw new WebDavException("Unexpected method", e, this);
}
} catch (PermissionDeniedCacheException e) {
throw WebDavExceptions.permissionDenied(this);
} catch (CacheException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ public void process(FilterChain chain, Request request, Response response)
*/
response.setStatus(Response.Status.SC_TEMPORARY_REDIRECT);
response.setLocationHeader(e.getUrl());
} catch (MethodNotAllowedException e) {
responseHandler.respondMethodNotAllowed(e.getResource(), response, request);
} catch (WebDavException e) {
log.warn("Internal server error: {}", e.toString());
responseHandler.respondServerError(request, response, e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/* dCache - http://www.dcache.org/
*
* Copyright (C) 2019 Deutsches Elektronen-Synchrotron
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.dcache.webdav;

import io.milton.resource.Resource;

/**
* Return 405 Method Not Allowed to client.
*/
public class MethodNotAllowedException extends WebDavException
{
public MethodNotAllowedException(String message, Throwable cause, Resource resource)
{
super(message, cause, resource);
}
}

0 comments on commit e8e5aa4

Please sign in to comment.