Skip to content

Conversation

@TingDaoK
Copy link
Contributor

@TingDaoK TingDaoK commented Aug 11, 2021

HTTP2 headers

  • pseudo headers are pushed into the front of the array list, and other than that, it will be treated the same as normal headers
  • Trade off:
    • We know that push front to the array list is expensive. But, it should be used only few times, as you don't want to change pseudo headers a lot and there are at most 4 of them. More than that, we don't need to do the push front later when we need to send the headers into the wire.
    • The advantage of it is that we will have the mostly the same behavior as netty, which is used by Java SDK team already.
  • add will push the pseudo header to the front of the list when needed (the last header is NOT pseudo header)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@TingDaoK TingDaoK requested a review from graebm August 11, 2021 17:34
* TODO: (Maybe more, connection-specific header will be removed, etc...)
*/
AWS_HTTP_API
struct aws_http_message *aws_http2_message_new_from_http1(struct aws_http_message *http1_msg);
Copy link
Contributor

Choose a reason for hiding this comment

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

until we have reference counting on input-streams, this is dangerous

it's ok if you make this private

OR JUST ADD REFERENCE COUNTING TO INPUT_STREAMS!!! WOOO!

@TingDaoK TingDaoK changed the title H2 message headers only http2 headers Oct 4, 2021
@TingDaoK TingDaoK marked this pull request as draft October 4, 2021 19:17
@TingDaoK TingDaoK marked this pull request as ready for review October 4, 2021 22:29
@TingDaoK TingDaoK changed the base branch from h2_message_main to main October 26, 2021 16:50

int aws_http_headers_add_header(struct aws_http_headers *headers, const struct aws_http_header *header) {
/* Add pesudo headers to the front and not checking any violation until we send the header to the wire */
bool pesudo = aws_strutil_is_http_pseudo_header_name(header->name);
Copy link
Contributor

Choose a reason for hiding this comment

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

pesudo -> pseudo

if (pesudo && aws_http_headers_count(headers)) {
struct aws_http_header last_header;
AWS_ZERO_STRUCT(last_header);
if (aws_http_headers_get_index(headers, aws_http_headers_count(headers) - 1, &last_header)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

trivial: don't need to check this error. it can only fail if the range is wrong, and you just checked the range

}
front = !aws_strutil_is_http_pseudo_header_name(last_header.name);
}
return s_http_headers_add_header_impl(headers, header, front);
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this is weird but ughghghgh whatever

@TingDaoK TingDaoK merged commit f774425 into main Oct 27, 2021
@TingDaoK TingDaoK deleted the h2_message branch October 27, 2021 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants