trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zw...@apache.org
Subject [1/4] trafficserver git commit: TS-3801: Add error handling when stream is CLOSED
Date Tue, 04 Aug 2015 16:04:36 GMT
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 <mkoshiba@yahoo-corp.jp>
Authored: Mon Jul 27 17:18:53 2015 +0900
Committer: Leif Hedstrom <zwoop@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;
 


Mime
View raw message