Content-Length is the boss (#158)

graebm · web-flow · commit 722cc072fd25 · 2019-11-01T15:16:39.000-07:00
* Content-Length is the boss
- Don't freak out if body stream is set when Content-Length is 0.
  - aws-crt-java unit tests were doing this.
- Obey Content-Length when sending body, rather than trusting end-of-stream.
diff --git a/include/aws/http/http.h b/include/aws/http/http.h
@@ -30,7 +30,6 @@ enum aws_http_errors {
     AWS_ERROR_HTTP_INVALID_STATUS_CODE,
     AWS_ERROR_HTTP_MISSING_BODY_STREAM,
     AWS_ERROR_HTTP_INVALID_BODY_STREAM,
-    AWS_ERROR_HTTP_MISSING_BODY_HEADERS,
     AWS_ERROR_HTTP_CONNECTION_CLOSED,
     AWS_ERROR_HTTP_SWITCHED_PROTOCOLS,
     AWS_ERROR_HTTP_UNSUPPORTED_PROTOCOL,
diff --git a/include/aws/http/private/h1_encoder.h b/include/aws/http/private/h1_encoder.h
@@ -26,6 +26,7 @@ struct aws_h1_encoder_message {
     /* Upon creation, the "head" (everything preceding body) is buffered here. */
     struct aws_byte_buf outgoing_head_buf;
     struct aws_input_stream *body;
+    uint64_t content_length;
     bool has_connection_close_header;
 };
 
@@ -41,7 +42,7 @@ struct aws_h1_encoder {
 
     enum aws_h1_encoder_state state;
     struct aws_h1_encoder_message *message;
-    size_t outgoing_head_progress;
+    uint64_t progress_bytes;
     void *logging_id;
 };
 
diff --git a/source/h1_encoder.c b/source/h1_encoder.c
@@ -18,6 +18,8 @@
 #include <aws/io/logging.h>
 #include <aws/io/stream.h>
 
+#include <inttypes.h>
+
 #define ENCODER_LOGF(level, encoder, text, ...)                                                                        \
     AWS_LOGF_##level(AWS_LS_HTTP_STREAM, "id=%p: " text, encoder->logging_id, __VA_ARGS__)
 #define ENCODER_LOG(level, encoder, text) ENCODER_LOGF(level, encoder, "%s", text)
@@ -26,19 +28,16 @@
  * Scan headers to detect errors and determine anything we'll need to know later (ex: total length).
  */
 static int s_scan_outgoing_headers(
+    struct aws_h1_encoder_message *encoder_message,
     const struct aws_http_message *message,
     size_t *out_header_lines_len,
-    bool *out_has_connection_close_header,
     bool body_headers_ignored,
     bool body_headers_forbidden) {
 
     size_t total = 0;
-
     bool has_body_stream = aws_http_message_get_body_stream(message);
     bool has_body_headers = false;
 
-    bool has_connection_close_header = false;
-
     const size_t num_headers = aws_http_message_get_header_count(message);
     for (size_t i = 0; i < num_headers; ++i) {
         struct aws_http_header header;
@@ -49,19 +48,22 @@ static int s_scan_outgoing_headers(
             case AWS_HTTP_HEADER_CONNECTION: {
                 struct aws_byte_cursor trimmed_value = aws_strutil_trim_http_whitespace(header.value);
                 if (aws_byte_cursor_eq_c_str(&trimmed_value, "close")) {
-                    has_connection_close_header = true;
+                    encoder_message->has_connection_close_header = true;
                 }
             } break;
             case AWS_HTTP_HEADER_CONTENT_LENGTH: {
-                uint64_t content_length = 0;
-                aws_strutil_read_unsigned_num(aws_strutil_trim_http_whitespace(header.value), &content_length);
-                if (content_length > 0) {
+                struct aws_byte_cursor trimmed_value = aws_strutil_trim_http_whitespace(header.value);
+                if (aws_strutil_read_unsigned_num(trimmed_value, &encoder_message->content_length)) {
+                    AWS_LOGF_ERROR(AWS_LS_HTTP_STREAM, "id=static: Invalid Content-Length");
+                    return aws_raise_error(AWS_ERROR_HTTP_INVALID_HEADER_VALUE);
+                }
+                if (encoder_message->content_length > 0) {
                     has_body_headers = true;
                 }
             } break;
             case AWS_HTTP_HEADER_TRANSFER_ENCODING:
-                has_body_headers = true;
-                break;
+                AWS_LOGF_ERROR(AWS_LS_HTTP_STREAM, "id=static: Sending of chunked messages not yet implemented");
+                return aws_raise_error(AWS_ERROR_UNIMPLEMENTED);
             default:
                 break;
         }
@@ -80,23 +82,16 @@ static int s_scan_outgoing_headers(
     }
 
     if (body_headers_ignored) {
-        /* no body should follow, no matter what the headers are */
-        if (has_body_stream) {
-            return aws_raise_error(AWS_ERROR_HTTP_INVALID_BODY_STREAM);
-        }
+        /* Don't send body, no matter what the headers are */
         has_body_headers = false;
+        encoder_message->content_length = 0;
     }
 
     if (has_body_headers && !has_body_stream) {
         return aws_raise_error(AWS_ERROR_HTTP_MISSING_BODY_STREAM);
     }
 
-    if (!has_body_headers && has_body_stream) {
-        return aws_raise_error(AWS_ERROR_HTTP_MISSING_BODY_HEADERS);
-    }
-
     *out_header_lines_len = total;
-    *out_has_connection_close_header = has_connection_close_header;
     return AWS_OP_SUCCESS;
 }
 
@@ -151,11 +146,7 @@ int aws_h1_encoder_message_init_from_request(
 
     size_t header_lines_len;
     err = s_scan_outgoing_headers(
-        request,
-        &header_lines_len,
-        &message->has_connection_close_header,
-        false /*body_headers_ignored*/,
-        false /*body_headers_forbidden*/);
+        message, request, &header_lines_len, false /*body_headers_ignored*/, false /*body_headers_forbidden*/);
     if (err) {
         goto error;
     }
@@ -241,12 +232,7 @@ int aws_h1_encoder_message_init_from_response(
      */
     body_headers_ignored |= status_int == AWS_HTTP_STATUS_304_NOT_MODIFIED;
     bool body_headers_forbidden = status_int == AWS_HTTP_STATUS_204_NO_CONTENT || status_int / 100 == 1;
-    err = s_scan_outgoing_headers(
-        response,
-        &header_lines_len,
-        &message->has_connection_close_header,
-        body_headers_ignored,
-        body_headers_forbidden);
+    err = s_scan_outgoing_headers(message, response, &header_lines_len, body_headers_ignored, body_headers_forbidden);
     if (err) {
         goto error;
     }
@@ -328,7 +314,7 @@ int aws_h1_encoder_start_message(
     encoder->logging_id = log_as_stream;
     encoder->message = message;
     encoder->state = AWS_H1_ENCODER_STATE_HEAD;
-    encoder->outgoing_head_progress = 0;
+    encoder->progress_bytes = 0;
 
     return AWS_OP_SUCCESS;
 }
@@ -354,34 +340,35 @@ int aws_h1_encoder_process(struct aws_h1_encoder *encoder, struct aws_byte_buf *
 
         /* Copy data from outgoing_head_buf */
         struct aws_byte_buf *src = &encoder->message->outgoing_head_buf;
-        size_t src_progress = encoder->outgoing_head_progress;
+        size_t src_progress = (size_t)encoder->progress_bytes;
         size_t src_remaining = src->len - src_progress;
         size_t transferring = src_remaining < dst_available ? src_remaining : dst_available;
 
         bool success = aws_byte_buf_write(dst, src->buffer + src_progress, transferring);
         (void)success;
         AWS_ASSERT(success);
 
-        encoder->outgoing_head_progress += transferring;
+        encoder->progress_bytes += transferring;
 
         ENCODER_LOGF(
             TRACE,
             encoder,
-            "Writing to message, outgoing head progress %zu/%zu.",
-            encoder->outgoing_head_progress,
+            "Writing to message, outgoing head progress %" PRIu64 "/%zu.",
+            encoder->progress_bytes,
             encoder->message->outgoing_head_buf.len);
 
-        if (encoder->outgoing_head_progress == src->len) {
+        if (encoder->progress_bytes == src->len) {
             /* Don't NEED to free this buffer now, but we don't need it anymore, so why not */
             aws_byte_buf_clean_up(&encoder->message->outgoing_head_buf);
 
+            encoder->progress_bytes = 0;
             encoder->state++;
         }
     }
 
     if (encoder->state == AWS_H1_ENCODER_STATE_BODY) {
-        if (!encoder->message->body) {
-            ENCODER_LOG(TRACE, encoder, "No body to send.")
+        if (!encoder->message->body || !encoder->message->content_length) {
+            ENCODER_LOG(TRACE, encoder, "Skipping body")
             encoder->state++;
         } else {
             while (true) {
@@ -408,29 +395,51 @@ int aws_h1_encoder_process(struct aws_h1_encoder *encoder, struct aws_byte_buf *
                     return AWS_OP_ERR;
                 }
 
-                ENCODER_LOGF(TRACE, encoder, "Writing %zu body bytes to message", amount_read);
-
-                struct aws_stream_status status;
-                err = aws_input_stream_get_status(encoder->message->body, &status);
-                if (err) {
+                if ((amount_read > encoder->message->content_length) ||
+                    (encoder->progress_bytes > encoder->message->content_length - amount_read)) {
                     ENCODER_LOGF(
-                        TRACE,
+                        ERROR,
                         encoder,
-                        "Failed to query body stream status, error %d (%s)",
-                        aws_last_error(),
-                        aws_error_name(aws_last_error()));
-
-                    return AWS_OP_ERR;
+                        "Body stream has exceeded Content-Length: %" PRIu64,
+                        encoder->message->content_length);
+                    return aws_raise_error(AWS_ERROR_HTTP_OUTGOING_STREAM_LENGTH_INCORRECT);
                 }
-                if (status.is_end_of_stream) {
-                    ENCODER_LOG(TRACE, encoder, "Done sending body.");
 
+                encoder->progress_bytes += amount_read;
+
+                ENCODER_LOGF(TRACE, encoder, "Writing %zu body bytes to message", amount_read);
+
+                if (encoder->progress_bytes == encoder->message->content_length) {
+                    ENCODER_LOG(TRACE, encoder, "Done sending body.");
+                    encoder->progress_bytes = 0;
                     encoder->state++;
                     break;
                 }
 
                 /* Return if user failed to write anything. Maybe their data isn't ready yet. */
                 if (amount_read == 0) {
+                    /* Ensure we're not at end-of-stream too early */
+                    struct aws_stream_status status;
+                    err = aws_input_stream_get_status(encoder->message->body, &status);
+                    if (err) {
+                        ENCODER_LOGF(
+                            TRACE,
+                            encoder,
+                            "Failed to query body stream status, error %d (%s)",
+                            aws_last_error(),
+                            aws_error_name(aws_last_error()));
+
+                        return AWS_OP_ERR;
+                    }
+                    if (status.is_end_of_stream) {
+                        ENCODER_LOGF(
+                            ERROR,
+                            encoder,
+                            "Reached end of body stream before Content-Length: %" PRIu64 " sent",
+                            encoder->message->content_length);
+                        return aws_raise_error(AWS_ERROR_HTTP_OUTGOING_STREAM_LENGTH_INCORRECT);
+                    }
+
                     ENCODER_LOG(
                         TRACE,
                         encoder,
diff --git a/source/http.c b/source/http.c
@@ -58,9 +58,6 @@ static struct aws_error_info s_errors[] = {
     AWS_DEFINE_ERROR_INFO_HTTP(
         AWS_ERROR_HTTP_INVALID_BODY_STREAM,
         "A body stream provided, but the message does not allow body (ex: response for HEAD Request and 304 response)"),
-    AWS_DEFINE_ERROR_INFO_HTTP(
-        AWS_ERROR_HTTP_MISSING_BODY_HEADERS,
-        "Missing headers (ex: Content-Length) required to send a body."),
     AWS_DEFINE_ERROR_INFO_HTTP(
         AWS_ERROR_HTTP_CONNECTION_CLOSED,
         "The connection has closed or is closing."),
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -39,6 +39,9 @@ add_test_case(h1_client_request_send_headers)
 add_test_case(h1_client_request_send_body)
 add_test_case(h1_client_request_send_large_body)
 add_test_case(h1_client_request_send_large_head)
+add_test_case(h1_client_request_content_length_0_ok)
+add_test_case(h1_client_request_content_length_too_small_is_error)
+add_test_case(h1_client_request_content_length_too_large_is_error)
 add_test_case(h1_client_request_send_multiple_in_1_io_message)
 add_test_case(h1_client_request_close_header_ends_connection)
 add_test_case(h1_client_request_close_header_with_pipelining)
diff --git a/tests/test_h1_client.c b/tests/test_h1_client.c
