Add "range-end is inclusive" comments everywhere we do that tricky ma…

…th. We've had errors around this in the past.
awslabs · Aug 24, 2023 · e3bd4a8 · e3bd4a8
1 parent 2c43088
commit e3bd4a8
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 11 deletions.
diff --git a/include/aws/s3/private/s3_auto_ranged_get.h b/include/aws/s3/private/s3_auto_ranged_get.h
@@ -25,7 +25,9 @@ struct aws_s3_auto_ranged_get {
         uint64_t object_range_start;
 
         /* The last byte of the data that will be retrieved from the object.
-         * (ignore this if object_range_empty) */
+         * (ignore this if object_range_empty)
+         * Note this is inclusive: https://developer.mozilla.org/en-US/docs/Web/HTTP/Range_requests
+         * So if begin=0 and end=0 then 1 byte is being downloaded. */
         uint64_t object_range_end;
 
         /* The total number of parts that are being used in downloading the object range. Note that "part" here

diff --git a/source/s3_auto_ranged_get.c b/source/s3_auto_ranged_get.c
@@ -193,7 +193,7 @@ static bool s_s3_auto_ranged_get_update(
                         AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS | AWS_S3_REQUEST_FLAG_PART_SIZE_RESPONSE_BODY);
 
                     request->part_range_start = 0;
-                    request->part_range_end = meta_request->part_size - 1;
+                    request->part_range_end = meta_request->part_size - 1; /* range-end is inclusive */
                     request->discovers_object_size = true;
 
                     ++auto_ranged_get->synced_data.num_parts_requested;
@@ -498,7 +498,7 @@ static int s_discover_object_range_and_content_length(
              * object range and total object size. Otherwise, the size and range should be equal to the
              * total_content_length. */
             if (!auto_ranged_get->initial_message_has_range_header) {
-                object_range_end = total_content_length - 1;
+                object_range_end = total_content_length - 1; /* range-end is inclusive */
             } else if (aws_s3_parse_content_range_response_header(
                            meta_request->allocator,
                            request->send_data.response_headers,
@@ -557,7 +557,7 @@ static int s_discover_object_range_and_content_length(
 
             /* When discovering the object size via first-part, the object range is the entire object. */
             object_range_start = 0;
-            object_range_end = total_content_length - 1;
+            object_range_end = total_content_length - 1; /* range-end is inclusive */
 
             result = AWS_OP_SUCCESS;
             break;
@@ -703,11 +703,13 @@ static void s_s3_auto_ranged_get_request_finished(
                     if (meta_request->progress_callback != NULL) {
                         struct aws_s3_meta_request_event event = {.type = AWS_S3_META_REQUEST_EVENT_PROGRESS};
                         event.u.progress.info.bytes_transferred = request->send_data.response_body.len;
-                        event.u.progress.info.content_length =
-                            auto_ranged_get->synced_data.object_range_empty
-                                ? 0
-                                : (auto_ranged_get->synced_data.object_range_end + 1 -
-                                   auto_ranged_get->synced_data.object_range_start);
+                        if (auto_ranged_get->synced_data.object_range_empty) {
+                            event.u.progress.info.content_length = 0;
+                        } else {
+                            /* Note that range-end is inclusive */
+                            event.u.progress.info.content_length = auto_ranged_get->synced_data.object_range_end + 1 -
+                                                                   auto_ranged_get->synced_data.object_range_start;
+                        }
                         aws_s3_meta_request_add_event_for_delivery_synced(meta_request, &event);
                     }
 

diff --git a/source/s3_copy_object.c b/source/s3_copy_object.c
@@ -443,6 +443,7 @@ static struct aws_future_void *s_s3_copy_object_prepare_request(struct aws_s3_re
         case AWS_S3_COPY_OBJECT_REQUEST_TAG_MULTIPART_COPY: {
             /* Create a new uploadPartCopy message to upload a part. */
             /* compute sub-request range */
+            /* note that range-end is inclusive */
             uint64_t range_start = (request->part_number - 1) * copy_object->synced_data.part_size;
             uint64_t range_end = range_start + copy_object->synced_data.part_size - 1;
             if (range_end >= copy_object->synced_data.content_length) {

diff --git a/source/s3_util.c b/source/s3_util.c
@@ -470,7 +470,7 @@ uint32_t aws_s3_get_num_parts(size_t part_size, uint64_t object_range_start, uin
 
     /* If the range has room for a second part, calculate the additional amount of parts. */
     if (second_part_start <= object_range_end) {
-        uint64_t aligned_range_remainder = object_range_end + 1 - second_part_start;
+        uint64_t aligned_range_remainder = object_range_end + 1 - second_part_start; /* range-end is inclusive */
         num_parts += (uint32_t)(aligned_range_remainder / (uint64_t)part_size);
 
         if ((aligned_range_remainder % part_size) > 0) {
@@ -515,7 +515,7 @@ void aws_s3_get_part_range(
         /* Else, find the next part by adding the object range + total number of whole parts before this one + initial
          * part size*/
         *out_part_range_start = object_range_start + ((uint64_t)(part_index - 1)) * part_size_uint64 + first_part_size;
-        *out_part_range_end = *out_part_range_start + part_size_uint64 - 1;
+        *out_part_range_end = *out_part_range_start + part_size_uint64 - 1; /* range-end is inclusive */
     }
 
     /* Cap the part's range end using the object's range end. */