Skip to content

Commit

Permalink
Report Error headers in S3 Finished Response Context (#736)
Browse files Browse the repository at this point in the history
  • Loading branch information
waahm7 authored Dec 18, 2023
1 parent e68c96a commit da9725c
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/
package software.amazon.awssdk.crt.s3;

import software.amazon.awssdk.crt.http.HttpHeader;

public class S3FinishedResponseContext {
private final int errorCode;
private final int responseStatus;
Expand All @@ -12,6 +14,7 @@ public class S3FinishedResponseContext {
private final boolean didValidateChecksum;

private final Throwable cause;
private final HttpHeader[] errorHeaders;

/*
* errorCode The CRT error code
Expand All @@ -21,13 +24,14 @@ public class S3FinishedResponseContext {
* didValidateChecksum which is true if the response was validated.
* cause of the error such as a Java exception in a callback. Maybe NULL if there was no exception in a callback.
*/
S3FinishedResponseContext(final int errorCode, final int responseStatus, final byte[] errorPayload, final ChecksumAlgorithm checksumAlgorithm, final boolean didValidateChecksum, Throwable cause) {
S3FinishedResponseContext(final int errorCode, final int responseStatus, final byte[] errorPayload, final ChecksumAlgorithm checksumAlgorithm, final boolean didValidateChecksum, Throwable cause, final HttpHeader[] errorHeaders) {
this.errorCode = errorCode;
this.responseStatus = responseStatus;
this.errorPayload = errorPayload;
this.checksumAlgorithm = checksumAlgorithm;
this.didValidateChecksum = didValidateChecksum;
this.cause = cause;
this.errorHeaders = errorHeaders;
}

public int getErrorCode() {
Expand Down Expand Up @@ -63,10 +67,19 @@ public boolean isChecksumValidated() {

/**
* Cause of the error, such as a Java exception from a callback. May be NULL if there was no exception in a callback.
*
* @return throwable
*/
public Throwable getCause() {
return cause;
}

/**
* In the case of a failed HTTP response, get the headers of the response. May be NULL.
*
* @return array of headers
*/
public HttpHeader[] getErrorHeaders() {
return errorHeaders;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ int onResponseBody(byte[] bodyBytesIn, long objectRangeStart, long objectRangeEn
return this.responseHandler.onResponseBody(ByteBuffer.wrap(bodyBytesIn), objectRangeStart, objectRangeEnd);
}

void onFinished(int errorCode, int responseStatus, byte[] errorPayload, int checksumAlgorithm, boolean didValidateChecksum, Throwable cause) {
S3FinishedResponseContext context = new S3FinishedResponseContext(errorCode, responseStatus, errorPayload, ChecksumAlgorithm.getEnumValueFromInteger(checksumAlgorithm), didValidateChecksum, cause);
void onFinished(int errorCode, int responseStatus, byte[] errorPayload, int checksumAlgorithm, boolean didValidateChecksum, Throwable cause, final ByteBuffer headersBlob) {
HttpHeader[] errorHeaders = headersBlob == null ? null : HttpHeader.loadHeadersFromMarshalledHeadersBlob(headersBlob);
S3FinishedResponseContext context = new S3FinishedResponseContext(errorCode, responseStatus, errorPayload, ChecksumAlgorithm.getEnumValueFromInteger(checksumAlgorithm), didValidateChecksum, cause, errorHeaders);
this.responseHandler.onFinished(context);
}

Expand Down
2 changes: 1 addition & 1 deletion src/native/http_request_response.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ int aws_java_http_stream_on_incoming_headers_fn(

binding->response_status = resp_status;

if (aws_marshal_http_headers_to_dynamic_buffer(&binding->headers_buf, header_array, num_headers)) {
if (aws_marshal_http_headers_array_to_dynamic_buffer(&binding->headers_buf, header_array, num_headers)) {
AWS_LOGF_ERROR(
AWS_LS_HTTP_STREAM, "id=%p: Failed to allocate buffer space for incoming headers", (void *)stream);
return AWS_OP_ERR;
Expand Down
16 changes: 15 additions & 1 deletion src/native/http_request_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -222,14 +222,15 @@ static inline int s_marshal_http_header_to_buffer(
return AWS_OP_ERR;
}

/* This will append to the buffer without overwriting anything */
aws_byte_buf_write_be32(buf, (uint32_t)name->len);
aws_byte_buf_write_from_whole_cursor(buf, *name);
aws_byte_buf_write_be32(buf, (uint32_t)value->len);
aws_byte_buf_write_from_whole_cursor(buf, *value);
return AWS_OP_SUCCESS;
}

int aws_marshal_http_headers_to_dynamic_buffer(
int aws_marshal_http_headers_array_to_dynamic_buffer(
struct aws_byte_buf *buf,
const struct aws_http_header *header_array,
size_t num_headers) {
Expand All @@ -241,6 +242,19 @@ int aws_marshal_http_headers_to_dynamic_buffer(

return AWS_OP_SUCCESS;
}

int aws_marshal_http_headers_to_dynamic_buffer(struct aws_byte_buf *buf, const struct aws_http_headers *headers) {
for (size_t i = 0; i < aws_http_headers_count(headers); ++i) {
struct aws_http_header header;
aws_http_headers_get_index(headers, i, &header);
if (s_marshal_http_header_to_buffer(buf, &header.name, &header.value)) {
return AWS_OP_ERR;
}
}

return AWS_OP_SUCCESS;
}

/**
* Unmarshal the request from java.
*
Expand Down
4 changes: 3 additions & 1 deletion src/native/http_request_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@ struct aws_http_message *aws_http_request_new_from_java_http_request(

struct aws_http_headers *aws_http_headers_new_from_java_http_headers(JNIEnv *env, jbyteArray marshalled_headers);

int aws_marshal_http_headers_to_dynamic_buffer(
int aws_marshal_http_headers_array_to_dynamic_buffer(
struct aws_byte_buf *buf,
const struct aws_http_header *header_array,
size_t num_headers);

int aws_marshal_http_headers_to_dynamic_buffer(struct aws_byte_buf *buf, const struct aws_http_headers *headers);

/* if this fails a java exception has been set. */
int aws_apply_java_http_request_changes_to_native_request(
JNIEnv *env,
Expand Down
2 changes: 1 addition & 1 deletion src/native/java_class_ids.c
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ static void s_cache_s3_meta_request_response_handler_native_adapter_properties(J
AWS_FATAL_ASSERT(s3_meta_request_response_handler_native_adapter_properties.onResponseBody);

s3_meta_request_response_handler_native_adapter_properties.onFinished =
(*env)->GetMethodID(env, cls, "onFinished", "(II[BIZLjava/lang/Throwable;)V");
(*env)->GetMethodID(env, cls, "onFinished", "(II[BIZLjava/lang/Throwable;Ljava/nio/ByteBuffer;)V");
AWS_FATAL_ASSERT(s3_meta_request_response_handler_native_adapter_properties.onFinished);

s3_meta_request_response_handler_native_adapter_properties.onResponseHeaders =
Expand Down
77 changes: 59 additions & 18 deletions src/native/s3_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,30 @@ static int s_on_s3_meta_request_body_callback(
return return_value;
}

static int s_marshal_http_headers_to_buf(const struct aws_http_headers *headers, struct aws_byte_buf *out_headers_buf) {
/* calculate initial header capacity */
size_t headers_initial_capacity = 0;
for (size_t header_index = 0; header_index < aws_http_headers_count(headers); ++header_index) {
struct aws_http_header header;
aws_http_headers_get_index(headers, header_index, &header);
/* aws_marshal_http_headers_array_to_dynamic_buffer() impl drives this calculation */
headers_initial_capacity += header.name.len + header.value.len + 8;
}

struct aws_allocator *allocator = aws_jni_get_allocator();

if (aws_byte_buf_init(out_headers_buf, allocator, headers_initial_capacity)) {
return AWS_OP_ERR;
}

if (aws_marshal_http_headers_to_dynamic_buffer(out_headers_buf, headers)) {
aws_byte_buf_clean_up(out_headers_buf);
return AWS_OP_ERR;
}

return AWS_OP_SUCCESS;
}

static int s_on_s3_meta_request_headers_callback(
struct aws_s3_meta_request *meta_request,
const struct aws_http_headers *headers,
Expand All @@ -657,27 +681,13 @@ static int s_on_s3_meta_request_headers_callback(
}

jobject java_headers_buffer = NULL;
struct aws_allocator *allocator = aws_jni_get_allocator();
/* calculate initial header capacity */
size_t headers_initial_capacity = 0;
for (size_t header_index = 0; header_index < aws_http_headers_count(headers); ++header_index) {
struct aws_http_header header;
aws_http_headers_get_index(headers, header_index, &header);
/* aws_marshal_http_headers_to_dynamic_buffer() impl drives this calculation */
headers_initial_capacity += header.name.len + header.value.len + 8;
}

struct aws_byte_buf headers_buf;
AWS_ZERO_STRUCT(headers_buf);
if (aws_byte_buf_init(&headers_buf, allocator, headers_initial_capacity)) {
goto cleanup; /* safe since we zeroed */
}

for (size_t header_index = 0; header_index < aws_http_headers_count(headers); ++header_index) {
struct aws_http_header header;
aws_http_headers_get_index(headers, header_index, &header);
aws_marshal_http_headers_to_dynamic_buffer(&headers_buf, &header, 1);
if (s_marshal_http_headers_to_buf(headers, &headers_buf)) {
goto cleanup;
}

java_headers_buffer = aws_jni_direct_byte_buffer_from_raw_ptr(env, headers_buf.buffer, headers_buf.len);
if (java_headers_buffer == NULL) {
aws_jni_check_and_clear_exception(env);
Expand Down Expand Up @@ -746,6 +756,31 @@ static void s_on_s3_meta_request_finish_callback(
jthrowable java_exception =
meta_request_result->error_code == AWS_ERROR_HTTP_CALLBACK_FAILURE ? callback_data->java_exception : NULL;

jobject java_error_headers_buffer = NULL;
struct aws_byte_buf headers_buf;
AWS_ZERO_STRUCT(headers_buf);
if (meta_request_result->error_response_headers) {
/* Ignore any errors and just report the original error without headers */
if (s_marshal_http_headers_to_buf(meta_request_result->error_response_headers, &headers_buf) ==
AWS_OP_SUCCESS) {
java_error_headers_buffer =
aws_jni_direct_byte_buffer_from_raw_ptr(env, headers_buf.buffer, headers_buf.len);
if (java_error_headers_buffer == NULL) {
AWS_LOGF_ERROR(
AWS_LS_S3_META_REQUEST,
"id=%p: Ignored Exception from "
"S3MetaRequest.onFinished.aws_jni_direct_byte_buffer_from_raw_ptr",
(void *)meta_request);
aws_jni_check_and_clear_exception(env);
}
} else {
AWS_LOGF_ERROR(
AWS_LS_S3_META_REQUEST,
"id=%p: Ignored Exception from S3MetaRequest.onFinished.s_marshal_http_headers_to_buf",
(void *)meta_request);
}
}

(*env)->CallVoidMethod(
env,
callback_data->java_s3_meta_request_response_handler_native_adapter,
Expand All @@ -755,7 +790,8 @@ static void s_on_s3_meta_request_finish_callback(
jni_payload,
meta_request_result->validation_algorithm,
meta_request_result->did_validate,
java_exception);
java_exception,
java_error_headers_buffer);

if (aws_jni_check_and_clear_exception(env)) {
AWS_LOGF_ERROR(
Expand All @@ -766,6 +802,11 @@ static void s_on_s3_meta_request_finish_callback(
if (jni_payload) {
(*env)->DeleteLocalRef(env, jni_payload);
}

aws_byte_buf_clean_up(&headers_buf);
if (java_error_headers_buffer) {
(*env)->DeleteLocalRef(env, java_error_headers_buffer);
}
}

aws_jni_release_thread_env(callback_data->jvm, env);
Expand Down
47 changes: 47 additions & 0 deletions src/test/java/software/amazon/awssdk/crt/test/S3ClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,53 @@ public void onFinished(S3FinishedResponseContext context) {
}
}

@Test
public void testS3GetErrorHeadersAreReported() {
skipIfAndroid();
skipIfNetworkUnavailable();
Assume.assumeTrue(hasAwsCredentials());
S3ClientOptions clientOptions = new S3ClientOptions().withRegion(REGION);
try (S3Client client = createS3Client(clientOptions)) {
CompletableFuture<Integer> onFinishedFuture = new CompletableFuture<>();
S3MetaRequestResponseHandler responseHandler = new S3MetaRequestResponseHandler() {

@Override
public int onResponseBody(ByteBuffer bodyBytesIn, long objectRangeStart, long objectRangeEnd) {
byte[] bytes = new byte[bodyBytesIn.remaining()];
bodyBytesIn.get(bytes);
Log.log(Log.LogLevel.Info, Log.LogSubject.JavaCrtS3, "Body Response: " + Arrays.toString(bytes));
return 0;
}

@Override
public void onFinished(S3FinishedResponseContext context) {
Log.log(Log.LogLevel.Info, Log.LogSubject.JavaCrtS3,
"Meta request finished with error code " + context.getErrorCode());
try {
assertNotNull(context.getErrorHeaders());
assertTrue(context.getErrorCode() > 0);
onFinishedFuture.complete(0); // Assertions passed
} catch (AssertionError e) {
onFinishedFuture.complete(-1); // Assertions failed
}
}
};

HttpHeader[] headers = { new HttpHeader("Host", ENDPOINT) };
HttpRequest httpRequest = new HttpRequest("GET", "/key_does_not_exist.txt", headers, null);

S3MetaRequestOptions metaRequestOptions = new S3MetaRequestOptions()
.withMetaRequestType(MetaRequestType.GET_OBJECT).withHttpRequest(httpRequest)
.withResponseHandler(responseHandler);

try (S3MetaRequest metaRequest = client.makeMetaRequest(metaRequestOptions)) {
Assert.assertEquals(Integer.valueOf(0), onFinishedFuture.get());
}
} catch (InterruptedException | ExecutionException ex) {
Assert.fail(ex.getMessage());
}
}

@Test
public void testS3GetAfterClientIsClose() {
skipIfAndroid();
Expand Down

0 comments on commit da9725c

Please sign in to comment.