Send GOAWAY when spdylay_frame_unpack_* is failed with non-fatal error.

SPDYLAY_ERR_ZLIB is now moved back to non-fatal error and it is subject
to GOAWAY if it occurred.

lib/includes/spdylay/spdylay.h

@@ -43,6 +43,7 @@ typedef struct spdylay_session spdylay_session;
 
 typedef enum {
   SPDYLAY_ERR_INVALID_ARGUMENT = -501,
+  SPDYLAY_ERR_ZLIB = -502,
   SPDYLAY_ERR_WOULDBLOCK = -504,
   SPDYLAY_ERR_PROTO = -505,
   SPDYLAY_ERR_INVALID_FRAME = -506,
@@ -54,7 +55,6 @@ typedef enum {
   SPDYLAY_ERR_FATAL = -900,
   SPDYLAY_ERR_NOMEM = -901,
   SPDYLAY_ERR_CALLBACK_FAILURE = -902,
-  SPDYLAY_ERR_ZLIB = -903,
 } spdylay_error;
 
 typedef enum {
@@ -373,12 +373,20 @@ int spdylay_session_recv(spdylay_session *session);
 /*
  * Returns non-zero value if |session| want to receive data from the
  * remote peer, or 0.
+ *
+ * if both spdylay_session_want_read() and
+ * spdylay_session_want_write() return 0, the application should drop
+ * the connection.
  */
 int spdylay_session_want_read(spdylay_session *session);
 
 /*
  * Returns non-zero value if |session| want to send data to the remote
  * peer, or 0.
+ *
+ * if both spdylay_session_want_read() and
+ * spdylay_session_want_write() return 0, the application should drop
+ * the connection.
  */
 int spdylay_session_want_write(spdylay_session *session);
 

lib/spdylay_session.c

@@ -1299,6 +1299,25 @@ int spdylay_session_on_headers_received(spdylay_session *session,
   return r;
 }
 
+/*
+ * This function should be called when the session wants to drop
+ * connection after sending GOAWAY. For example, when it receives bad
+ * zlib data.
+ */
+static int spdylay_session_fail_session(spdylay_session *session)
+{
+  session->goaway_flags |= SPDYLAY_GOAWAY_FAIL_ON_SEND;
+  return spdylay_submit_goaway(session);
+}
+
+/*
+ * Returns non-zero if |error| is non-fatal error.
+ */
+static int spdylay_is_non_fatal(int error)
+{
+  return error < 0 && error > SPDYLAY_ERR_FATAL;
+}
+
 static int spdylay_session_process_ctrl_frame(spdylay_session *session)
 {
   int r = 0;
@@ -1321,10 +1340,11 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session)
     if(r == 0) {
       r = spdylay_session_on_syn_stream_received(session, &frame);
       spdylay_frame_syn_stream_free(&frame.syn_stream);
-    } else {
       /* TODO if r indicates mulformed NV pairs (multiple nulls) or
          invalid frame, send RST_STREAM with PROTOCOL_ERROR. Same for
          other control frames. */
+    } else if(spdylay_is_non_fatal(r)) {
+      r = spdylay_session_fail_session(session);
     }
     break;
   case SPDYLAY_SYN_REPLY:
@@ -1341,6 +1361,8 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session)
     if(r == 0) {
       r = spdylay_session_on_syn_reply_received(session, &frame);
       spdylay_frame_syn_reply_free(&frame.syn_reply);
+    } else if(spdylay_is_non_fatal(r)) {
+      r = spdylay_session_fail_session(session);
     }
     break;
   case SPDYLAY_RST_STREAM:
@@ -1352,6 +1374,8 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session)
     if(r == 0) {
       r = spdylay_session_on_rst_stream_received(session, &frame);
       spdylay_frame_rst_stream_free(&frame.rst_stream);
+    } else if(spdylay_is_non_fatal(r)) {
+      r = spdylay_session_fail_session(session);
     }
     break;
   case SPDYLAY_SETTINGS:
@@ -1363,6 +1387,8 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session)
     if(r == 0) {
       r = spdylay_session_on_settings_received(session, &frame);
       spdylay_frame_settings_free(&frame.settings);
+    } else if(spdylay_is_non_fatal(r)) {
+      r = spdylay_session_fail_session(session);
     }
     break;
   case SPDYLAY_NOOP:
@@ -1376,6 +1402,8 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session)
     if(r == 0) {
       r = spdylay_session_on_ping_received(session, &frame);
       spdylay_frame_ping_free(&frame.ping);
+    } else if(spdylay_is_non_fatal(r)) {
+      r = spdylay_session_fail_session(session);
     }
     break;
   case SPDYLAY_GOAWAY:
@@ -1387,6 +1415,8 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session)
     if(r == 0) {
       r = spdylay_session_on_goaway_received(session, &frame);
       spdylay_frame_goaway_free(&frame.goaway);
+    } else if(spdylay_is_non_fatal(r)) {
+      r = spdylay_session_fail_session(session);
     }
     break;
   case SPDYLAY_HEADERS:
@@ -1403,6 +1433,8 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session)
     if(r == 0) {
       r = spdylay_session_on_headers_received(session, &frame);
       spdylay_frame_headers_free(&frame.headers);
+    } else if(spdylay_is_non_fatal(r)) {
+      r = spdylay_session_fail_session(session);
     }
     break;
   }
