Skip to content

Commit

Permalink
Merge pull request #131 from scireum/jvo/issue_123
Browse files Browse the repository at this point in the history 
Standard-Compliant Error Messages
  • Loading branch information
@andyHa
andyHa authored Jan 3, 2020
2 parents 95fa2fd + 6d79500 commit 8f0528f
Show file tree
Hide file tree
Showing 9 changed files with 205 additions and 42 deletions.
108 changes: 80 additions & 28 deletions src/main/java/ninja/S3Dispatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -97,6 +99,9 @@ private static class S3Request {
@Part
private AwsHashCalculator hashCalculator;

@Part
private S3ErrorSynthesizer errorSynthesizer;

@ConfigValue("storage.multipartDir")
private String multipartDir;

Expand Down Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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;
}

Expand All @@ -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();
Expand Down Expand Up @@ -510,33 +518,35 @@ 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;
}

if (!bucket.exists()) {
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,
Expand All @@ -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,
Expand Down Expand Up @@ -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())) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}

Expand All @@ -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());
}
}

Expand All @@ -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;
}

Expand All @@ -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 {
Expand All @@ -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");
}
}

Expand Down Expand Up @@ -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;
}

Expand Down
39 changes: 39 additions & 0 deletions src/main/java/ninja/errors/S3ErrorCode.java
View file
Original file line number Diff line number Diff line change
@@ -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 - [email protected]
*/

package ninja.errors;

import io.netty.handler.codec.http.HttpResponseStatus;

/**
* Lists some <em>S3</em> <a href="https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html">error codes</a>
* 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;
}
}
58 changes: 58 additions & 0 deletions src/main/java/ninja/errors/S3ErrorSynthesizer.java
View file
Original file line number Diff line number Diff line change
@@ -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 - [email protected]
*/

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 <em>S3</em> <a href="https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html">error
* responses</a>.
*/
@Register(classes = S3ErrorSynthesizer.class)
public class S3ErrorSynthesizer {

/**
* Synthesizes an error response.
*
* @param ctx the request to process.
* @param bucket the requested bucket, potentially <b>null</b>.
* @param key the requested object's key, potentially <b>null</b>.
* @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();
}
}
1 change: 1 addition & 0 deletions src/main/java/ninja/queries/BucketAclSynthesizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*/
@Register(name = "acl")
public class BucketAclSynthesizer implements S3QuerySynthesizer {

@Override
public void processQuery(@Nonnull WebContext ctx,
@Nullable String bucket,
Expand Down
1 change: 1 addition & 0 deletions src/main/java/ninja/queries/BucketCorsSynthesizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*/
@Register(name = "cors")
public class BucketCorsSynthesizer implements S3QuerySynthesizer {

@Override
public void processQuery(@Nonnull WebContext ctx,
@Nullable String bucket,
Expand Down
Loading

0 comments on commit 8f0528f

Please sign in to comment.