Skip to content

Commit 8092ec4

Browse files
Http2 to origin fix (#4)
* Fix for connection level window mismatch + force connection level window update * Fix for HPACK corruption on closed streams Todo: check stream level error returns, these should still decode the headers to prevent HPACK corruption for following header frames --------- Co-authored-by: Kees Spoelstra <kd.spoelstra@gmail.com>
1 parent c42037a commit 8092ec4

File tree

2 files changed

+19
-5
lines changed

2 files changed

+19
-5
lines changed

proxy/http2/Http2CommonSession.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,7 @@ Http2CommonSession::do_process_frame_read(int event, VIO *vio, bool inside_frame
369369
// Return if there was an error
370370
if (err > Http2ErrorCode::HTTP2_ERROR_NO_ERROR || do_start_frame_read(err) < 0) {
371371
// send an error if specified. Otherwise, just go away
372+
this->connection_state.restart_receiving(nullptr);
372373
if (err > Http2ErrorCode::HTTP2_ERROR_NO_ERROR) {
373374
if (!this->connection_state.is_state_closed()) {
374375
this->connection_state.send_goaway_frame(this->connection_state.get_latest_stream_id_in(), err);
@@ -387,6 +388,7 @@ Http2CommonSession::do_process_frame_read(int event, VIO *vio, bool inside_frame
387388

388389
if (this->_should_do_something_else()) {
389390
if (this->_reenable_event == nullptr) {
391+
this->connection_state.restart_receiving(nullptr);
390392
vio->disable();
391393
this->_reenable_event = this->get_mutex()->thread_holding->schedule_in(this->get_proxy_session(), HRTIME_MSECONDS(1),
392394
HTTP2_SESSION_EVENT_REENABLE, vio);
@@ -397,6 +399,7 @@ Http2CommonSession::do_process_frame_read(int event, VIO *vio, bool inside_frame
397399

398400
// If the client hasn't shut us down, reenable
399401
if (!this->get_proxy_session()->is_peer_closed()) {
402+
this->connection_state.restart_receiving(nullptr);
400403
vio->reenable();
401404
}
402405
return 0;

proxy/http2/Http2ConnectionState.cc

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,9 @@ Http2ConnectionState::rcv_data_frame(const Http2Frame &frame)
8989

9090
Http2StreamDebug(this->session, id, "Received DATA frame");
9191

92+
// Update connection window size, before any stream specific handling
93+
this->decrement_local_rwnd(payload_length);
94+
9295
if (this->get_zombie_event()) {
9396
Warning("Data frame for zombied session %" PRId64, this->session->get_connection_id());
9497
}
@@ -174,7 +177,8 @@ Http2ConnectionState::rcv_data_frame(const Http2Frame &frame)
174177
}
175178

176179
// Check whether Window Size is acceptable
177-
if (!this->_local_rwnd_is_shrinking && this->get_local_rwnd() < payload_length) {
180+
// compare to 0 because we already decreased the connection rwnd with payload_length
181+
if (!this->_local_rwnd_is_shrinking && this->get_local_rwnd() < 0) {
178182
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_FLOW_CONTROL_ERROR,
179183
"recv data this->local_rwnd < payload_length");
180184
}
@@ -183,8 +187,7 @@ Http2ConnectionState::rcv_data_frame(const Http2Frame &frame)
183187
"recv data stream->local_rwnd < payload_length");
184188
}
185189

186-
// Update Window size
187-
this->decrement_local_rwnd(payload_length);
190+
// Update stream window size
188191
stream->decrement_local_rwnd(payload_length);
189192

190193
if (is_debug_tag_set("http2_con")) {
@@ -316,9 +319,17 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame &frame)
316319
}
317320
}
318321

319-
// Ignoring HEADERS frame on a closed stream. The HdrHeap has gone away and it will core.
322+
// HEADERS frame on a closed stream. The HdrHeap has gone away and it will core.
320323
if (stream->get_state() == Http2StreamState::HTTP2_STREAM_STATE_CLOSED) {
321-
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE);
324+
Http2StreamDebug(session, stream_id, "Replaced closed stream");
325+
free_stream_after_decoding = true;
326+
stream = THREAD_ALLOC_INIT(http2StreamAllocator, this_ethread(), session->get_proxy_session(), stream_id,
327+
peer_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE), true, false);
328+
if (!stream) {
329+
// This happening is possibly catastrophic, the HPACK tables can be out of sync
330+
// Maybe this is a connection level error?
331+
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE);
332+
}
322333
}
323334

324335
Http2HeadersParameter params;

0 commit comments

Comments
 (0)