Refactor GetObject request with Range Header to Skip HeadRequest - Part 3

include/aws/s3/private/s3_auto_ranged_get.h

@@ -18,6 +18,17 @@ struct aws_s3_auto_ranged_get {
     struct aws_s3_meta_request base;
 
     enum aws_s3_checksum_algorithm validation_algorithm;
+
+    struct aws_string *etag;
+
+    bool initial_message_has_start_range;
+    bool initial_message_has_end_range;
+    uint64_t initial_range_start;
+    uint64_t initial_range_end;
+
+    uint64_t object_size_hint;
+    bool object_size_hint_available;
+
     /* Members to only be used when the mutex in the base type is locked. */
     struct {
         /* The starting byte of the data that we will be retrieved from the object.
@@ -54,10 +65,6 @@ struct aws_s3_auto_ranged_get {
 
     uint32_t initial_message_has_range_header : 1;
     uint32_t initial_message_has_if_match_header : 1;
-
-    struct aws_string *etag;
-    uint64_t object_size_hint;
-    bool object_size_hint_available;
 };
 
 AWS_EXTERN_C_BEGIN

include/aws/s3/private/s3_util.h

@@ -245,6 +245,18 @@ int aws_s3_parse_content_length_response_header(
     struct aws_http_headers *response_headers,
     uint64_t *out_content_length);
 
+/*
+ * Given the request headers list, finds the Range header and parses the range-start and range-end. All arguments are
+ * required.
+ * */
+AWS_S3_API
+int aws_s3_parse_request_range_header(
+    struct aws_http_headers *request_headers,
+    bool *out_has_start_range,
+    bool *out_has_end_range,
+    uint64_t *out_start_range,
+    uint64_t *out_end_range);
+
 /* Calculate the number of parts based on overall object-range and part_size. */
 AWS_S3_API
 uint32_t aws_s3_calculate_auto_ranged_get_num_parts(

source/s3_meta_request.c

@@ -1346,7 +1346,9 @@ static int s_s3_meta_request_incoming_body(
         request->send_data.response_status,
         (uint64_t)data->len,
         (void *)connection);
-    if (request->send_data.response_status < 200 || request->send_data.response_status > 299) {
+    bool successful_response =
+        s_s3_meta_request_error_code_from_response_status(request->send_data.response_status) == AWS_ERROR_SUCCESS;
+    if (!successful_response) {
         AWS_LOGF_TRACE(AWS_LS_S3_META_REQUEST, "response body: \n" PRInSTR "\n", AWS_BYTE_CURSOR_PRI(*data));
     }
 
@@ -1355,7 +1357,7 @@ static int s_s3_meta_request_incoming_body(
     }
 
     if (request->send_data.response_body.capacity == 0) {
-        if (request->has_part_size_response_body) {
+        if (request->has_part_size_response_body && successful_response) {
             AWS_FATAL_ASSERT(request->ticket);
             request->send_data.response_body =
                 aws_s3_buffer_pool_acquire_buffer(request->meta_request->client->buffer_pool, request->ticket);

source/s3_util.c

@@ -488,6 +488,83 @@ int aws_s3_parse_content_length_response_header(
     return result;
 }
 
+int aws_s3_parse_request_range_header(
+    struct aws_http_headers *request_headers,
+    bool *out_has_start_range,
+    bool *out_has_end_range,
+    uint64_t *out_start_range,
+    uint64_t *out_end_range) {
+
+    AWS_PRECONDITION(request_headers);
+    AWS_PRECONDITION(out_has_start_range);
+    AWS_PRECONDITION(out_has_end_range);
+    AWS_PRECONDITION(out_start_range);
+    AWS_PRECONDITION(out_end_range);
+
+    bool has_start_range = false;
+    bool has_end_range = false;
+    uint64_t start_range = 0;
+    uint64_t end_range = 0;
+
+    struct aws_byte_cursor range_header_value;
+
+    if (aws_http_headers_get(request_headers, g_range_header_name, &range_header_value)) {
+        return aws_raise_error(AWS_ERROR_S3_INVALID_RANGE_HEADER);
+    }
+
+    struct aws_byte_cursor range_header_start = aws_byte_cursor_from_c_str("bytes=");
+
+    /* verify bytes= */
+    if (!aws_byte_cursor_starts_with(&range_header_value, &range_header_start)) {
+        return aws_raise_error(AWS_ERROR_S3_INVALID_RANGE_HEADER);
+    }
+
+    aws_byte_cursor_advance(&range_header_value, range_header_start.len);
+    struct aws_byte_cursor substr = {0};
+    /* parse start range */
+    if (!aws_byte_cursor_next_split(&range_header_value, '-', &substr)) {
+        return aws_raise_error(AWS_ERROR_S3_INVALID_RANGE_HEADER);
+    }
+    if (substr.len > 0) {
+        if (aws_byte_cursor_utf8_parse_u64(substr, &start_range)) {
+            return aws_raise_error(AWS_ERROR_S3_INVALID_RANGE_HEADER);
+        }
+        has_start_range = true;
+    }
+
+    /* parse end range */
+    if (!aws_byte_cursor_next_split(&range_header_value, '-', &substr)) {
+        return aws_raise_error(AWS_ERROR_S3_INVALID_RANGE_HEADER);
+    }
+    if (substr.len > 0) {
+        if (aws_byte_cursor_utf8_parse_u64(substr, &end_range)) {
+            return aws_raise_error(AWS_ERROR_S3_INVALID_RANGE_HEADER);
+        }
+        has_end_range = true;
+    }
+
+    /* verify that there is nothing extra */
+    if (aws_byte_cursor_next_split(&range_header_value, '-', &substr)) {
+        return aws_raise_error(AWS_ERROR_S3_INVALID_RANGE_HEADER);
+    }
+
+    /* verify that start-range <= end-range */
+    if (has_end_range && start_range > end_range) {
+        return aws_raise_error(AWS_ERROR_S3_INVALID_RANGE_HEADER);
+    }
+
+    /* verify that start-range or end-range is present */
+    if (!has_start_range && !has_end_range) {
+        return aws_raise_error(AWS_ERROR_S3_INVALID_RANGE_HEADER);
+    }
+
+    *out_has_start_range = has_start_range;
+    *out_has_end_range = has_end_range;
+    *out_start_range = start_range;
+    *out_end_range = end_range;
+    return AWS_OP_SUCCESS;
+}
+
 uint32_t aws_s3_calculate_auto_ranged_get_num_parts(
     size_t part_size,
     uint64_t first_part_size,

tests/CMakeLists.txt

@@ -180,13 +180,16 @@ add_net_test_case(test_s3_auto_ranged_put_sending_user_agent)
 add_net_test_case(test_s3_default_sending_meta_request_user_agent)
 add_net_test_case(test_s3_range_requests)
 add_net_test_case(test_s3_not_satisfiable_range)
+add_net_test_case(test_s3_invalid_start_range_greator_than_end_range)
+add_net_test_case(test_s3_invalid_empty_file_with_range)
 
 add_net_test_case(test_s3_bad_endpoint)
 add_net_test_case(test_s3_different_endpoints)
 
 add_test_case(test_s3_request_type_operation_name)
 add_test_case(test_s3_replace_quote_entities)
 add_test_case(test_s3_strip_quotes)
+add_test_case(test_s3_parse_request_range_header)
 add_test_case(test_s3_parse_content_range_response_header)
 add_test_case(test_s3_parse_content_length_response_header)
 add_test_case(test_s3_get_num_parts_and_get_part_range)

tests/mock_s3_server/mock_s3_server.py

@@ -424,6 +424,10 @@ def handle_get_object(wrapper, request, parsed_path, head_request=False):
     else:
         RETRY_REQUEST_COUNT = 0
 
+    if parsed_path.path == "/get_object_invalid_response_missing_content_range" or parsed_path.path == "/get_object_invalid_response_missing_etags":
+        # Don't generate the body for those requests
+        return response_config
+
     body_range_value = get_request_header_value(request, "range")
 
     if body_range_value:
@@ -439,9 +443,6 @@ def handle_get_object(wrapper, request, parsed_path, head_request=False):
 
     if parsed_path.path == "/get_object_modified":
         return handle_get_object_modified(start_range, end_range, request)
-    elif parsed_path.path == "/get_object_invalid_response_missing_content_range" or parsed_path.path == "/get_object_invalid_response_missing_etags":
-        # Don't generate the body for those requests
-        return response_config
 
     response_config.generate_body_size = data_length
     return response_config

tests/s3_cancel_tests.c

@@ -290,8 +290,8 @@ static int s3_cancel_test_helper_ex(
         struct aws_s3_meta_request_test_results meta_request_test_results;
         aws_s3_meta_request_test_results_init(&meta_request_test_results, allocator);
 
-        // Range for the second 16k
-        const struct aws_byte_cursor range = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("bytes=16384-32767");
+        /* Specify a range without start-range to trigger HeadRequest */
+        const struct aws_byte_cursor range = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("bytes=-32767");
 
         struct aws_s3_tester_meta_request_options options = {
             .allocator = allocator,

tests/s3_data_plane_tests.c

@@ -5779,7 +5779,7 @@ static int s_test_s3_not_satisfiable_range(struct aws_allocator *allocator, void
 
     ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &options, &results));
 
-    ASSERT_TRUE(results.finished_response_status == AWS_HTTP_STATUS_CODE_416_REQUESTED_RANGE_NOT_SATISFIABLE);
+    ASSERT_INT_EQUALS(AWS_HTTP_STATUS_CODE_416_REQUESTED_RANGE_NOT_SATISFIABLE, results.finished_response_status);
     ASSERT_NOT_NULL(results.error_response_operation_name);
     ASSERT_TRUE(
         aws_string_eq_c_str(results.error_response_operation_name, "GetObject") ||
@@ -5793,6 +5793,88 @@ static int s_test_s3_not_satisfiable_range(struct aws_allocator *allocator, void
     return 0;
 }
 
+AWS_TEST_CASE(test_s3_invalid_start_range_greator_than_end_range, s_test_s3_invalid_start_range_greator_than_end_range)
+static int s_test_s3_invalid_start_range_greator_than_end_range(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+
+    struct aws_s3_tester tester;
+    ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
+
+    struct aws_s3_tester_client_options client_options = {
+        .part_size = 16 * 1024,
+    };
+
+    struct aws_s3_client *client = NULL;
+    ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client));
+
+    struct aws_s3_tester_meta_request_options options = {
+        .allocator = allocator,
+        .client = client,
+        .meta_request_type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT,
+        .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE,
+        .get_options =
+            {
+                .object_path = g_pre_existing_object_1MB,
+                .object_range = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("bytes=20-10"),
+            },
+    };
+
+    struct aws_s3_meta_request_test_results results;
+    aws_s3_meta_request_test_results_init(&results, allocator);
+
+    ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &options, &results));
+    ASSERT_INT_EQUALS(results.finished_error_code, AWS_ERROR_S3_INVALID_RANGE_HEADER);
+
+    aws_s3_meta_request_test_results_clean_up(&results);
+
+    aws_s3_client_release(client);
+    aws_s3_tester_clean_up(&tester);
+
+    return 0;
+}
+
+AWS_TEST_CASE(test_s3_invalid_empty_file_with_range, s_test_s3_invalid_empty_file_with_range)
+static int s_test_s3_invalid_empty_file_with_range(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+
+    struct aws_s3_tester tester;
+    ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
+
+    struct aws_s3_tester_client_options client_options = {
+        .part_size = 16 * 1024,
+    };
+
+    struct aws_s3_client *client = NULL;
+    ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client));
+
+    struct aws_s3_tester_meta_request_options options = {
+        .allocator = allocator,
+        .client = client,
+        .meta_request_type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT,
+        .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE,
+        .get_options =
+            {
+                .object_path = g_pre_existing_empty_object,
+                .object_range = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("bytes=0-0"),
+            },
+    };
+
+    struct aws_s3_meta_request_test_results results;
+    aws_s3_meta_request_test_results_init(&results, allocator);
+
+    ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &options, &results));
+    ASSERT_INT_EQUALS(AWS_HTTP_STATUS_CODE_416_REQUESTED_RANGE_NOT_SATISFIABLE, results.finished_response_status);
+    ASSERT_NOT_NULL(results.error_response_operation_name);
+    ASSERT_TRUE(aws_string_eq_c_str(results.error_response_operation_name, "GetObject"));
+
+    aws_s3_meta_request_test_results_clean_up(&results);
+
+    aws_s3_client_release(client);
+    aws_s3_tester_clean_up(&tester);
+
+    return 0;
+}
+
 static const struct aws_byte_cursor g_x_amz_copy_source_name =
     AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source");
 

tests/s3_mock_server_tests.c

@@ -433,7 +433,7 @@ TEST_CASE(get_object_invalid_responses_mock_server) {
         aws_s3_test_get_object_request_new(allocator, *aws_uri_authority(&mock_server), object_path);
     struct aws_http_header range_header = {
         .name = g_range_header_name,
-        .value = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("bytes=0-1"),
+        .value = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("bytes=-1"),
     };
     ASSERT_SUCCESS(aws_http_message_add_header(message, range_header));
     get_options.get_options.object_path = object_path;