diff --git a/src/main/java/ninja/S3Dispatcher.java b/src/main/java/ninja/S3Dispatcher.java
index cb9cd60..cb492a1 100644
--- a/src/main/java/ninja/S3Dispatcher.java
+++ b/src/main/java/ninja/S3Dispatcher.java
@@ -18,6 +18,8 @@
import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.handler.codec.http.HttpMethod;
import io.netty.handler.codec.http.HttpResponseStatus;
+import ninja.errors.S3ErrorCode;
+import ninja.errors.S3ErrorSynthesizer;
import ninja.queries.S3QuerySynthesizer;
import sirius.kernel.async.CallContext;
import sirius.kernel.commons.Callback;
@@ -97,6 +99,9 @@ private static class S3Request {
@Part
private AwsHashCalculator hashCalculator;
+ @Part
+ private S3ErrorSynthesizer errorSynthesizer;
+
@ConfigValue("storage.multipartDir")
private String multipartDir;
@@ -293,9 +298,12 @@ private void forwardQueryToSynthesizer(WebContext ctx, S3Request request) {
if (synthesizer != null) {
synthesizer.processQuery(ctx, request.bucket, request.key, request.query);
} else {
- // todo: synthesize better error repsonse, see #123
Log.BACKGROUND.WARN("Received unknown query '%s'.", request.query);
- ctx.respondWith().error(HttpResponseStatus.SERVICE_UNAVAILABLE);
+ errorSynthesizer.synthesiseError(ctx,
+ request.bucket,
+ request.key,
+ S3ErrorCode.InvalidRequest,
+ String.format("Received unknown query '%s'.", request.query));
}
}
@@ -325,11 +333,11 @@ private String getAuthHash(WebContext ctx) {
/**
* Writes an API error to the log
*/
- private void signalObjectError(WebContext ctx, HttpResponseStatus status, String message) {
+ private void signalObjectError(WebContext ctx, String bucket, String key, S3ErrorCode errorCode, String message) {
if (ctx.getRequest().method() == HEAD) {
- ctx.respondWith().status(status);
+ ctx.respondWith().status(errorCode.getHttpStatusCode());
} else {
- ctx.respondWith().error(status, message);
+ errorSynthesizer.synthesiseError(ctx, bucket, key, errorCode, message);
}
log.log(ctx.getRequest().method().name(),
message + " - " + ctx.getRequestedURI(),
@@ -398,7 +406,7 @@ private void outputOwnerInfo(XMLStructuredOutput out, String name) {
private void bucket(WebContext ctx, String bucketName) {
Bucket bucket = storage.getBucket(bucketName);
- if (!objectCheckAuth(ctx, bucket)) {
+ if (!objectCheckAuth(ctx, bucket, null)) {
return;
}
@@ -409,13 +417,13 @@ private void bucket(WebContext ctx, String bucketName) {
signalObjectSuccess(ctx);
ctx.respondWith().status(HttpResponseStatus.OK);
} else {
- signalObjectError(ctx, HttpResponseStatus.NOT_FOUND, ERROR_BUCKET_DOES_NOT_EXIST);
+ signalObjectError(ctx, bucketName, null, S3ErrorCode.NoSuchBucket, ERROR_BUCKET_DOES_NOT_EXIST);
}
} else if (GET == method) {
if (bucket.exists()) {
listObjects(ctx, bucket);
} else {
- signalObjectError(ctx, HttpResponseStatus.NOT_FOUND, ERROR_BUCKET_DOES_NOT_EXIST);
+ signalObjectError(ctx, bucketName, null, S3ErrorCode.NoSuchBucket, ERROR_BUCKET_DOES_NOT_EXIST);
}
} else if (DELETE == method) {
bucket.delete();
@@ -510,10 +518,10 @@ private void writeObject(WebContext ctx, String bucketName, String objectId, Inp
private boolean checkObjectRequest(WebContext ctx, Bucket bucket, String id) {
if (Strings.isEmpty(id)) {
- signalObjectError(ctx, HttpResponseStatus.NOT_FOUND, "Please provide an object id");
+ signalObjectError(ctx, bucket.getName(), id, S3ErrorCode.NoSuchKey, "Please provide an object id.");
return false;
}
- if (!objectCheckAuth(ctx, bucket)) {
+ if (!objectCheckAuth(ctx, bucket, id)) {
return false;
}
@@ -521,22 +529,24 @@ private boolean checkObjectRequest(WebContext ctx, Bucket bucket, String id) {
if (storage.isAutocreateBuckets()) {
bucket.create();
} else {
- signalObjectError(ctx, HttpResponseStatus.NOT_FOUND, ERROR_BUCKET_DOES_NOT_EXIST);
+ signalObjectError(ctx, bucket.getName(), id, S3ErrorCode.NoSuchBucket, ERROR_BUCKET_DOES_NOT_EXIST);
return false;
}
}
return true;
}
- private boolean objectCheckAuth(WebContext ctx, Bucket bucket) {
+ private boolean objectCheckAuth(WebContext ctx, Bucket bucket, String key) {
String hash = getAuthHash(ctx);
if (hash != null) {
String expectedHash = hashCalculator.computeHash(ctx, "");
String alternativeHash = hashCalculator.computeHash(ctx, "/s3");
if (!expectedHash.equals(hash) && !alternativeHash.equals(hash)) {
- ctx.respondWith()
- .error(HttpResponseStatus.UNAUTHORIZED,
- Strings.apply("Invalid Hash (Expected: %s, Found: %s)", expectedHash, hash));
+ errorSynthesizer.synthesiseError(ctx,
+ bucket.getName(),
+ key,
+ S3ErrorCode.BadDigest,
+ Strings.apply("Invalid Hash (Expected: %s, Found: %s)", expectedHash, hash));
log.log(ctx.getRequest().method().name(),
ctx.getRequestedURI(),
APILog.Result.REJECTED,
@@ -545,7 +555,11 @@ private boolean objectCheckAuth(WebContext ctx, Bucket bucket) {
}
}
if (bucket.isPrivate() && !ctx.get("noAuth").isFilled() && hash == null) {
- ctx.respondWith().error(HttpResponseStatus.UNAUTHORIZED, "Authentication required");
+ errorSynthesizer.synthesiseError(ctx,
+ bucket.getName(),
+ key,
+ S3ErrorCode.AccessDenied,
+ "Authentication required");
log.log(ctx.getRequest().method().name(),
ctx.getRequestedURI(),
APILog.Result.REJECTED,
@@ -599,7 +613,7 @@ private void putObject(WebContext ctx, Bucket bucket, String id, InputStreamHand
throws IOException {
StoredObject object = bucket.getObject(id);
if (inputStream == null) {
- signalObjectError(ctx, HttpResponseStatus.BAD_REQUEST, "No content posted");
+ signalObjectError(ctx, bucket.getName(), id, S3ErrorCode.IncompleteBody, "No content posted");
return;
}
try (FileOutputStream out = new FileOutputStream(object.getFile())) {
@@ -613,7 +627,9 @@ private void putObject(WebContext ctx, Bucket bucket, String id, InputStreamHand
if (properties.containsKey("Content-MD5") && !md5.equals(contentMd5)) {
object.delete();
signalObjectError(ctx,
- HttpResponseStatus.BAD_REQUEST,
+ bucket.getName(),
+ id,
+ S3ErrorCode.BadDigest,
Strings.apply("Invalid MD5 checksum (Input: %s, Expected: %s)", contentMd5, md5));
return;
}
@@ -653,19 +669,31 @@ private String etag(String etag) {
private void copyObject(WebContext ctx, Bucket bucket, String id, String copy) throws IOException {
StoredObject object = bucket.getObject(id);
if (!copy.contains(PATH_DELIMITER)) {
- signalObjectError(ctx, HttpResponseStatus.BAD_REQUEST, "Source must contain '/'");
+ signalObjectError(ctx,
+ null,
+ null,
+ S3ErrorCode.InvalidRequest,
+ String.format("Source '%s' must contain '/'", copy));
return;
}
String srcBucketName = copy.substring(1, copy.lastIndexOf(PATH_DELIMITER));
String srcId = copy.substring(copy.lastIndexOf(PATH_DELIMITER) + 1);
Bucket srcBucket = storage.getBucket(srcBucketName);
if (!srcBucket.exists()) {
- signalObjectError(ctx, HttpResponseStatus.BAD_REQUEST, "Source bucket does not exist");
+ signalObjectError(ctx,
+ srcBucketName,
+ srcId,
+ S3ErrorCode.NoSuchBucket,
+ String.format("Source bucket '%s' does not exist", srcBucketName));
return;
}
StoredObject src = srcBucket.getObject(srcId);
if (!src.exists()) {
- signalObjectError(ctx, HttpResponseStatus.BAD_REQUEST, "Source object does not exist");
+ signalObjectError(ctx,
+ srcBucketName,
+ srcId,
+ S3ErrorCode.NoSuchKey,
+ String.format("Source object '%s/%s' does not exist", srcBucketName, srcId));
return;
}
Files.copy(src.getFile(), object.getFile());
@@ -697,7 +725,7 @@ private void copyObject(WebContext ctx, Bucket bucket, String id, String copy) t
private void getObject(WebContext ctx, Bucket bucket, String id, boolean sendFile) throws IOException {
StoredObject object = bucket.getObject(id);
if (!object.exists()) {
- signalObjectError(ctx, HttpResponseStatus.NOT_FOUND, "Object does not exist");
+ signalObjectError(ctx, bucket.getName(), id, S3ErrorCode.NoSuchKey, "Object does not exist");
return;
}
Response response = ctx.respondWith();
@@ -778,7 +806,11 @@ private void startMultipartUpload(WebContext ctx, Bucket bucket, String id) {
*/
private void multiObject(WebContext ctx, String uploadId, String partNumber, InputStreamHandler part) {
if (!multipartUploads.contains(uploadId)) {
- ctx.respondWith().error(HttpResponseStatus.NOT_FOUND, ERROR_MULTIPART_UPLOAD_DOES_NOT_EXIST);
+ errorSynthesizer.synthesiseError(ctx,
+ null,
+ null,
+ S3ErrorCode.NoSuchUpload,
+ ERROR_MULTIPART_UPLOAD_DOES_NOT_EXIST);
return;
}
@@ -798,7 +830,11 @@ private void multiObject(WebContext ctx, String uploadId, String partNumber, Inp
.addHeader(HttpHeaderNames.ACCESS_CONTROL_EXPOSE_HEADERS, HTTP_HEADER_NAME_ETAG)
.status(HttpResponseStatus.OK);
} catch (IOException e) {
- ctx.respondWith().error(HttpResponseStatus.INTERNAL_SERVER_ERROR, Exceptions.handle(e));
+ errorSynthesizer.synthesiseError(ctx,
+ null,
+ null,
+ S3ErrorCode.InternalError,
+ Exceptions.handle(e).getMessage());
}
}
@@ -817,7 +853,11 @@ private void completeMultipartUpload(WebContext ctx,
final String uploadId,
InputStreamHandler in) {
if (!multipartUploads.remove(uploadId)) {
- ctx.respondWith().error(HttpResponseStatus.NOT_FOUND, ERROR_MULTIPART_UPLOAD_DOES_NOT_EXIST);
+ errorSynthesizer.synthesiseError(ctx,
+ null,
+ null,
+ S3ErrorCode.NoSuchUpload,
+ ERROR_MULTIPART_UPLOAD_DOES_NOT_EXIST);
return;
}
@@ -844,7 +884,11 @@ private void completeMultipartUpload(WebContext ctx,
file.deleteOnExit();
if (!file.exists()) {
- ctx.respondWith().error(HttpResponseStatus.NOT_FOUND, "Multipart File does not exist");
+ errorSynthesizer.synthesiseError(ctx,
+ null,
+ null,
+ S3ErrorCode.NoSuchUpload,
+ "Multipart File does not exist");
return;
}
try {
@@ -870,7 +914,11 @@ private void completeMultipartUpload(WebContext ctx,
out.endOutput();
} catch (IOException e) {
Exceptions.ignore(e);
- ctx.respondWith().error(HttpResponseStatus.INTERNAL_SERVER_ERROR, "Could not build response");
+ errorSynthesizer.synthesiseError(ctx,
+ null,
+ null,
+ S3ErrorCode.InternalError,
+ "Could not build response");
}
}
@@ -935,7 +983,11 @@ private static void delete(File file) {
*/
private void getPartList(WebContext ctx, Bucket bucket, String id, String uploadId) {
if (!multipartUploads.contains(uploadId)) {
- ctx.respondWith().error(HttpResponseStatus.NOT_FOUND, ERROR_MULTIPART_UPLOAD_DOES_NOT_EXIST);
+ errorSynthesizer.synthesiseError(ctx,
+ null,
+ null,
+ S3ErrorCode.NoSuchUpload,
+ ERROR_MULTIPART_UPLOAD_DOES_NOT_EXIST);
return;
}
diff --git a/src/main/java/ninja/errors/S3ErrorCode.java b/src/main/java/ninja/errors/S3ErrorCode.java
new file mode 100644
index 0000000..8f604bc
--- /dev/null
+++ b/src/main/java/ninja/errors/S3ErrorCode.java
@@ -0,0 +1,39 @@
+/*
+ * Made with all the love in the world
+ * by scireum in Remshalden, Germany
+ *
+ * Copyright by scireum GmbH
+ * http://www.scireum.de - info@scireum.de
+ */
+
+package ninja.errors;
+
+import io.netty.handler.codec.http.HttpResponseStatus;
+
+/**
+ * Lists some S3 error codes
+ * along with their respective {@linkplain HttpResponseStatus HTTP response codes}.
+ */
+public enum S3ErrorCode {
+ AccessDenied(HttpResponseStatus.FORBIDDEN),
+ BadDigest(HttpResponseStatus.BAD_REQUEST),
+ IncompleteBody(HttpResponseStatus.BAD_REQUEST),
+ InternalError(HttpResponseStatus.INTERNAL_SERVER_ERROR),
+ InvalidDigest(HttpResponseStatus.BAD_REQUEST),
+ InvalidRequest(HttpResponseStatus.BAD_REQUEST),
+ NoSuchBucket(HttpResponseStatus.NOT_FOUND),
+ NoSuchBucketPolicy(HttpResponseStatus.NOT_FOUND),
+ NoSuchKey(HttpResponseStatus.NOT_FOUND),
+ NoSuchLifecycleConfiguration(HttpResponseStatus.NOT_FOUND),
+ NoSuchUpload(HttpResponseStatus.NOT_FOUND);
+
+ private final HttpResponseStatus httpStatusCode;
+
+ S3ErrorCode(HttpResponseStatus httpStatusCode) {
+ this.httpStatusCode = httpStatusCode;
+ }
+
+ public HttpResponseStatus getHttpStatusCode() {
+ return httpStatusCode;
+ }
+}
diff --git a/src/main/java/ninja/errors/S3ErrorSynthesizer.java b/src/main/java/ninja/errors/S3ErrorSynthesizer.java
new file mode 100644
index 0000000..c5cd5dd
--- /dev/null
+++ b/src/main/java/ninja/errors/S3ErrorSynthesizer.java
@@ -0,0 +1,58 @@
+/*
+ * Made with all the love in the world
+ * by scireum in Remshalden, Germany
+ *
+ * Copyright by scireum GmbH
+ * http://www.scireum.de - info@scireum.de
+ */
+
+package ninja.errors;
+
+import sirius.kernel.commons.Strings;
+import sirius.kernel.di.std.Register;
+import sirius.kernel.xml.XMLStructuredOutput;
+import sirius.web.http.WebContext;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+/**
+ * Synthesizes S3 error
+ * responses.
+ */
+@Register(classes = S3ErrorSynthesizer.class)
+public class S3ErrorSynthesizer {
+
+ /**
+ * Synthesizes an error response.
+ *
+ * @param ctx the request to process.
+ * @param bucket the requested bucket, potentially null.
+ * @param key the requested object's key, potentially null.
+ * @param code the error code to send.
+ * @param message a human-readable description of the error.
+ */
+ public void synthesiseError(@Nonnull WebContext ctx,
+ @Nullable String bucket,
+ @Nullable String key,
+ @Nonnull S3ErrorCode code,
+ @Nullable String message) {
+ XMLStructuredOutput
+ xml = new XMLStructuredOutput(ctx.respondWith()
+ .outputStream(code.getHttpStatusCode(), "text/xml"));
+
+ String resource = null;
+ if (Strings.isFilled(bucket)) {
+ resource = "/" + bucket;
+ if (Strings.isFilled(key)) {
+ resource += "/" + key;
+ }
+ }
+
+ xml.beginOutput("Error");
+ xml.property("Code", code.toString());
+ xml.propertyIfFilled("Message", message);
+ xml.propertyIfFilled("Resource", resource);
+ xml.endOutput();
+ }
+}
diff --git a/src/main/java/ninja/queries/BucketAclSynthesizer.java b/src/main/java/ninja/queries/BucketAclSynthesizer.java
index 0002d92..bde3d80 100644
--- a/src/main/java/ninja/queries/BucketAclSynthesizer.java
+++ b/src/main/java/ninja/queries/BucketAclSynthesizer.java
@@ -20,6 +20,7 @@
*/
@Register(name = "acl")
public class BucketAclSynthesizer implements S3QuerySynthesizer {
+
@Override
public void processQuery(@Nonnull WebContext ctx,
@Nullable String bucket,
diff --git a/src/main/java/ninja/queries/BucketCorsSynthesizer.java b/src/main/java/ninja/queries/BucketCorsSynthesizer.java
index d3ac660..857fa53 100644
--- a/src/main/java/ninja/queries/BucketCorsSynthesizer.java
+++ b/src/main/java/ninja/queries/BucketCorsSynthesizer.java
@@ -20,6 +20,7 @@
*/
@Register(name = "cors")
public class BucketCorsSynthesizer implements S3QuerySynthesizer {
+
@Override
public void processQuery(@Nonnull WebContext ctx,
@Nullable String bucket,
diff --git a/src/main/java/ninja/queries/BucketLifecycleSynthesizer.java b/src/main/java/ninja/queries/BucketLifecycleSynthesizer.java
index 2264864..40f2cfc 100644
--- a/src/main/java/ninja/queries/BucketLifecycleSynthesizer.java
+++ b/src/main/java/ninja/queries/BucketLifecycleSynthesizer.java
@@ -8,9 +8,10 @@
package ninja.queries;
-import io.netty.handler.codec.http.HttpResponseStatus;
+import ninja.errors.S3ErrorCode;
+import ninja.errors.S3ErrorSynthesizer;
+import sirius.kernel.di.std.Part;
import sirius.kernel.di.std.Register;
-import sirius.kernel.xml.XMLStructuredOutput;
import sirius.web.http.WebContext;
import javax.annotation.Nonnull;
@@ -22,15 +23,19 @@
*/
@Register(name = "lifecycle")
public class BucketLifecycleSynthesizer implements S3QuerySynthesizer {
+
+ @Part
+ private S3ErrorSynthesizer errorSynthesizer;
+
@Override
public void processQuery(@Nonnull WebContext ctx,
@Nullable String bucket,
@Nullable String key,
@Nonnull String query) {
- // todo: replace with error handler, see #123
- XMLStructuredOutput xml = new XMLStructuredOutput(ctx.respondWith().outputStream(HttpResponseStatus.NOT_FOUND, "text/xml"));
- xml.beginOutput("Error");
- xml.property("Code", "NoSuchLifecycleConfiguration");
- xml.endOutput();
+ errorSynthesizer.synthesiseError(ctx,
+ bucket,
+ key,
+ S3ErrorCode.NoSuchLifecycleConfiguration,
+ "There is no lifecycle configuration.");
}
}
diff --git a/src/main/java/ninja/queries/BucketLocationSynthesizer.java b/src/main/java/ninja/queries/BucketLocationSynthesizer.java
index cb434fb..94b4de7 100644
--- a/src/main/java/ninja/queries/BucketLocationSynthesizer.java
+++ b/src/main/java/ninja/queries/BucketLocationSynthesizer.java
@@ -21,6 +21,7 @@
*/
@Register(name = "location")
public class BucketLocationSynthesizer implements S3QuerySynthesizer {
+
@Override
public void processQuery(@Nonnull WebContext ctx,
@Nullable String bucket,
diff --git a/src/main/java/ninja/queries/BucketPolicySynthesizer.java b/src/main/java/ninja/queries/BucketPolicySynthesizer.java
index 5941671..931cd41 100644
--- a/src/main/java/ninja/queries/BucketPolicySynthesizer.java
+++ b/src/main/java/ninja/queries/BucketPolicySynthesizer.java
@@ -8,9 +8,10 @@
package ninja.queries;
-import io.netty.handler.codec.http.HttpResponseStatus;
+import ninja.errors.S3ErrorCode;
+import ninja.errors.S3ErrorSynthesizer;
+import sirius.kernel.di.std.Part;
import sirius.kernel.di.std.Register;
-import sirius.kernel.xml.XMLStructuredOutput;
import sirius.web.http.WebContext;
import javax.annotation.Nonnull;
@@ -22,15 +23,19 @@
*/
@Register(name = "policy")
public class BucketPolicySynthesizer implements S3QuerySynthesizer {
+
+ @Part
+ private S3ErrorSynthesizer errorSynthesizer;
+
@Override
public void processQuery(@Nonnull WebContext ctx,
@Nullable String bucket,
@Nullable String key,
@Nonnull String query) {
- // todo: replace with error handler, see #123
- XMLStructuredOutput xml = new XMLStructuredOutput(ctx.respondWith().outputStream(HttpResponseStatus.NOT_FOUND, "text/xml"));
- xml.beginOutput("Error");
- xml.property("Code", "NoSuchBucketPolicy");
- xml.endOutput();
+ errorSynthesizer.synthesiseError(ctx,
+ bucket,
+ key,
+ S3ErrorCode.NoSuchBucketPolicy,
+ "The bucket does not have a policy.");
}
}
diff --git a/src/main/java/ninja/queries/BucketRequestPaymentSynthesizer.java b/src/main/java/ninja/queries/BucketRequestPaymentSynthesizer.java
index 20d4a5d..1f41c81 100644
--- a/src/main/java/ninja/queries/BucketRequestPaymentSynthesizer.java
+++ b/src/main/java/ninja/queries/BucketRequestPaymentSynthesizer.java
@@ -21,6 +21,7 @@
*/
@Register(name = "requestPayment")
public class BucketRequestPaymentSynthesizer implements S3QuerySynthesizer {
+
@Override
public void processQuery(@Nonnull WebContext ctx,
@Nullable String bucket,