Don't send RST_STREAM to idle stream

orbea · Jun 18, 2020 · c8bf8c6 · c8bf8c6
1 parent b6b135c
commit c8bf8c6
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 2 deletions.
diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c
@@ -959,6 +959,18 @@ int nghttp2_session_add_rst_stream(nghttp2_session *session, int32_t stream_id,
     return 0;
   }
 
+  /* Sending RST_STREAM to an idle stream is subject to protocol
+     violation.  Historically, nghttp2 allows this.  In order not to
+     disrupt the existing applications, we don't error out this case
+     and simply ignore it. */
+  if (nghttp2_session_is_my_stream_id(session, stream_id)) {
+    if ((uint32_t)stream_id >= session->next_stream_id) {
+      return 0;
+    }
+  } else if (session->last_recv_stream_id < stream_id) {
+    return 0;
+  }
+
   /* Cancel pending request HEADERS in ob_syn if this RST_STREAM
      refers to that stream. */
   if (!session->server && nghttp2_session_is_my_stream_id(session, stream_id) &&
@@ -969,8 +981,7 @@ int nghttp2_session_add_rst_stream(nghttp2_session *session, int32_t stream_id,
     headers_frame = &nghttp2_outbound_queue_top(&session->ob_syn)->frame;
     assert(headers_frame->hd.type == NGHTTP2_HEADERS);
 
-    if (headers_frame->hd.stream_id <= stream_id &&
-        (uint32_t)stream_id < session->next_stream_id) {
+    if (headers_frame->hd.stream_id <= stream_id) {
 
       for (item = session->ob_syn.head; item; item = item->qnext) {
         aux_data = &item->aux_data.headers;

diff --git a/tests/main.c b/tests/main.c
@@ -211,6 +211,8 @@ int main() {
       !CU_add_test(pSuite, "submit_extension", test_nghttp2_submit_extension) ||
       !CU_add_test(pSuite, "submit_altsvc", test_nghttp2_submit_altsvc) ||
       !CU_add_test(pSuite, "submit_origin", test_nghttp2_submit_origin) ||
+      !CU_add_test(pSuite, "submit_rst_stream",
+                   test_nghttp2_submit_rst_stream) ||
       !CU_add_test(pSuite, "session_open_stream",
                    test_nghttp2_session_open_stream) ||
       !CU_add_test(pSuite, "session_open_stream_with_idle_stream_dep",

diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c
@@ -6398,6 +6398,107 @@ void test_nghttp2_submit_origin(void) {
   nghttp2_session_del(session);
 }
 
+void test_nghttp2_submit_rst_stream(void) {
+  nghttp2_session *session;
+  nghttp2_session_callbacks callbacks;
+  nghttp2_outbound_item *item;
+  int rv;
+  int32_t stream_id;
+
+  memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
+
+  /* Sending RST_STREAM to idle stream (local) is ignored */
+  nghttp2_session_client_new(&session, &callbacks, NULL);
+
+  rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, 1,
+                                 NGHTTP2_NO_ERROR);
+
+  CU_ASSERT(0 == rv);
+
+  item = nghttp2_outbound_queue_top(&session->ob_reg);
+
+  CU_ASSERT(NULL == item);
+
+  nghttp2_session_del(session);
+
+  /* Sending RST_STREAM to idle stream (remote) is ignored */
+  nghttp2_session_client_new(&session, &callbacks, NULL);
+
+  rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, 2,
+                                 NGHTTP2_NO_ERROR);
+
+  CU_ASSERT(0 == rv);
+
+  item = nghttp2_outbound_queue_top(&session->ob_reg);
+
+  CU_ASSERT(NULL == item);
+
+  nghttp2_session_del(session);
+
+  /* Sending RST_STREAM to non-idle stream (local) */
+  nghttp2_session_client_new(&session, &callbacks, NULL);
+
+  open_sent_stream(session, 1);
+
+  rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, 1,
+                                 NGHTTP2_NO_ERROR);
+
+  CU_ASSERT(0 == rv);
+
+  item = nghttp2_outbound_queue_top(&session->ob_reg);
+
+  CU_ASSERT(NULL != item);
+  CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type);
+  CU_ASSERT(1 == item->frame.hd.stream_id);
+
+  nghttp2_session_del(session);
+
+  /* Sending RST_STREAM to non-idle stream (remote) */
+  nghttp2_session_client_new(&session, &callbacks, NULL);
+
+  open_recv_stream(session, 2);
+
+  rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, 2,
+                                 NGHTTP2_NO_ERROR);
+
+  CU_ASSERT(0 == rv);
+
+  item = nghttp2_outbound_queue_top(&session->ob_reg);
+
+  CU_ASSERT(NULL != item);
+  CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type);
+  CU_ASSERT(2 == item->frame.hd.stream_id);
+
+  nghttp2_session_del(session);
+
+  /* Sending RST_STREAM to pending stream */
+  nghttp2_session_client_new(&session, &callbacks, NULL);
+
+  stream_id =
+      nghttp2_submit_request(session, NULL, reqnv, ARRLEN(reqnv), NULL, NULL);
+
+  CU_ASSERT(stream_id > 0);
+
+  item = nghttp2_outbound_queue_top(&session->ob_syn);
+
+  CU_ASSERT(NULL != item);
+  CU_ASSERT(NGHTTP2_HEADERS == item->frame.hd.type);
+  CU_ASSERT(0 == item->aux_data.headers.canceled);
+
+  rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, stream_id,
+                                 NGHTTP2_NO_ERROR);
+
+  CU_ASSERT(0 == rv);
+
+  item = nghttp2_outbound_queue_top(&session->ob_syn);
+
+  CU_ASSERT(NULL != item);
+  CU_ASSERT(NGHTTP2_HEADERS == item->frame.hd.type);
+  CU_ASSERT(1 == item->aux_data.headers.canceled);
+
+  nghttp2_session_del(session);
+}
+
 void test_nghttp2_session_open_stream(void) {
   nghttp2_session *session;
   nghttp2_session_callbacks callbacks;

diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h
@@ -103,6 +103,7 @@ void test_nghttp2_submit_invalid_nv(void);
 void test_nghttp2_submit_extension(void);
 void test_nghttp2_submit_altsvc(void);
 void test_nghttp2_submit_origin(void);
+void test_nghttp2_submit_rst_stream(void);
 void test_nghttp2_session_open_stream(void);
 void test_nghttp2_session_open_stream_with_idle_stream_dep(void);
 void test_nghttp2_session_get_next_ob_item(void);