Repository: trafficserver Updated Branches: refs/heads/6.0.x 6062188f5 -> 252eb2131
TS-3801: Add error handling when stream is CLOSED This closes #269 Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/2ce049b0 Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/2ce049b0 Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/2ce049b0 Branch: refs/heads/6.0.x Commit: 2ce049b037328e5788f35cff18d1be5a4f3bd83a Parents: 35c5a45 Author: Masaori Koshiba <mkosh...@yahoo-corp.jp> Authored: Mon Jul 27 17:18:53 2015 +0900 Committer: Leif Hedstrom <zw...@apache.org> Committed: Mon Aug 3 20:08:24 2015 -0600 ---------------------------------------------------------------------- proxy/http2/Http2ConnectionState.cc | 52 +++++++++++++++++++++++--------- proxy/http2/Http2ConnectionState.h | 10 ++++++ 2 files changed, 48 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2ce049b0/proxy/http2/Http2ConnectionState.cc ---------------------------------------------------------------------- diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc index 3bec688..ca7abeb 100644 --- a/proxy/http2/Http2ConnectionState.cc +++ b/proxy/http2/Http2ConnectionState.cc @@ -66,18 +66,24 @@ rcv_data_frame(Http2ClientSession &cs, Http2ConnectionState &cstate, const Http2 char buf[BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_DATA])]; unsigned nbytes = 0; Http2StreamId id = frame.header().streamid; - Http2Stream *stream = cstate.find_stream(id); uint8_t pad_length = 0; const uint32_t payload_length = frame.header().length; DebugSsn(&cs, "http2_cs", "[%" PRId64 "] Received DATA frame.", cs.connection_id()); - // If a DATA frame is received whose stream identifier field is 0x0, the recipient MUST - // respond with a connection error of type PROTOCOL_ERROR. - if (id == 0 || stream == NULL) { + if (!http2_is_client_streamid(id)) { return HTTP2_ERROR_PROTOCOL_ERROR; } + Http2Stream *stream = cstate.find_stream(id); + if (stream == NULL) { + if (id <= cstate.get_latest_stream_id()) { + return HTTP2_ERROR_STREAM_CLOSED; + } else { + return HTTP2_ERROR_PROTOCOL_ERROR; + } + } + // If a DATA frame is received whose stream is not in "open" or "half closed (local)" state, // the recipient MUST respond with a stream error of type STREAM_CLOSED. if (stream->get_state() != HTTP2_STREAM_STATE_OPEN && stream->get_state() != HTTP2_STREAM_STATE_HALF_CLOSED_LOCAL) { @@ -163,8 +169,13 @@ rcv_headers_frame(Http2ClientSession &cs, Http2ConnectionState &cstate, const Ht return HTTP2_ERROR_PROTOCOL_ERROR; } + Http2Stream *stream = cstate.find_stream(id); + if (stream == NULL && id <= cstate.get_latest_stream_id()) { + return HTTP2_ERROR_STREAM_CLOSED; + } + // Create new stream - Http2Stream *stream = cstate.create_stream(id); + stream = cstate.create_stream(id); if (!stream) { return HTTP2_ERROR_PROTOCOL_ERROR; } @@ -290,11 +301,21 @@ rcv_rst_stream_frame(Http2ClientSession &cs, Http2ConnectionState &cstate, const DebugSsn(&cs, "http2_cs", "[%" PRId64 "] Received RST_STREAM frame.", cs.connection_id()); - Http2Stream *stream = cstate.find_stream(frame.header().streamid); - if (frame.header().streamid == 0) { + Http2StreamId stream_id = frame.header().streamid; + + if (!http2_is_client_streamid(stream_id)) { return HTTP2_ERROR_PROTOCOL_ERROR; } + Http2Stream *stream = cstate.find_stream(stream_id); + if (stream == NULL) { + if (stream_id <= cstate.get_latest_stream_id()) { + return HTTP2_ERROR_NO_ERROR; + } else { + return HTTP2_ERROR_PROTOCOL_ERROR; + } + } + if (frame.header().length != HTTP2_RST_STREAM_LEN) { return HTTP2_ERROR_FRAME_SIZE_ERROR; } @@ -480,13 +501,12 @@ rcv_window_update_frame(Http2ClientSession &cs, Http2ConnectionState &cstate, co // Stream level window update Http2Stream *stream = cstate.find_stream(sid); - // This means that a receiver could receive a - // WINDOW_UPDATE frame on a "half closed (remote)" or "closed" stream. - // A receiver MUST NOT treat this as an error. if (stream == NULL) { - // Temporarily ignore WINDOW_UPDATE - // TODO After supporting PRIORITY, it should be handled correctly. - return HTTP2_ERROR_NO_ERROR; + if (sid <= cstate.get_latest_stream_id()) { + return HTTP2_ERROR_NO_ERROR; + } else { + return HTTP2_ERROR_PROTOCOL_ERROR; + } } frame.reader()->memcpy(buf, sizeof(buf), 0); @@ -521,7 +541,11 @@ rcv_continuation_frame(Http2ClientSession &cs, Http2ConnectionState &cstate, con // Find opened stream Http2Stream *stream = cstate.find_stream(stream_id); if (stream == NULL) { - return HTTP2_ERROR_PROTOCOL_ERROR; + if (stream_id <= cstate.get_latest_stream_id()) { + return HTTP2_ERROR_STREAM_CLOSED; + } else { + return HTTP2_ERROR_PROTOCOL_ERROR; + } } // A CONTINUATION frame MUST be preceded by a HEADERS, PUSH_PROMISE or http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2ce049b0/proxy/http2/Http2ConnectionState.h ---------------------------------------------------------------------- diff --git a/proxy/http2/Http2ConnectionState.h b/proxy/http2/Http2ConnectionState.h index 6ba0753..14d7f01 100644 --- a/proxy/http2/Http2ConnectionState.h +++ b/proxy/http2/Http2ConnectionState.h @@ -249,6 +249,12 @@ public: void update_initial_rwnd(Http2WindowSize new_size); + Http2StreamId + get_latest_stream_id() const + { + return latest_streamid; + } + // Continuated header decoding Http2StreamId get_continued_id() const @@ -285,6 +291,10 @@ private: Http2ConnectionState(const Http2ConnectionState &); // noncopyable Http2ConnectionState &operator=(const Http2ConnectionState &); // noncopyable + // NOTE: 'stream_list' has only active streams. + // If given Stream Identifier is not found in stream_list and it is less than or equal to latest_streamid, the state of Stream + // is CLOSED. + // If given Stream Identifier is not found in stream_list and it is greater than latest_streamid, the state of Stream is IDLE. DLL<Http2Stream> stream_list; Http2StreamId latest_streamid;