[
https://issues.apache.org/jira/browse/TS-4092?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15205304#comment-15205304
]
ASF GitHub Bot commented on TS-4092:
------------------------------------
Github user bryancall commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/460#discussion_r56910134
--- Diff: proxy/http2/HTTP2.cc ---
@@ -484,246 +498,175 @@ 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) ||
--- End diff --
This wouldn't need to be done if the logic is in HPACK
> Decouple HPACK from HTTP/2
> --------------------------
>
> Key: TS-4092
> URL: https://issues.apache.org/jira/browse/TS-4092
> Project: Traffic Server
> Issue Type: Improvement
> Components: HTTP/2
> Reporter: Masakazu Kitajo
> Assignee: Masakazu Kitajo
> Fix For: 6.2.0
>
>
> Our HTTP/2 implementation and HPACK implementation are coupled tightly. This
> is bad. It makes things complicated.
> I tried to write a test runner which uses [hpack-test-case
> |https://github.com/http2jp/hpack-test-case], however, I had to call
> functions in HTTP2.cc. Because HPACK.cc has only primitive encoder and
> decoder, and doesn't handle header blocks. HTTP2.cc covers not only RFC7540
> but also some part of RFC7541.
> On the other hand, HPACK.h exports pseudo header names as constants. They
> should be in HTTP2.h or MIME.h as WKS. We don't need them in HPACK
> implementation.
> Also, HPACK is used with QUIC (at least in current draft). We should decouple
> HPACK from HTTP/2 so that we can use the module with QUIC in the future.
> Once we have done this, we can write tests for these improvements more easily.
> TS-4002, TS-4061, TS-4014 and TS-3478
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
