Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions include/aws/http/private/h1_encoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ struct aws_h1_chunk {
struct aws_byte_buf chunk_line;
};

struct aws_h1_trailer {
struct aws_allocator *allocator;
struct aws_byte_buf trailer_data;
};

/**
* Message to be submitted to encoder.
* Contains data necessary for encoder to write an outgoing request or response.
Expand All @@ -36,6 +41,9 @@ struct aws_h1_encoder_message {
* A chunk with data_size=0 means "final chunk" */
struct aws_linked_list *pending_chunk_list;

/* Pointer to chunked_trailer, used for chunked_trailer. */
struct aws_h1_trailer *trailer;

/* If non-zero, length of unchunked body to send */
uint64_t content_length;
bool has_connection_close_header;
Expand Down Expand Up @@ -71,6 +79,11 @@ struct aws_h1_encoder {
};

struct aws_h1_chunk *aws_h1_chunk_new(struct aws_allocator *allocator, const struct aws_http1_chunk_options *options);
struct aws_h1_trailer *aws_h1_trailer_new(
struct aws_allocator *allocator,
const struct aws_http_headers *trailing_headers);

void aws_h1_trailer_destroy(struct aws_h1_trailer *trailer);

/* Just destroy the chunk (don't fire callback) */
void aws_h1_chunk_destroy(struct aws_h1_chunk *chunk);
Expand Down
12 changes: 12 additions & 0 deletions include/aws/http/private/h1_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ struct aws_h1_stream {
* Encoder completes/frees/pops front chunk when it's done sending. */
struct aws_linked_list pending_chunk_list;

struct aws_h1_encoder_message message;

/* Size of stream's flow-control window.
* Only body data (not headers, etc) counts against the stream's flow-control window. */
uint64_t stream_window;
Expand All @@ -79,6 +81,10 @@ struct aws_h1_stream {
* but haven't yet moved to encoder_message.pending_chunk_list where the encoder will find them. */
struct aws_linked_list pending_chunk_list;

/* trailing headers which have been submitted by user,
* but haven't yet moved to encoder_message where the encoder will find them. */
struct aws_h1_trailer *pending_trailer;

enum aws_h1_stream_api_state api_state;

/* Sum of all aws_http_stream_update_window() calls that haven't yet moved to thread_data.stream_window */
Expand All @@ -93,6 +99,12 @@ struct aws_h1_stream {

/* Whether the outgoing message is using chunked encoding */
bool using_chunked_encoding : 1;

/* Whether the final 0 length chunk has already been sent */
bool has_final_chunk : 1;

/* Whether the chunked trailer has already been sent */
bool has_added_trailer : 1;
} synced_data;
};

Expand Down
21 changes: 21 additions & 0 deletions include/aws/http/private/http_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,28 @@ enum aws_http_header_name {
AWS_HTTP_HEADER_EXPECT,
AWS_HTTP_HEADER_TRANSFER_ENCODING,
AWS_HTTP_HEADER_COOKIE,
AWS_HTTP_HEADER_SET_COOKIE,
AWS_HTTP_HEADER_HOST,
AWS_HTTP_HEADER_CACHE_CONTROL,
AWS_HTTP_HEADER_MAX_FORWARDS,
AWS_HTTP_HEADER_PRAGMA,
AWS_HTTP_HEADER_RANGE,
AWS_HTTP_HEADER_TE,
AWS_HTTP_HEADER_CONTENT_ENCODING,
AWS_HTTP_HEADER_CONTENT_TYPE,
AWS_HTTP_HEADER_CONTENT_RANGE,
AWS_HTTP_HEADER_TRAILER,
AWS_HTTP_HEADER_WWW_AUTHENTICATE,
AWS_HTTP_HEADER_AUTHORIZATION,
AWS_HTTP_HEADER_PROXY_AUTHENTICATE,
AWS_HTTP_HEADER_PROXY_AUTHORIZATION,
AWS_HTTP_HEADER_AGE,
AWS_HTTP_HEADER_EXPIRES,
AWS_HTTP_HEADER_DATE,
AWS_HTTP_HEADER_LOCATION,
AWS_HTTP_HEADER_RETRY_AFTER,
AWS_HTTP_HEADER_VARY,
AWS_HTTP_HEADER_WARNING,

AWS_HTTP_HEADER_COUNT, /* Number of enums */
};
Expand Down
1 change: 1 addition & 0 deletions include/aws/http/private/request_response_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ struct aws_http_stream_vtable {
int (*activate)(struct aws_http_stream *stream);

int (*http1_write_chunk)(struct aws_http_stream *http1_stream, const struct aws_http1_chunk_options *options);
int (*http1_add_trailer)(struct aws_http_stream *http1_stream, const struct aws_http_headers *trailing_headers);

int (*http2_reset_stream)(struct aws_http_stream *http2_stream, uint32_t http2_error);
int (*http2_get_received_error_code)(struct aws_http_stream *http2_stream, uint32_t *http2_error);
Expand Down
21 changes: 21 additions & 0 deletions include/aws/http/request_response.h
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,27 @@ AWS_HTTP_API int aws_http1_stream_write_chunk(
struct aws_http_stream *http1_stream,
const struct aws_http1_chunk_options *options);

/**
* Add a list of headers to be added as trailing headers sent after the last chunk is sent.
* The stream must have specified "chunked" in a "transfer-encoding" header. The stream should also have
* a "Trailer" header field which indicates the fields present in the trailer.
*
* Certain headers are forbidden in the trailer (e.g., Transfer-Encoding, Content-Length, Host). See RFC-7541
* Section 4.1.2 for more details.
*
* For client streams, activate() must be called before any chunks are submitted.
*
* For server streams, the response must be submitted before the trailer can be added
*
* aws_http1_stream_add_chunked_trailer must be called before the final size 0 chunk, and at the moment can only
* be called once, though this could change if need be.
*
* Returns AWS_OP_SUCCESS if the chunk has been submitted.
*/
AWS_HTTP_API int aws_http1_stream_add_chunked_trailer(
struct aws_http_stream *http1_stream,
const struct aws_http_headers *trailing_headers);

/**
* Get the message's aws_http_headers.
*
Expand Down
113 changes: 104 additions & 9 deletions source/h1_encoder.c
Original file line number Diff line number Diff line change
Expand Up @@ -156,20 +156,84 @@ static int s_scan_outgoing_headers(
return AWS_OP_SUCCESS;
}

static int s_scan_outgoing_trailer(const struct aws_http_headers *headers, size_t *out_size) {
const size_t num_headers = aws_http_headers_count(headers);
size_t total = 0;
for (size_t i = 0; i < num_headers; i++) {
struct aws_http_header header;
aws_http_headers_get_index(headers, i, &header);
/* Validate header field-name (RFC-7230 3.2): field-name = token */
if (!aws_strutil_is_http_token(header.name)) {
AWS_LOGF_ERROR(AWS_LS_HTTP_STREAM, "id=static: Header name is invalid");
return aws_raise_error(AWS_ERROR_HTTP_INVALID_HEADER_NAME);
}

/* Validate header field-value.
* The value itself isn't supposed to have whitespace on either side,
* but we'll trim it off before validation so we don't start needlessly
* failing requests that used to work before we added validation.
* This should be OK because field-value can be sent with any amount
* of whitespace around it, which the other side will just ignore (RFC-7230 3.2):
* header-field = field-name ":" OWS field-value OWS */
struct aws_byte_cursor field_value = aws_strutil_trim_http_whitespace(header.value);
if (!aws_strutil_is_http_field_value(field_value)) {
AWS_LOGF_ERROR(
AWS_LS_HTTP_STREAM,
"id=static: Header '" PRInSTR "' has invalid value",
AWS_BYTE_CURSOR_PRI(header.name));
return aws_raise_error(AWS_ERROR_HTTP_INVALID_HEADER_VALUE);
}

enum aws_http_header_name name_enum = aws_http_str_to_header_name(header.name);
if (name_enum == AWS_HTTP_HEADER_TRANSFER_ENCODING || name_enum == AWS_HTTP_HEADER_CONTENT_LENGTH ||
name_enum == AWS_HTTP_HEADER_HOST || name_enum == AWS_HTTP_HEADER_EXPECT ||
name_enum == AWS_HTTP_HEADER_CACHE_CONTROL || name_enum == AWS_HTTP_HEADER_MAX_FORWARDS ||
name_enum == AWS_HTTP_HEADER_PRAGMA || name_enum == AWS_HTTP_HEADER_RANGE ||
name_enum == AWS_HTTP_HEADER_TE || name_enum == AWS_HTTP_HEADER_CONTENT_ENCODING ||
name_enum == AWS_HTTP_HEADER_CONTENT_TYPE || name_enum == AWS_HTTP_HEADER_CONTENT_RANGE ||
name_enum == AWS_HTTP_HEADER_TRAILER || name_enum == AWS_HTTP_HEADER_WWW_AUTHENTICATE ||
name_enum == AWS_HTTP_HEADER_AUTHORIZATION || name_enum == AWS_HTTP_HEADER_PROXY_AUTHENTICATE ||
name_enum == AWS_HTTP_HEADER_PROXY_AUTHORIZATION || name_enum == AWS_HTTP_HEADER_SET_COOKIE ||
name_enum == AWS_HTTP_HEADER_COOKIE || name_enum == AWS_HTTP_HEADER_AGE ||
name_enum == AWS_HTTP_HEADER_EXPIRES || name_enum == AWS_HTTP_HEADER_DATE ||
name_enum == AWS_HTTP_HEADER_LOCATION || name_enum == AWS_HTTP_HEADER_RETRY_AFTER ||
name_enum == AWS_HTTP_HEADER_VARY || name_enum == AWS_HTTP_HEADER_WARNING) {
AWS_LOGF_ERROR(
AWS_LS_HTTP_STREAM,
"id=static: Trailing Header '" PRInSTR "' has invalid value",
AWS_BYTE_CURSOR_PRI(header.name));
return aws_raise_error(AWS_ERROR_HTTP_INVALID_HEADER_FIELD);
}

int err = 0;
err |= aws_add_size_checked(header.name.len, total, &total);
err |= aws_add_size_checked(header.value.len, total, &total);
err |= aws_add_size_checked(4, total, &total); /* ": " + "\r\n" */
if (err) {
return AWS_OP_ERR;
}
}
if (aws_add_size_checked(2, total, &total)) { /* "\r\n" */
return AWS_OP_ERR;
}
*out_size = total;
return AWS_OP_SUCCESS;
}

