From e8e5aa4bd881494e2bae29529aaa62be5991b3d6 Mon Sep 17 00:00:00 2001 From: Paul Millar Date: Fri, 4 Jan 2019 12:17:03 +0100 Subject: [PATCH] webdav: work-around Milton racy API for creating collections 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: https://github.com/miltonio/milton2/issues/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 --- .../webdav/DcacheDirectoryResource.java | 38 ++++++++++++++++++- .../dcache/webdav/DcacheStandardFilter.java | 2 + .../webdav/MethodNotAllowedException.java | 31 +++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 modules/dcache-webdav/src/main/java/org/dcache/webdav/MethodNotAllowedException.java diff --git a/modules/dcache-webdav/src/main/java/org/dcache/webdav/DcacheDirectoryResource.java b/modules/dcache-webdav/src/main/java/org/dcache/webdav/DcacheDirectoryResource.java index 855884aad0e..46ec44060bc 100644 --- a/modules/dcache-webdav/src/main/java/org/dcache/webdav/DcacheDirectoryResource.java +++ b/modules/dcache-webdav/src/main/java/org/dcache/webdav/DcacheDirectoryResource.java @@ -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) { diff --git a/modules/dcache-webdav/src/main/java/org/dcache/webdav/DcacheStandardFilter.java b/modules/dcache-webdav/src/main/java/org/dcache/webdav/DcacheStandardFilter.java index 8ab3132f775..ee376c2b222 100644 --- a/modules/dcache-webdav/src/main/java/org/dcache/webdav/DcacheStandardFilter.java +++ b/modules/dcache-webdav/src/main/java/org/dcache/webdav/DcacheStandardFilter.java @@ -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()); diff --git a/modules/dcache-webdav/src/main/java/org/dcache/webdav/MethodNotAllowedException.java b/modules/dcache-webdav/src/main/java/org/dcache/webdav/MethodNotAllowedException.java new file mode 100644 index 00000000000..80089aef0bf --- /dev/null +++ b/modules/dcache-webdav/src/main/java/org/dcache/webdav/MethodNotAllowedException.java @@ -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 . + */ +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); + } +}