@@ -1577,6 +1609,12 @@ int spdylay_session_recv(spdylay_session *session)
 
 int spdylay_session_want_read(spdylay_session *session)
 {
+  /* If these flags are set, we don't want to read. The application
+     should drop the connection. */
+  if((session->goaway_flags & SPDYLAY_GOAWAY_FAIL_ON_SEND) &&
+     (session->goaway_flags & SPDYLAY_GOAWAY_SEND)) {
+    return 0;
+  }
   /* Unless GOAWAY is sent or received, we always want to read
      incoming frames. After GOAWAY is sent or received, we are only
      interested in active streams. */
@@ -1585,6 +1623,12 @@ int spdylay_session_want_read(spdylay_session *session)
 
 int spdylay_session_want_write(spdylay_session *session)
 {
+  /* If these flags are set, we don't want to write any data. The
+     application should drop the connection. */
+  if((session->goaway_flags & SPDYLAY_GOAWAY_FAIL_ON_SEND) &&
+     (session->goaway_flags & SPDYLAY_GOAWAY_SEND)) {
+    return 0;
+  }
   /*
    * Unless GOAWAY is sent or received, we want to write frames if
    * there is pending ones. If pending frame is SYN_STREAM and

lib/spdylay_session.h

@@ -104,7 +104,9 @@ typedef enum {
   /* Flag means GOAWAY frame is sent to the remote peer. */
   SPDYLAY_GOAWAY_SEND = 0x1,
   /* Flag means GOAWAY frame is received from the remote peer. */
-  SPDYLAY_GOAWAY_RECV = 0x2
+  SPDYLAY_GOAWAY_RECV = 0x2,
+  /* Flag means connection should be dropped after sending GOAWAY. */
+  SPDYLAY_GOAWAY_FAIL_ON_SEND = 0x4
 } spdylay_goaway_flag;
 
 struct spdylay_session {

tests/main.c

@@ -119,6 +119,8 @@ int main(int argc, char* argv[])
                    test_spdylay_session_stop_data_with_rst_stream) ||
       !CU_add_test(pSuite, "session_stream_close_on_syn_stream",
                    test_spdylay_session_stream_close_on_syn_stream) ||
+      !CU_add_test(pSuite, "session_recv_invalid_frame",
+                   test_spdylay_session_recv_invalid_frame) ||
       !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

@@ -1214,3 +1214,52 @@ void test_spdylay_session_stream_close_on_syn_stream()
   spdylay_frame_syn_stream_free(&frame.syn_stream);
   spdylay_session_del(session);
 }
+
+void test_spdylay_session_recv_invalid_frame()
+{
+  spdylay_session *session;
+  spdylay_session_callbacks callbacks;
+  scripted_data_feed df;
+  my_user_data user_data;
+  const char *nv[] = {
+    "url", "/", NULL
+  };
+  uint8_t *framedata = NULL, *nvbuf = NULL;
+  size_t framedatalen = 0, nvbuflen = 0;
+  ssize_t framelen;
+  spdylay_frame frame;
+
+  memset(&callbacks, 0, sizeof(spdylay_session_callbacks));
+  callbacks.recv_callback = scripted_recv_callback;
+  callbacks.send_callback = null_send_callback;
+  callbacks.on_ctrl_send_callback = on_ctrl_send_callback;
+
+  user_data.df = &df;
+  user_data.ctrl_send_cb_called = 0;
+  spdylay_session_server_new(&session, &callbacks, &user_data);
+  spdylay_frame_syn_stream_init(&frame.syn_stream, SPDYLAY_FLAG_NONE, 1, 0, 3,
+                                dup_nv(nv));
+  framelen = spdylay_frame_pack_syn_stream(&framedata, &framedatalen,
+                                           &nvbuf, &nvbuflen,
+                                           &frame.syn_stream,
+                                           &session->hd_deflater);
+  scripted_data_feed_init(&df, framedata, framelen);
+
+  CU_ASSERT(0 == spdylay_session_recv(session));
+  CU_ASSERT(0 == spdylay_session_send(session));
+  CU_ASSERT(0 == user_data.ctrl_send_cb_called);
+
+  /* Receive exactly same bytes of SYN_STREAM causes error */
+  scripted_data_feed_init(&df, framedata, framelen);
+
+  CU_ASSERT(0 == spdylay_session_recv(session));
+  CU_ASSERT(0 == spdylay_session_send(session));
+  CU_ASSERT(1 == user_data.ctrl_send_cb_called);
+  CU_ASSERT(SPDYLAY_GOAWAY == user_data.sent_frame_type);
+
+  free(framedata);
+  free(nvbuf);
+  spdylay_frame_syn_stream_free(&frame.syn_stream);
+
+  spdylay_session_del(session);
+}

tests/spdylay_session_test.h

@@ -51,5 +51,6 @@ void test_spdylay_session_max_concurrent_streams();
 void test_spdylay_session_data_backoff_by_high_pri_frame();
 void test_spdylay_session_stop_data_with_rst_stream();
 void test_spdylay_session_stream_close_on_syn_stream();
+void test_spdylay_session_recv_invalid_frame();
 
 #endif // SPDYLAY_SESSION_TEST_H