Discard inbound HEADERS and DATA in CLOSING state. Handle stream shutdown when DATA is received with FIN bit set.

lib/spdylay_session.c

@@ -875,7 +875,7 @@ int spdylay_session_on_headers_received(spdylay_session *session,
   /* First we check readability from this stream. */
   if(stream && (stream->shut_flags & SPDYLAY_SHUT_RD) == 0) {
     if(spdylay_session_is_my_stream_id(session, frame->headers.stream_id)) {
-      if(stream && stream->state == SPDYLAY_STREAM_OPENED) {
+      if(stream->state == SPDYLAY_STREAM_OPENED) {
         valid = 1;
         spdylay_session_call_on_ctrl_frame_received(session, SPDYLAY_HEADERS,
                                                     frame);
@@ -891,8 +891,11 @@ int spdylay_session_on_headers_received(spdylay_session *session,
       }
     } else {
       /* If this is remote peer initiated stream, it is OK unless it
-         have sent FIN frame already. */
+         have sent FIN frame already. But if stream is in
+         SPDYLAY_STREAM_CLOSING, we discard the frame. This is a race
+         condition. */
       valid = 1;
+      if(stream->state != SPDYLAY_STREAM_CLOSING) {
         spdylay_session_call_on_ctrl_frame_received(session, SPDYLAY_HEADERS,
                                                     frame);
         if(frame->headers.hd.flags & SPDYLAY_FLAG_FIN) {
@@ -901,6 +904,7 @@ int spdylay_session_on_headers_received(spdylay_session *session,
         }
       }
     }
+  }
   if(!valid) {
     r = spdylay_session_handle_invalid_stream
       (session, frame->headers.stream_id, SPDYLAY_HEADERS, frame,
@@ -1000,20 +1004,14 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session)
   }
 }
 
-static int spdylay_session_process_data_frame(spdylay_session *session)
+int spdylay_session_on_data_received(spdylay_session *session,
+                                     uint8_t flags, int32_t length,
+                                     int32_t stream_id)
 {
-  int32_t stream_id;
-  uint8_t flags;
-  int32_t length;
-  spdylay_stream *stream;
-  int status_code = 0;
   int valid = 0;
   int r = 0;
-  stream_id = spdylay_get_uint32(session->iframe.headbuf) &
+  spdylay_status_code status_code = 0;
-    SPDYLAY_STREAM_ID_MASK;
+  spdylay_stream *stream;
-  flags = session->iframe.headbuf[4];
-  length = spdylay_get_uint32(&session->iframe.headbuf[4]) &
-    SPDYLAY_LENGTH_MASK;
   stream = spdylay_session_get_stream(session, stream_id);
   if(stream) {
     if((stream->shut_flags & SPDYLAY_SHUT_RD) == 0) {
@@ -1023,11 +1021,18 @@ static int spdylay_session_process_data_frame(spdylay_session *session)
         } else if(stream->state != SPDYLAY_STREAM_CLOSING) {
           status_code = SPDYLAY_PROTOCOL_ERROR;
         }
-      } else {
+      } else if(stream->state != SPDYLAY_STREAM_CLOSING) {
         /* It is OK if this is remote peer initiated stream and we did
-           not receive FIN. */
+           not receive FIN unless stream is in SPDYLAY_STREAM_CLOSING
+           state. This is a race condition. */
         valid = 1;
       }
+      if(valid) {
+        if(flags & SPDYLAY_FLAG_FIN) {
+          spdylay_stream_shutdown(stream, SPDYLAY_SHUT_RD);
+          spdylay_session_close_stream_if_shut_rdwr(session, stream);
+        }
+      }
     } else {
       status_code = SPDYLAY_PROTOCOL_ERROR;
     }
@@ -1042,6 +1047,21 @@ static int spdylay_session_process_data_frame(spdylay_session *session)
   } else if(status_code != 0) {
     r = spdylay_session_add_rst_stream(session, stream_id, status_code);
   }
+  return r;
+}
+
+static int spdylay_session_process_data_frame(spdylay_session *session)
+{
+  uint8_t flags;
+  int32_t length;
+  int32_t stream_id;
+  int r;
+  stream_id = spdylay_get_uint32(session->iframe.headbuf) &
+    SPDYLAY_STREAM_ID_MASK;
+  flags = session->iframe.headbuf[4];
+  length = spdylay_get_uint32(&session->iframe.headbuf[4]) &
+    SPDYLAY_LENGTH_MASK;
+  r = spdylay_session_on_data_received(session, flags, length, stream_id);
   if(r < SPDYLAY_ERR_FATAL) {
     return r;
   } else {

lib/spdylay_session.h

@@ -208,6 +208,13 @@ int spdylay_session_on_goaway_received(spdylay_session *session,
 int spdylay_session_on_headers_received(spdylay_session *session,
                                         spdylay_frame *frame);
 
+/*
+ * Called when DATA is received.
+ */
+int spdylay_session_on_data_received(spdylay_session *session,
+                                     uint8_t flags, int32_t length,
+                                     int32_t stream_id);
+
 /*
  * Returns spdylay_stream* object whose stream ID is |stream_id|.  It
  * could be NULL if such stream does not exist.

tests/main.c

@@ -88,6 +88,8 @@ int main()
                    test_spdylay_session_on_ping_received) ||
       !CU_add_test(pSuite, "session_on_goaway_received",
                    test_spdylay_session_on_goaway_received) ||
+      !CU_add_test(pSuite, "session_on_data_received",
+                   test_spdylay_session_on_data_received) ||
       !CU_add_test(pSuite, "frame_unpack_nv", test_spdylay_frame_unpack_nv) ||
       !CU_add_test(pSuite, "frame_count_nv_space",
                    test_spdylay_frame_count_nv_space) ||

tests/spdylay_session_test.c

@@ -330,6 +330,7 @@ void test_spdylay_session_on_syn_reply_received()
   my_user_data user_data;
   const char *nv[] = { NULL };
   spdylay_frame frame;
+  spdylay_stream *stream;
   user_data.valid = 0;
   user_data.invalid = 0;
 
@@ -349,6 +350,16 @@ void test_spdylay_session_on_syn_reply_received()
   CU_ASSERT(SPDYLAY_STREAM_CLOSING ==
             ((spdylay_stream*)spdylay_map_find(&session->streams, 1))->state);
 
+  /* Check the situation when SYN_REPLY is received after peer sends
+     FIN */
+  stream = spdylay_session_open_stream(session, 3, SPDYLAY_FLAG_NONE, 0,
+                                       SPDYLAY_STREAM_OPENED);
+  spdylay_stream_shutdown(stream, SPDYLAY_SHUT_RD);
+  frame.syn_reply.stream_id = 3;
+
+  CU_ASSERT(0 == spdylay_session_on_syn_reply_received(session, &frame));
+  CU_ASSERT(2 == user_data.invalid);
+
   spdylay_frame_syn_reply_free(&frame.syn_reply);
   spdylay_session_del(session);
 }
@@ -522,6 +533,16 @@ void test_spdylay_session_on_headers_received()
   CU_ASSERT(0 == spdylay_session_on_headers_received(session, &frame));
   CU_ASSERT(1 == user_data.invalid);
 
+  /* Check to see when SPDYLAY_STREAM_CLOSING, incoming HEADERS is
+     discarded. */
+  spdylay_session_open_stream(session, 3, SPDYLAY_FLAG_NONE, 0,
+                              SPDYLAY_STREAM_CLOSING);
+  frame.headers.stream_id = 3;
+  frame.headers.hd.flags = SPDYLAY_FLAG_NONE;
+  CU_ASSERT(0 == spdylay_session_on_headers_received(session, &frame));
+  CU_ASSERT(2 == user_data.valid);
+  CU_ASSERT(1 == user_data.invalid);
+
   /* Server initiated stream */
   spdylay_session_open_stream(session, 2, SPDYLAY_FLAG_NONE, 0,
                               SPDYLAY_STREAM_OPENING);
@@ -609,3 +630,46 @@ void test_spdylay_session_on_goaway_received()
   spdylay_frame_goaway_free(&frame.goaway);
   spdylay_session_del(session);
 }
+
+void test_spdylay_session_on_data_received()
+{
+  spdylay_session *session;
+  spdylay_session_callbacks callbacks;
+  memset(&callbacks, 0, sizeof(spdylay_session_callbacks));
+  my_user_data user_data;
+  spdylay_outbound_item *top;
+  int32_t stream_id = 2;
+  spdylay_stream *stream;
+
+  spdylay_session_client_new(&session, &callbacks, &user_data);
+  stream = spdylay_session_open_stream(session, stream_id, SPDYLAY_FLAG_NONE,
+                                       3, SPDYLAY_STREAM_OPENING);
+  CU_ASSERT(0 == spdylay_session_on_data_received(session, SPDYLAY_FLAG_NONE,
+                                                  4096, stream_id));
+  CU_ASSERT(0 == stream->shut_flags);
+
+  CU_ASSERT(0 == spdylay_session_on_data_received(session, SPDYLAY_FLAG_FIN,
+                                                  4096, stream_id));
+  CU_ASSERT(SPDYLAY_SHUT_RD == stream->shut_flags);
+
+  /* If SPDYLAY_STREAM_CLOSING state, DATA frame is discarded. */
+  stream_id = 4;
+
+  spdylay_session_open_stream(session, stream_id, SPDYLAY_FLAG_NONE,
+                              3, SPDYLAY_STREAM_CLOSING);
+  CU_ASSERT(0 == spdylay_session_on_data_received(session, SPDYLAY_FLAG_NONE,
+                                                  4096, stream_id));
+  CU_ASSERT(NULL == spdylay_session_get_ob_pq_top(session));
+
+  /* Check INVALID_STREAM case: DATA frame with stream ID which does
+     not exist. */
+  stream_id = 6;
+
+  CU_ASSERT(0 == spdylay_session_on_data_received(session, SPDYLAY_FLAG_NONE,
+                                                  4096, stream_id));
+  top = spdylay_session_get_ob_pq_top(session);
+  CU_ASSERT(SPDYLAY_RST_STREAM == top->frame_type);
+  CU_ASSERT(SPDYLAY_INVALID_STREAM == top->frame->rst_stream.status_code);
+
+  spdylay_session_del(session);
+}

tests/spdylay_session_test.h

@@ -37,5 +37,6 @@ void test_spdylay_session_reply_fail();
 void test_spdylay_session_on_headers_received();
 void test_spdylay_session_on_ping_received();
 void test_spdylay_session_on_goaway_received();
+void test_spdylay_session_on_data_received();
 
 #endif // SPDYLAY_SESSION_TEST_H