Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/460#discussion_r53417506
--- Diff: proxy/http2/HTTP2.cc ---
@@ -484,246 +498,162 @@ convert_from_2_to_1_1_header(HTTPHdr *headers)
return PARSE_DONE;
}
-static int64_t
-http2_write_header_field(uint8_t *out, const uint8_t *end,
MIMEFieldWrapper &header, Http2IndexingTable &indexing_table)
-{
- HpackFieldType field_type = HPACK_FIELD_INDEXED_LITERAL;
-
- // Cookie less that 20 bytes and Authorization are never indexed
- // This policy is refer to Firefox and nghttp2
- int name_len = 0, value_len = 0;
- const char *name = header.name_get(&name_len);
- header.value_get(&value_len);
- if ((ptr_len_casecmp(name, name_len, MIME_FIELD_COOKIE, MIME_LEN_COOKIE)
== 0 && value_len < 20) ||
- (ptr_len_casecmp(name, name_len, MIME_FIELD_AUTHORIZATION,
MIME_LEN_AUTHORIZATION) == 0)) {
- field_type = HPACK_FIELD_NEVERINDEX_LITERAL;
- }
-
- // TODO Enable to configure selecting header field representation
+void
+http2_convert_header_from_1_1_to_2(HTTPHdr *headers)
+{
+ HTTPHdr tmp;
+ tmp.create(http_hdr_type_get(headers->m_http));
+ tmp.copy(headers);
+ headers->fields_clear();
+
+ if (http_hdr_type_get(tmp.m_http) == HTTP_TYPE_RESPONSE) {
+ char status_str[HTTP2_LEN_STATUS_VALUE_STR + 1];
+ snprintf(status_str, sizeof(status_str), "%d", tmp.status_get());
+
+ // Add ':status' header field
+ MIMEField *status_field = headers->field_create(HTTP2_VALUE_STATUS,
HTTP2_LEN_STATUS);
+ status_field->value_set(headers->m_heap, headers->m_mime, status_str,
HTTP2_LEN_STATUS_VALUE_STR);
+ headers->field_attach(status_field);
+
+ MIMEFieldIter field_iter;
+ for (MIMEField *field = tmp.iter_get_first(&field_iter); field !=
NULL; field = tmp.iter_get_next(&field_iter)) {
+ // Intermediaries SHOULD remove connection-specific header fields.
+ const char *name;
+ int name_len;
+ const char *value;
+ int value_len;
+ name = field->name_get(&name_len);
+ if ((name_len == MIME_LEN_CONNECTION && strncasecmp(name,
MIME_FIELD_CONNECTION, name_len) == 0) ||
+ (name_len == MIME_LEN_KEEP_ALIVE && strncasecmp(name,
MIME_FIELD_KEEP_ALIVE, name_len) == 0) ||
+ (name_len == MIME_LEN_PROXY_CONNECTION && strncasecmp(name,
MIME_FIELD_PROXY_CONNECTION, name_len) == 0) ||
+ (name_len == MIME_LEN_TRANSFER_ENCODING && strncasecmp(name,
MIME_FIELD_TRANSFER_ENCODING, name_len) == 0) ||
+ (name_len == MIME_LEN_UPGRADE && strncasecmp(name,
MIME_FIELD_UPGRADE, name_len) == 0)) {
+ continue;
+ }
- const Http2LookupIndexResult &result = indexing_table.get_index(header);
- if (result.index > 0) {
- if (result.value_is_indexed) {
- return encode_indexed_header_field(out, end, result.index);
- } else {
- return encode_literal_header_field_with_indexed_name(out, end,
header, result.index, indexing_table, field_type);
+ MIMEField *newfield;
+ name = field->name_get(&name_len);
+ newfield = headers->field_create(name, name_len);
+ value = field->value_get(&value_len);
+ newfield->value_set(headers->m_heap, headers->m_mime, value,
value_len);
+ tmp.field_delete(field);
+ headers->field_attach(newfield);
+ // Set sensitive flag (See RFC7541 7.1.3)
+ // - Authorization header obviously should not be indexed
+ // - Short Cookie header should not be indexed because of low entropy
+ if ((ptr_len_casecmp(name, name_len, MIME_FIELD_COOKIE,
MIME_LEN_COOKIE) == 0 && value_len < 20) ||
+ (ptr_len_casecmp(name, name_len, MIME_FIELD_AUTHORIZATION,
MIME_LEN_AUTHORIZATION) == 0)) {
+ newfield->m_sensitive = 1;
+ }
}
- } else {
- return encode_literal_header_field_with_new_name(out, end, header,
indexing_table, field_type);
}
+ tmp.destroy();
}
int64_t
-http2_write_psuedo_headers(HTTPHdr *in, uint8_t *out, uint64_t out_len,
Http2IndexingTable &indexing_table)
+http2_encode_header_blocks(HTTPHdr *in, uint8_t *out, uint32_t out_len,
HpackHandle &handle)
{
- uint8_t *p = out;
- uint8_t *end = out + out_len;
- int64_t len;
-
- ink_assert(http_hdr_type_get(in->m_http) != HTTP_TYPE_UNKNOWN);
-
- // TODO Check whether buffer size is enough
-
- // Set psuedo header
- if (http_hdr_type_get(in->m_http) == HTTP_TYPE_RESPONSE) {
- char status_str[HPACK_LEN_STATUS_VALUE_STR + 1];
- snprintf(status_str, sizeof(status_str), "%d", in->status_get());
-
- // Add 'Status:' dummy header field
- MIMEField *status_field = mime_field_create(in->m_heap,
in->m_http->m_fields_impl);
- mime_field_name_value_set(in->m_heap, in->m_mime, status_field, -1,
HPACK_VALUE_STATUS, HPACK_LEN_STATUS, status_str,
- HPACK_LEN_STATUS_VALUE_STR, 0,
HPACK_LEN_STATUS + HPACK_LEN_STATUS_VALUE_STR, true);
- mime_hdr_field_attach(in->m_mime, status_field, 1, NULL);
-
- // Encode psuedo headers by HPACK
- MIMEFieldWrapper header(status_field, in->m_heap,
in->m_http->m_fields_impl);
-
- len = http2_write_header_field(p, end, header, indexing_table);
- if (len == -1)
- return -1;
- p += len;
-
- // Remove dummy header field
- in->field_delete(HPACK_VALUE_STATUS, HPACK_LEN_STATUS);
- }
-
- return p - out;
-}
-
-int64_t
-http2_write_header_fragment(HTTPHdr *in, MIMEFieldIter &field_iter,
uint8_t *out, uint64_t out_len,
- Http2IndexingTable &indexing_table, bool &cont)
-{
- uint8_t *p = out;
- uint8_t *end = out + out_len;
- int64_t len;
-
- ink_assert(http_hdr_type_get(in->m_http) != HTTP_TYPE_UNKNOWN);
- ink_assert(in);
-
- // TODO Get a index value from the tables for the header field, and then
- // choose a representation type.
- // TODO Each indexing types per field should be passed by a caller,
HTTP/2
- // implementation.
-
- // Get first header field which is required encoding
- MIMEField *field;
- if (!field_iter.m_block) {
- field = in->iter_get_first(&field_iter);
- } else {
- field = in->iter_get(&field_iter);
- }
-
- // Set mime headers
- cont = false;
- for (; field != NULL; field = in->iter_get_next(&field_iter)) {
- // Intermediaries SHOULD remove connection-specific header fields.
- int name_len;
- const char *name = field->name_get(&name_len);
- if ((name_len == MIME_LEN_CONNECTION && strncasecmp(name,
MIME_FIELD_CONNECTION, name_len) == 0) ||
- (name_len == MIME_LEN_KEEP_ALIVE && strncasecmp(name,
MIME_FIELD_KEEP_ALIVE, name_len) == 0) ||
- (name_len == MIME_LEN_PROXY_CONNECTION && strncasecmp(name,
MIME_FIELD_PROXY_CONNECTION, name_len) == 0) ||
- (name_len == MIME_LEN_TRANSFER_ENCODING && strncasecmp(name,
MIME_FIELD_TRANSFER_ENCODING, name_len) == 0) ||
- (name_len == MIME_LEN_UPGRADE && strncasecmp(name,
MIME_FIELD_UPGRADE, name_len) == 0)) {
- continue;
- }
-
- MIMEFieldWrapper header(field, in->m_heap, in->m_http->m_fields_impl);
- if ((len = http2_write_header_field(p, end, header, indexing_table))
== -1) {
- if (p == out) {
- // no progress was made, header was too big for the buffer,
skipping for now
- continue;
- }
- if (!cont) {
- // Parsing a part of headers is done
- cont = true;
- return p - out;
- } else {
- // Parse error
- return -1;
- }
- }
- p += len;
- }
-
- // Parsing all headers is done
- return p - out;
+ // TODO: It would be better to split Cookie header value
+ return hpack_encode_header_block(handle, out, out_len, in);
}
/*
* Decode Header Blocks to Header List.
*/
int64_t
-http2_decode_header_blocks(HTTPHdr *hdr, const uint8_t *buf_start, const
uint8_t *buf_end, Http2IndexingTable &indexing_table,
+http2_decode_header_blocks(HTTPHdr *hdr, const uint8_t *buf_start, const
uint32_t buf_len, HpackHandle &handle,
bool &trailing_header)
{
- const uint8_t *cursor = buf_start;
- HdrHeap *heap = hdr->m_heap;
- HTTPHdrImpl *hh = hdr->m_http;
- bool header_field_started = false;
+ const MIMEField *field;
+ const char *value;
+ int len;
bool is_trailing_header = trailing_header;
+ int64_t result = hpack_decode_header_block(handle, hdr, buf_start,
buf_len);
- while (cursor < buf_end) {
- int64_t read_bytes = 0;
+ if (result < 0) {
+ return result;
+ }
- // decode a header field encoded by HPACK
- MIMEField *field = mime_field_create(heap, hh->m_fields_impl);
- MIMEFieldWrapper header(field, heap, hh->m_fields_impl);
- HpackFieldType ftype = hpack_parse_field_type(*cursor);
- switch (ftype) {
- case HPACK_FIELD_INDEX:
- read_bytes = decode_indexed_header_field(header, cursor, buf_end,
indexing_table);
- if (read_bytes == HPACK_ERROR_COMPRESSION_ERROR) {
- return HPACK_ERROR_COMPRESSION_ERROR;
- }
- cursor += read_bytes;
- header_field_started = true;
- break;
- case HPACK_FIELD_INDEXED_LITERAL:
- case HPACK_FIELD_NOINDEX_LITERAL:
- case HPACK_FIELD_NEVERINDEX_LITERAL:
- read_bytes = decode_literal_header_field(header, cursor, buf_end,
indexing_table);
- if (read_bytes == HPACK_ERROR_COMPRESSION_ERROR) {
- return HPACK_ERROR_COMPRESSION_ERROR;
- }
- cursor += read_bytes;
- header_field_started = true;
- break;
- case HPACK_FIELD_TABLESIZE_UPDATE:
- if (header_field_started) {
- return HPACK_ERROR_COMPRESSION_ERROR;
+ MIMEFieldIter iter;
+ unsigned int expected_pseudo_header_count = 4;
+ unsigned int pseudo_header_count = 0;
+
+ if (is_trailing_header) {
+ expected_pseudo_header_count = 0;
+ }
+ for (field = hdr->iter_get_first(&iter); field != NULL; field =
hdr->iter_get_next(&iter)) {
+ value = field->name_get(&len);
+ // Pseudo headers must appear before regular headers
+ if (len && value[0] == ':') {
+ ++pseudo_header_count;
+ if (pseudo_header_count > expected_pseudo_header_count) {
+ return -result;
--- End diff --
Is it necessary to return this? Those values are used in
`decode_header_blocks` in Http2Stream.h to detect COMPRESSION_ERROR or
PROTOCOL_ERROR.
How about returning Http2ErrorCode directly?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
