[
https://issues.apache.org/jira/browse/TS-4092?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15153539#comment-15153539
]
ASF GitHub Bot commented on TS-4092:
------------------------------------
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/460#discussion_r53414029
--- Diff: proxy/http2/Http2ConnectionState.cc ---
@@ -992,43 +998,50 @@ Http2ConnectionState::send_headers_frame(FetchSM
*fetch_sm)
DebugHttp2Stream(ua_session, stream->get_id(), "Send HEADERS frame");
- // Write pseudo headers
- payload_length += http2_write_psuedo_headers(resp_header,
payload_buffer, buf_len, *(this->remote_indexing_table));
-
- // If response body is empty, set END_STREAM flag to HEADERS frame
- // Must check to ensure content-length is there. Otherwise the value
defaults
- // to 0
- if (resp_header->presence(MIME_PRESENCE_CONTENT_LENGTH) &&
resp_header->get_content_length() == 0) {
- flags |= HTTP2_FLAGS_HEADERS_END_STREAM;
+ http2_convert_header_from_1_1_to_2(resp_header);
+ buf_len = resp_header->length_get() * 2; // Make it double just in case
+ buf = (uint8_t *)ats_malloc(buf_len);
+ if (buf == NULL) {
+ return;
+ }
+ header_blocks_size = http2_encode_header_blocks(resp_header, buf,
buf_len, *(this->remote_hpack_handle));
+ if (header_blocks_size < 0) {
+ ats_free(buf);
+ return;
}
- MIMEFieldIter field_iter;
- bool cont = false;
- do {
- // Handle first sending frame is as HEADERS
- Http2FrameType type = cont ? HTTP2_FRAME_TYPE_CONTINUATION :
HTTP2_FRAME_TYPE_HEADERS;
-
- // Encode by HPACK naive
- payload_length += http2_write_header_fragment(resp_header, field_iter,
payload_buffer + payload_length,
- buf_len -
payload_length, *(this->remote_indexing_table), cont);
-
- // If buffer size is enough to send rest of headers, set END_HEADERS
flag
- if (buf_len >= payload_length && !cont) {
- flags |= HTTP2_FLAGS_HEADERS_END_HEADERS;
- }
-
- // Create HEADERS or CONTINUATION frame
- Http2Frame headers(type, stream->get_id(), flags);
- headers.alloc(buffer_size_index[type]);
- http2_write_headers(payload_buffer, payload_length, headers.write());
+ // Send a HEADERS frame
+ if (header_blocks_size <=
BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_HEADERS]) -
HTTP2_FRAME_HEADER_LEN) {
+ payload_length = header_blocks_size;
+ flags |= HTTP2_FLAGS_HEADERS_END_HEADERS;
+ } else {
+ payload_length =
BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_HEADERS]) -
HTTP2_FRAME_HEADER_LEN;
+ }
+ Http2Frame headers(HTTP2_FRAME_TYPE_HEADERS, stream->get_id(), flags);
+ headers.alloc(buffer_size_index[HTTP2_FRAME_TYPE_HEADERS]);
+ http2_write_headers(buf, payload_length, headers.write());
+ headers.finalize(payload_length);
+ // xmit event
+ SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
+ this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &headers);
+ sent += payload_length;
+
+ // Send CONTINUATION frames
+ while (sent < header_blocks_size) {
+ DebugSsn(this->ua_session, "http2_cs", "[%" PRId64 "] Send
CONTINUATION frame.", this->ua_session->connection_id());
--- End diff --
It is better to use `DebugHttp2Stream`.
> 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)