static bool s_write_crlf(struct aws_byte_buf *dst) {
AWS_PRECONDITION(aws_byte_buf_is_valid(dst));
struct aws_byte_cursor crlf_cursor = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("\r\n");
return aws_byte_buf_write_from_whole_cursor(dst, crlf_cursor);
}

static void s_write_headers(struct aws_byte_buf *dst, const struct aws_http_message *message) {
static void s_write_headers(struct aws_byte_buf *dst, const struct aws_http_headers *headers) {

const size_t num_headers = aws_http_message_get_header_count(message);
const size_t num_headers = aws_http_headers_count(headers);

bool wrote_all = true;
for (size_t i = 0; i < num_headers; ++i) {
struct aws_http_header header;
aws_http_message_get_header(message, &header, i);
aws_http_headers_get_index(headers, i, &header);

/* header-line: "{name}: {value}\r\n" */
wrote_all &= aws_byte_buf_write_from_whole_cursor(dst, header.name);
Expand Down Expand Up @@ -264,7 +328,7 @@ int aws_h1_encoder_message_init_from_request(
wrote_all &= aws_byte_buf_write_from_whole_cursor(&message->outgoing_head_buf, version);
wrote_all &= s_write_crlf(&message->outgoing_head_buf);

s_write_headers(&message->outgoing_head_buf, request);
s_write_headers(&message->outgoing_head_buf, aws_http_message_get_const_headers(request));

wrote_all &= s_write_crlf(&message->outgoing_head_buf);
(void)wrote_all;
Expand Down Expand Up @@ -352,7 +416,7 @@ int aws_h1_encoder_message_init_from_response(
wrote_all &= aws_byte_buf_write_from_whole_cursor(&message->outgoing_head_buf, status_text);
wrote_all &= s_write_crlf(&message->outgoing_head_buf);

s_write_headers(&message->outgoing_head_buf, response);
s_write_headers(&message->outgoing_head_buf, aws_http_message_get_const_headers(response));

wrote_all &= s_write_crlf(&message->outgoing_head_buf);
(void)wrote_all;
Expand All @@ -368,6 +432,7 @@ int aws_h1_encoder_message_init_from_response(

void aws_h1_encoder_message_clean_up(struct aws_h1_encoder_message *message) {
aws_byte_buf_clean_up(&message->outgoing_head_buf);
aws_h1_trailer_destroy(message->trailer);
AWS_ZERO_STRUCT(*message);
}

Expand Down Expand Up @@ -443,6 +508,32 @@ static void s_populate_chunk_line_buffer(
AWS_ASSERT(wrote_chunk_line);
}

struct aws_h1_trailer *aws_h1_trailer_new(
struct aws_allocator *allocator,
const struct aws_http_headers *trailing_headers) {
/* Allocate trailer along with storage for the trailer-line */
size_t trailer_size = 0;
if (s_scan_outgoing_trailer(trailing_headers, &trailer_size)) {
return NULL;
}

struct aws_h1_trailer *trailer = aws_mem_calloc(allocator, 1, sizeof(struct aws_h1_trailer));
trailer->allocator = allocator;

aws_byte_buf_init(&trailer->trailer_data, allocator, trailer_size); /* cannot fail */
s_write_headers(&trailer->trailer_data, trailing_headers);
s_write_crlf(&trailer->trailer_data); /* \r\n */
return trailer;
}

void aws_h1_trailer_destroy(struct aws_h1_trailer *trailer) {
if (trailer == NULL) {
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YES it is great to do null-checks on destroy() functions.

Imagine a thing class with 3 members inside it: a, b, c and any of those 3 members could fail during creation. If destroy() functions didn't check NULL, then error-handling in the thing_new() function looks like this (a bit annoying to do so many null-checks, and a little error-prone):

error:
    if (thing->a) {
        a_destroy(a);
    }
    if (thing->b) {
        b_destroy(b);
    }
    if (thing->c) {
        c_destroy(c);
    }
    aws_mem_release(alloc, thing)
    return NULL;
}

or you avoid the if-checks by having distinct goto labels like, and tear things down in reverse-init order which is (VERY FRAGILE AND EASY TO SCREW UP):

c_failed:
    b_destroy(thing->b);
b_failed:
    a_destroy(thing->a);
a_failed:
    aws_mem_release(alloc, thing)
    return NULL;
}

but if we know that destroy() functions check for null, then we can just be simple and do like:

error:
    a_destroy(thing->a);
    b_destroy(thing->b);
    c_destroy(thing->c);
    aws_mem_release(alloc, thing);
    return NULL;
}

or even better, just have the thing call its own destructor:

error:
    thing_destroy(thing);
    return NULL;
}

and since I'm documenting every possible error handling strategy, here's the very worst: If, at any step things go wrong, try to clean up each previous step:

struct thing* thing_new(struct allocator *alloc, ...);
    Thing* thing = aws_mem_calloc(...);
    thing->alloc = alloc;

    A* thing->a = a_new(...);
    if (thing->a == NULL) {
        aws_mem_release(alloc, thing);
        return NULL;
    }

    B* thing->b = b_new(...);
    if (thing->b == NULL) {
        a_destroy(thing->a);
        aws_mem_release(alloc, thing);
        return NULL;
    }

    C* thing->c = c_new(...);
    if (thing->c == NULL) {
        a_destroy(thing->a);
        b_destroy(thing->b);
        aws_mem_release(alloc, thing);
        return NULL;
    }
    
    return thing;
}

which is UNGODLY FRAGILE and hard to get right
and that, boys and girls, is why we use goto in C

Copy link

@sdavtaker sdavtaker Oct 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you check for the nulls, just having a destroy_thing function wouldn't do the same as

error:
    thing_destroy(thing);
    return NULL;
}

But allow you to return to the place you called for the destroy? Not looking in this particular code, just in general.
Use case would be to emit log, or put an assert or some other thing after the destroy is executed.

}
aws_byte_buf_clean_up(&trailer->trailer_data);
aws_mem_release(trailer->allocator, trailer);
}

struct aws_h1_chunk *aws_h1_chunk_new(struct aws_allocator *allocator, const struct aws_http1_chunk_options *options) {
/* Allocate chunk along with storage for the chunk-line */
struct aws_h1_chunk *chunk;
Expand Down Expand Up @@ -748,11 +839,15 @@ static int s_state_fn_chunk_end(struct aws_h1_encoder *encoder, struct aws_byte_

/* Write out trailer after last chunk */
static int s_state_fn_chunk_trailer(struct aws_h1_encoder *encoder, struct aws_byte_buf *dst) {
/* We don't currently have API calls that lets users add trailing headers,
* so just write out the final CRLF */
bool done = s_write_crlf(dst);
bool done;
/* if a chunked trailer was set */
if (encoder->message->trailer) {
done = s_encode_buf(encoder, dst, &encoder->message->trailer->trailer_data);
} else {
done = s_write_crlf(dst);
}
if (!done) {
/* Remain in this state until done writing out CRLF */
/* Remain in this state until we're done writing out trailer */
return AWS_OP_SUCCESS;
}

Expand Down
Loading