From 60f8366d3325edbfd2819cb03a9863bd4724c5a1 Mon Sep 17 00:00:00 2001 From: jixinchi <91533837+jixinchi@users.noreply.github.com> Date: Wed, 18 Oct 2023 10:34:50 +0800 Subject: [PATCH] Add DELETE and HEAD methods for CORS and CORS headers for all responses --- README.md | 2 +- .../s3proxy/CrossOriginResourceSharing.java | 2 +- .../java/org/gaul/s3proxy/S3ProxyHandler.java | 94 ++++++++++--------- ...inResourceSharingAllowAllResponseTest.java | 4 +- .../CrossOriginResourceSharingRuleTest.java | 6 ++ 5 files changed, 59 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index 75bb48d..942b2fa 100644 --- a/README.md +++ b/README.md @@ -145,7 +145,7 @@ s3proxy.cors-allow-credential=true ``` CORS cannot be configured per bucket. `s3proxy.cors-allow-all=true` will accept any origin and header. -Actual CORS requests are supported for GET, PUT and POST methods. +Actual CORS requests are supported for GET, PUT, POST, HEAD and DELETE methods. The wiki collects [compatibility notes](https://github.com/gaul/s3proxy/wiki/Storage-backend-compatibility) diff --git a/src/main/java/org/gaul/s3proxy/CrossOriginResourceSharing.java b/src/main/java/org/gaul/s3proxy/CrossOriginResourceSharing.java index b6278e7..264b8fd 100644 --- a/src/main/java/org/gaul/s3proxy/CrossOriginResourceSharing.java +++ b/src/main/java/org/gaul/s3proxy/CrossOriginResourceSharing.java @@ -35,7 +35,7 @@ import org.slf4j.LoggerFactory; public final class CrossOriginResourceSharing { protected static final Collection SUPPORTED_METHODS = - ImmutableList.of("GET", "HEAD", "PUT", "POST"); + ImmutableList.of("GET", "HEAD", "PUT", "POST", "DELETE"); private static final String HEADER_VALUE_SEPARATOR = ", "; private static final String ALLOW_ANY_ORIGIN = "*"; diff --git a/src/main/java/org/gaul/s3proxy/S3ProxyHandler.java b/src/main/java/org/gaul/s3proxy/S3ProxyHandler.java index 3396ebc..d2ca3cf 100644 --- a/src/main/java/org/gaul/s3proxy/S3ProxyHandler.java +++ b/src/main/java/org/gaul/s3proxy/S3ProxyHandler.java @@ -660,26 +660,26 @@ public class S3ProxyHandler { switch (method) { case "DELETE": if (path.length <= 2 || path[2].isEmpty()) { - handleContainerDelete(response, blobStore, path[1]); + handleContainerDelete(request, response, blobStore, path[1]); return; } else if (uploadId != null) { handleAbortMultipartUpload(request, response, blobStore, path[1], path[2], uploadId); return; } else { - handleBlobRemove(response, blobStore, path[1], path[2]); + handleBlobRemove(request, response, blobStore, path[1], path[2]); return; } case "GET": if (uri.equals("/")) { - handleContainerList(response, blobStore); + handleContainerList(request, response, blobStore); return; } else if (path.length <= 2 || path[2].isEmpty()) { if (request.getParameter("acl") != null) { - handleGetContainerAcl(response, blobStore, path[1]); + handleGetContainerAcl(request, response, blobStore, path[1]); return; } else if (request.getParameter("location") != null) { - handleContainerLocation(response); + handleContainerLocation(request, response); return; } else if (request.getParameter("policy") != null) { handleBucketPolicy(blobStore, path[1]); @@ -693,7 +693,7 @@ public class S3ProxyHandler { return; } else { if (request.getParameter("acl") != null) { - handleGetBlobAcl(response, blobStore, path[1], + handleGetBlobAcl(request, response, blobStore, path[1], path[2]); return; } else if (uploadId != null) { @@ -707,7 +707,7 @@ public class S3ProxyHandler { } case "HEAD": if (path.length <= 2 || path[2].isEmpty()) { - handleContainerExists(blobStore, path[1]); + handleContainerExists(request, response, blobStore, path[1]); return; } else { handleBlobMetadata(request, response, blobStore, path[1], @@ -716,7 +716,7 @@ public class S3ProxyHandler { } case "POST": if (request.getParameter("delete") != null) { - handleMultiBlobRemove(response, is, blobStore, path[1]); + handleMultiBlobRemove(request, response, is, blobStore, path[1]); return; } else if (request.getParameter("uploads") != null) { handleInitiateMultipartUpload(request, response, blobStore, @@ -848,15 +848,6 @@ public class S3ProxyHandler { throw new S3Exception(S3ErrorCode.ACCESS_DENIED); } else { String containerName = path[1]; - /* - * Only check access on bucket level. The preflight request - * might be for a PUT, so the object is not yet there. - */ - ContainerAccess access = blobStore.getContainerAccess( - containerName); - if (access == ContainerAccess.PRIVATE) { - throw new S3Exception(S3ErrorCode.ACCESS_DENIED); - } handleOptionsBlob(request, response, blobStore, containerName); return; } @@ -868,15 +859,16 @@ public class S3ProxyHandler { throw new S3Exception(S3ErrorCode.NOT_IMPLEMENTED); } - private void handleGetContainerAcl(HttpServletResponse response, - BlobStore blobStore, String containerName) - throws IOException, S3Exception { + private void handleGetContainerAcl(HttpServletRequest request, + HttpServletResponse response, BlobStore blobStore, + String containerName) throws IOException, S3Exception { if (!blobStore.containerExists(containerName)) { throw new S3Exception(S3ErrorCode.NO_SUCH_BUCKET); } ContainerAccess access = blobStore.getContainerAccess(containerName); response.setCharacterEncoding(UTF_8); + addCorsResponseHeader(request, response); try (Writer writer = response.getWriter()) { response.setContentType(XML_CONTENT_TYPE); XMLStreamWriter xml = xmlOutputFactory.createXMLStreamWriter( @@ -967,14 +959,16 @@ public class S3ProxyHandler { } blobStore.setContainerAccess(containerName, access); + addCorsResponseHeader(request, response); } - private void handleGetBlobAcl(HttpServletResponse response, - BlobStore blobStore, String containerName, - String blobName) throws IOException { + private void handleGetBlobAcl(HttpServletRequest request, + HttpServletResponse response, BlobStore blobStore, + String containerName, String blobName) throws IOException { BlobAccess access = blobStore.getBlobAccess(containerName, blobName); response.setCharacterEncoding(UTF_8); + addCorsResponseHeader(request, response); try (Writer writer = response.getWriter()) { response.setContentType(XML_CONTENT_TYPE); XMLStreamWriter xml = xmlOutputFactory.createXMLStreamWriter( @@ -1066,6 +1060,7 @@ public class S3ProxyHandler { } blobStore.setBlobAccess(containerName, blobName, access); + addCorsResponseHeader(request, response); } /** Map XML ACLs to a canned policy if an exact tranformation exists. */ @@ -1105,11 +1100,12 @@ public class S3ProxyHandler { } } - private void handleContainerList(HttpServletResponse response, - BlobStore blobStore) throws IOException { + private void handleContainerList(HttpServletRequest request, + HttpServletResponse response, BlobStore blobStore) throws IOException { PageSet buckets = blobStore.list(); response.setCharacterEncoding(UTF_8); + addCorsResponseHeader(request, response); try (Writer writer = response.getWriter()) { response.setContentType(XML_CONTENT_TYPE); XMLStreamWriter xml = xmlOutputFactory.createXMLStreamWriter( @@ -1148,9 +1144,10 @@ public class S3ProxyHandler { } } - private void handleContainerLocation(HttpServletResponse response) - throws IOException { + private void handleContainerLocation(HttpServletRequest request, + HttpServletResponse response) throws IOException { response.setCharacterEncoding(UTF_8); + addCorsResponseHeader(request, response); try (Writer writer = response.getWriter()) { response.setContentType(XML_CONTENT_TYPE); XMLStreamWriter xml = xmlOutputFactory.createXMLStreamWriter( @@ -1191,6 +1188,7 @@ public class S3ProxyHandler { container); response.setCharacterEncoding(UTF_8); + addCorsResponseHeader(request, response); try (Writer writer = response.getWriter()) { response.setContentType(XML_CONTENT_TYPE); XMLStreamWriter xml = xmlOutputFactory.createXMLStreamWriter( @@ -1251,11 +1249,13 @@ public class S3ProxyHandler { } } - private static void handleContainerExists(BlobStore blobStore, + private void handleContainerExists(HttpServletRequest request, + HttpServletResponse response, BlobStore blobStore, String containerName) throws IOException, S3Exception { if (!blobStore.containerExists(containerName)) { throw new S3Exception(S3ErrorCode.NO_SUCH_BUCKET); } + addCorsResponseHeader(request, response); } private void handleContainerCreate(HttpServletRequest request, @@ -1331,11 +1331,12 @@ public class S3ProxyHandler { } response.addHeader(HttpHeaders.LOCATION, "/" + containerName); + addCorsResponseHeader(request, response); } - private static void handleContainerDelete(HttpServletResponse response, - BlobStore blobStore, String containerName) - throws IOException, S3Exception { + private void handleContainerDelete(HttpServletRequest request, + HttpServletResponse response, BlobStore blobStore, + String containerName) throws IOException, S3Exception { if (!blobStore.containerExists(containerName)) { throw new S3Exception(S3ErrorCode.NO_SUCH_BUCKET); } @@ -1354,6 +1355,7 @@ public class S3ProxyHandler { throw new S3Exception(S3ErrorCode.BUCKET_NOT_EMPTY); } + addCorsResponseHeader(request, response); response.setStatus(HttpServletResponse.SC_NO_CONTENT); } @@ -1554,15 +1556,18 @@ public class S3ProxyHandler { } } - private static void handleBlobRemove(HttpServletResponse response, - BlobStore blobStore, String containerName, - String blobName) throws IOException, S3Exception { + private void handleBlobRemove(HttpServletRequest request, + HttpServletResponse response, BlobStore blobStore, + String containerName, String blobName) + throws IOException, S3Exception { blobStore.removeBlob(containerName, blobName); + addCorsResponseHeader(request, response); response.sendError(HttpServletResponse.SC_NO_CONTENT); } - private void handleMultiBlobRemove(HttpServletResponse response, - InputStream is, BlobStore blobStore, String containerName) + private void handleMultiBlobRemove(HttpServletRequest request, + HttpServletResponse response, InputStream is, + BlobStore blobStore, String containerName) throws IOException, S3Exception { DeleteMultipleObjectsRequest dmor = mapper.readValue( is, DeleteMultipleObjectsRequest.class); @@ -1579,6 +1584,7 @@ public class S3ProxyHandler { blobStore.removeBlobs(containerName, blobNames); response.setCharacterEncoding(UTF_8); + addCorsResponseHeader(request, response); try (Writer writer = response.getWriter()) { response.setContentType(XML_CONTENT_TYPE); XMLStreamWriter xml = xmlOutputFactory.createXMLStreamWriter( @@ -1605,7 +1611,7 @@ public class S3ProxyHandler { } } - private static void handleBlobMetadata(HttpServletRequest request, + private void handleBlobMetadata(HttpServletRequest request, HttpServletResponse response, BlobStore blobStore, String containerName, String blobName) throws IOException, S3Exception { @@ -1650,6 +1656,7 @@ public class S3ProxyHandler { response.setStatus(HttpServletResponse.SC_OK); addMetadataToResponse(request, response, metadata); + addCorsResponseHeader(request, response); } private void handleOptionsBlob(HttpServletRequest request, @@ -1871,6 +1878,7 @@ public class S3ProxyHandler { BlobMetadata blobMetadata = blobStore.blobMetadata(destContainerName, destBlobName); response.setCharacterEncoding(UTF_8); + addCorsResponseHeader(request, response); try (Writer writer = response.getWriter()) { response.setContentType(XML_CONTENT_TYPE); XMLStreamWriter xml = xmlOutputFactory.createXMLStreamWriter( @@ -2200,6 +2208,7 @@ public class S3ProxyHandler { } response.setCharacterEncoding(UTF_8); + addCorsResponseHeader(request, response); try (Writer writer = response.getWriter()) { response.setContentType(XML_CONTENT_TYPE); XMLStreamWriter xml = xmlOutputFactory.createXMLStreamWriter( @@ -2217,8 +2226,6 @@ public class S3ProxyHandler { } catch (XMLStreamException xse) { throw new IOException(xse); } - - addCorsResponseHeader(request, response); } private void handleCompleteMultipartUpload(HttpServletRequest request, @@ -2325,6 +2332,7 @@ public class S3ProxyHandler { } response.setCharacterEncoding(UTF_8); + addCorsResponseHeader(request, response); try (PrintWriter writer = response.getWriter()) { response.setStatus(HttpServletResponse.SC_OK); response.setContentType(XML_CONTENT_TYPE); @@ -2388,8 +2396,6 @@ public class S3ProxyHandler { } catch (XMLStreamException xse) { throw new IOException(xse); } - - addCorsResponseHeader(request, response); } private void handleAbortMultipartUpload(HttpServletRequest request, @@ -2455,6 +2461,7 @@ public class S3ProxyHandler { String encodingType = request.getParameter("encoding-type"); response.setCharacterEncoding(UTF_8); + addCorsResponseHeader(request, response); try (Writer writer = response.getWriter()) { response.setContentType(XML_CONTENT_TYPE); XMLStreamWriter xml = xmlOutputFactory.createXMLStreamWriter( @@ -2512,8 +2519,6 @@ public class S3ProxyHandler { } catch (XMLStreamException xse) { throw new IOException(xse); } - - addCorsResponseHeader(request, response); } private void handleCopyPart(HttpServletRequest request, @@ -2689,6 +2694,7 @@ public class S3ProxyHandler { } response.setCharacterEncoding(UTF_8); + addCorsResponseHeader(request, response); try (Writer writer = response.getWriter()) { response.setContentType(XML_CONTENT_TYPE); XMLStreamWriter xml = xmlOutputFactory.createXMLStreamWriter( @@ -2707,8 +2713,6 @@ public class S3ProxyHandler { } catch (XMLStreamException xse) { throw new IOException(xse); } - - addCorsResponseHeader(request, response); } private void handleUploadPart(HttpServletRequest request, diff --git a/src/test/java/org/gaul/s3proxy/CrossOriginResourceSharingAllowAllResponseTest.java b/src/test/java/org/gaul/s3proxy/CrossOriginResourceSharingAllowAllResponseTest.java index 92593f1..e48aed6 100644 --- a/src/test/java/org/gaul/s3proxy/CrossOriginResourceSharingAllowAllResponseTest.java +++ b/src/test/java/org/gaul/s3proxy/CrossOriginResourceSharingAllowAllResponseTest.java @@ -157,7 +157,7 @@ public final class CrossOriginResourceSharingAllowAllResponseTest { HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS)).isTrue(); assertThat(response.getFirstHeader( HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS).getValue()) - .isEqualTo("GET, HEAD, PUT, POST"); + .isEqualTo("GET, HEAD, PUT, POST, DELETE"); assertThat(response.containsHeader( HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS)).isTrue(); assertThat(response.getFirstHeader( @@ -181,7 +181,7 @@ public final class CrossOriginResourceSharingAllowAllResponseTest { HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS)).isTrue(); assertThat(response.getFirstHeader( HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS).getValue()) - .isEqualTo("GET, HEAD, PUT, POST"); + .isEqualTo("GET, HEAD, PUT, POST, DELETE"); } @Test diff --git a/src/test/java/org/gaul/s3proxy/CrossOriginResourceSharingRuleTest.java b/src/test/java/org/gaul/s3proxy/CrossOriginResourceSharingRuleTest.java index 6c147af..3d3d1c6 100644 --- a/src/test/java/org/gaul/s3proxy/CrossOriginResourceSharingRuleTest.java +++ b/src/test/java/org/gaul/s3proxy/CrossOriginResourceSharingRuleTest.java @@ -107,6 +107,12 @@ public final class CrossOriginResourceSharingRuleTest { probe = "POST"; assertThat(corsAll.isMethodAllowed(probe)) .as("check '%s' as method", probe).isTrue(); + probe = "HEAD"; + assertThat(corsAll.isMethodAllowed(probe)) + .as("check '%s' as method", probe).isTrue(); + probe = "DELETE"; + assertThat(corsAll.isMethodAllowed(probe)) + .as("check '%s' as method", probe).isTrue(); } @Test