Skip to content

Http/2 to origin fixes #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
keesspoelstra wants to merge 4 commits into from
Closed

Http/2 to origin fixes #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
36ec24e
Connection error should be stream error according to https://datatrac…
keess2 Nov 2, 2021
e07ed26
Do not sent EOS or WRITE_COMPLETE to tunnel handlers which are not se…
keess2 Nov 2, 2021
fa3bc7a
Fix assertion crash on cause_of_death_errno in case of parse response…
keess2 Nov 2, 2021
d631617
Set end stream on request headers if method post etc & explicit length 0
keess2 Nov 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions proxy/http/HttpSM.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2262,6 +2262,10 @@ HttpSM::state_read_server_response_header(int event, void *data)
if (allow_error == false) {
SMDebug("http_seq", "Error parsing server response header");
t_state.current.state = HttpTransact::PARSE_ERROR;
// set to 0, because else HttpTransact::retry_server_connection_not_open
// will assert on default value
// TODO: can we use a better value here?
t_state.cause_of_death_errno = 0;

// If the server closed prematurely on us, use the
// server setup error routine since it will forward
Expand Down
20 changes: 14 additions & 6 deletions proxy/http2/Http2ConnectionState.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ rcv_data_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE);
}
if (!stream->payload_length_is_valid()) {
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
"recv data bad payload length");
}

Expand Down Expand Up @@ -2007,11 +2007,19 @@ Http2ConnectionState::send_headers_frame(Http2Stream *stream)
payload_length = header_blocks_size;
flags |= HTTP2_FLAGS_HEADERS_END_HEADERS;
if (stream->is_outbound_connection()) { // Will be sending a request_header
int method = send_hdr->method_get_wksidx();
if (!(method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT) &&
!send_hdr->presence(MIME_PRESENCE_TRANSFER_ENCODING) &&
((send_hdr->presence(MIME_PRESENCE_CONTENT_LENGTH) && send_hdr->get_content_length() == 0) ||
!send_hdr->presence(MIME_PRESENCE_CONTENT_LENGTH))) {
int method = send_hdr->method_get_wksidx();
bool content_method = method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT;
bool is_transfer_encoded = send_hdr->presence(MIME_PRESENCE_TRANSFER_ENCODING);
bool has_content_header = send_hdr->presence(MIME_PRESENCE_CONTENT_LENGTH);
bool explicit_zero_length = has_content_header && send_hdr->get_content_length() == 0;

bool expect_content_stream =
is_transfer_encoded || // transfer encoded content length is unknown
(!content_method && has_content_header && !explicit_zero_length) || // non zero content with GET,etc
(content_method && !explicit_zero_length); // content-length >0 or empty with POST etc

// send END_STREAM if we don't expect any content
if (!expect_content_stream) {
// TODO deal with the chunked encoding case
Http2StreamDebug(session, stream->get_id(), "request END_STREAM");
flags |= HTTP2_FLAGS_HEADERS_END_STREAM;
Expand Down
7 changes: 5 additions & 2 deletions proxy/http2/Http2Stream.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,8 @@ Http2Stream::initiating_close()
if (_sm) {
// Push out any last IO events
// First look for active write or read
if (write_vio.cont && write_vio.nbytes > 0) {
if (write_vio.cont && write_vio.nbytes > 0 &&
(!is_outbound_connection() || get_state() == Http2StreamState::HTTP2_STREAM_STATE_OPEN)) {
SCOPED_MUTEX_LOCK(lock, write_vio.mutex, this_ethread());
if (write_vio.nbytes > 0 && write_vio.ndone == write_vio.nbytes) {
write_event = send_tracked_event(write_event, VC_EVENT_WRITE_COMPLETE, &write_vio);
Expand All @@ -581,7 +582,9 @@ Http2Stream::initiating_close()
}

if (!sent_io_event) {
if (write_vio.cont && write_vio.buffer.writer()) {
if (write_vio.cont && write_vio.buffer.writer() &&
(!is_outbound_connection() || get_state() == Http2StreamState::HTTP2_STREAM_STATE_OPEN ||
get_state() == Http2StreamState::HTTP2_STREAM_STATE_HALF_CLOSED_LOCAL)) {
SCOPED_MUTEX_LOCK(lock, write_vio.mutex, this_ethread());
Http2StreamDebug("handle write from destroy (event=%d)", VC_EVENT_EOS);
write_event = send_tracked_event(write_event, VC_EVENT_EOS, &write_vio);
Expand Down