maskit commented on code in PR #8963:
URL: https://github.com/apache/trafficserver/pull/8963#discussion_r921885168
##########
proxy/http2/Http2ConnectionState.cc:
##########
@@ -1756,16 +1980,26 @@ Http2ConnectionState::send_headers_frame(Http2Stream
*stream)
Http2StreamDebug(session, stream->get_id(), "Send HEADERS frame");
- HTTPHdr *resp_hdr = &stream->response_header;
- http2_convert_header_from_1_1_to_2(resp_hdr);
+ // For outbound streams, set the ID if it has not yet already been set
+ // Need to defer setting the stream ID to avoid another later created stream
+ // sending out first. This may cause the peer to issue a stream or
connection
+ // error (new stream less that the greatest we have seen so far)
+ this->set_stream_id(stream);
+
+ HTTPHdr *send_hdr = stream->get_send_header();
+ if (stream->expect_send_trailer()) {
+ // Which is a no-op conversion
+ } else {
+ http2_convert_header_from_1_1_to_2(send_hdr);
+ }
- uint32_t buf_len = resp_hdr->length_get() * 2; // Make it double just in case
+ uint32_t buf_len = send_hdr->length_get() * 2; // Make it double just in case
Review Comment:
Would make another PR for this kind of name changes (e.g. send_hdr,
peer_settings, local_settings, etc.)? That would reduce the volume of this PR
(which is great for reviewers so they can focus on the logical changes for H2
to Origin), and would also ease maintaining this branch.
##########
proxy/http2/Http2ConnectionState.cc:
##########
@@ -1756,16 +1980,26 @@ Http2ConnectionState::send_headers_frame(Http2Stream
*stream)
Http2StreamDebug(session, stream->get_id(), "Send HEADERS frame");
- HTTPHdr *resp_hdr = &stream->response_header;
- http2_convert_header_from_1_1_to_2(resp_hdr);
+ // For outbound streams, set the ID if it has not yet already been set
+ // Need to defer setting the stream ID to avoid another later created stream
+ // sending out first. This may cause the peer to issue a stream or
connection
+ // error (new stream less that the greatest we have seen so far)
+ this->set_stream_id(stream);
+
+ HTTPHdr *send_hdr = stream->get_send_header();
+ if (stream->expect_send_trailer()) {
+ // Which is a no-op conversion
+ } else {
+ http2_convert_header_from_1_1_to_2(send_hdr);
+ }
- uint32_t buf_len = resp_hdr->length_get() * 2; // Make it double just in case
+ uint32_t buf_len = send_hdr->length_get() * 2; // Make it double just in case
Review Comment:
Would you make another PR for this kind of name changes (e.g. send_hdr,
peer_settings, local_settings, etc.)? That would reduce the volume of this PR
(which is great for reviewers so they can focus on the logical changes for H2
to Origin), and would also ease maintaining this branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
