Use FRAME_SIZE_ERROR for invalid payload length

lib/nghttp2_frame.c

@@ -273,7 +273,7 @@ int nghttp2_frame_unpack_headers_without_nv(nghttp2_headers *frame,
   }
   if(head[3] & NGHTTP2_FLAG_PRIORITY) {
     if(payloadlen < 4) {
-      return NGHTTP2_ERR_INVALID_FRAME;
+      return NGHTTP2_ERR_FRAME_SIZE_ERROR;
     }
     frame->pri = nghttp2_get_uint32(payload) & NGHTTP2_PRIORITY_MASK;
   } else {
@@ -304,7 +304,7 @@ int nghttp2_frame_unpack_priority(nghttp2_priority *frame,
                                   const uint8_t *payload, size_t payloadlen)
 {
   if(payloadlen != 4) {
-    return NGHTTP2_ERR_INVALID_FRAME;
+    return NGHTTP2_ERR_FRAME_SIZE_ERROR;
   }
   nghttp2_frame_unpack_frame_hd(&frame->hd, head);
   frame->pri = nghttp2_get_uint32(payload) & NGHTTP2_PRIORITY_MASK;
@@ -332,7 +332,7 @@ int nghttp2_frame_unpack_rst_stream(nghttp2_rst_stream *frame,
                                     const uint8_t *payload, size_t payloadlen)
 {
   if(payloadlen != 4) {
-    return NGHTTP2_ERR_INVALID_FRAME;
+    return NGHTTP2_ERR_FRAME_SIZE_ERROR;
   }
   nghttp2_frame_unpack_frame_hd(&frame->hd, head);
   frame->error_code = nghttp2_get_uint32(payload);
@@ -371,10 +371,12 @@ int nghttp2_frame_unpack_settings(nghttp2_settings *frame,
                                   const uint8_t *payload, size_t payloadlen)
 {
   int rv;
-  if(payloadlen % 8) {
-    return NGHTTP2_ERR_INVALID_FRAME;
-  }
   nghttp2_frame_unpack_frame_hd(&frame->hd, head);
+  if((payloadlen & 0x7) ||
+     ((frame->hd.flags & NGHTTP2_FLAG_ACK) && payloadlen > 0)) {
+    return NGHTTP2_ERR_FRAME_SIZE_ERROR;
+  }
+
   rv = nghttp2_frame_unpack_settings_payload(&frame->iv, &frame->niv,
                                              payload, payloadlen);
   if(rv != 0) {
@@ -464,7 +466,7 @@ int nghttp2_frame_unpack_push_promise_without_nv(nghttp2_push_promise *frame,
     return NGHTTP2_ERR_PROTO;
   }
   if(payloadlen < 4) {
-    return NGHTTP2_ERR_INVALID_FRAME;
+    return NGHTTP2_ERR_FRAME_SIZE_ERROR;
   }
   frame->promised_stream_id = nghttp2_get_uint32(payload) &
     NGHTTP2_STREAM_ID_MASK;
@@ -493,7 +495,7 @@ int nghttp2_frame_unpack_ping(nghttp2_ping *frame,
                               const uint8_t *payload, size_t payloadlen)
 {
   if(payloadlen != 8) {
-    return NGHTTP2_ERR_INVALID_FRAME;
+    return NGHTTP2_ERR_FRAME_SIZE_ERROR;
   }
   nghttp2_frame_unpack_frame_hd(&frame->hd, head);
   memcpy(frame->opaque_data, payload, sizeof(frame->opaque_data));
@@ -524,7 +526,7 @@ int nghttp2_frame_unpack_goaway(nghttp2_goaway *frame,
 {
   nghttp2_frame_unpack_frame_hd(&frame->hd, head);
   if(payloadlen < 8) {
-    return NGHTTP2_ERR_INVALID_FRAME;
+    return NGHTTP2_ERR_FRAME_SIZE_ERROR;
   }
   frame->last_stream_id = nghttp2_get_uint32(payload) & NGHTTP2_STREAM_ID_MASK;
   frame->error_code = nghttp2_get_uint32(payload+4);
@@ -562,7 +564,7 @@ int nghttp2_frame_unpack_window_update(nghttp2_window_update *frame,
                                        size_t payloadlen)
 {
   if(payloadlen != 4) {
-    return NGHTTP2_ERR_INVALID_FRAME;
+    return NGHTTP2_ERR_FRAME_SIZE_ERROR;
   }
   nghttp2_frame_unpack_frame_hd(&frame->hd, head);
   frame->window_size_increment = nghttp2_get_uint32(payload) &

lib/nghttp2_frame.h

@@ -132,8 +132,8 @@ ssize_t nghttp2_frame_pack_headers(uint8_t **buf_ptr,
  *     The inflate operation failed.
  * NGHTTP2_ERR_INVALID_HEADER_BLOCK
  *     Unpacking succeeds but the header block is invalid.
- * NGHTTP2_ERR_INVALID_FRAME
- *     The input data are invalid.
+ * NGHTTP2_ERR_FRAME_SIZE_ERROR
+ *     The input length is invalid
  * NGHTTP2_ERR_NOMEM
  *     Out of memory.
  */
@@ -149,8 +149,8 @@ int nghttp2_frame_unpack_headers(nghttp2_headers *frame,
  * This function returns 0 if it succeeds or one of the following
  * negative error codes:
  *
- * NGHTTP2_ERR_INVALID_FRAME
- *     The input data are invalid.
+ * NGHTTP2_ERR_FRAME_SIZE_ERROR
+ *     The input length is invalid
  */
 int nghttp2_frame_unpack_headers_without_nv(nghttp2_headers *frame,
                                             const uint8_t *head,
@@ -179,8 +179,8 @@ ssize_t nghttp2_frame_pack_priority(uint8_t **buf_ptr, size_t *buflen_ptr,
  * This function returns 0 if it succeeds or one of the following
  * negative error codes:
  *
- * NGHTTP2_ERR_INVALID_FRAME
- *     The input data are invalid.
+ * NGHTTP2_ERR_FRAME_SIZE_ERROR
+ *     The input length is invalid
  */
 int nghttp2_frame_unpack_priority(nghttp2_priority *frame,
                                   const uint8_t *head, size_t headlen,
@@ -208,8 +208,8 @@ ssize_t nghttp2_frame_pack_rst_stream(uint8_t **buf_ptr, size_t *buflen_ptr,
  * This function returns 0 if it succeeds or one of the following
  * negative error codes:
  *
- * NGHTTP2_ERR_INVALID_FRAME
- *     The input data are invalid.
+ * NGHTTP2_ERR_FRAME_SIZE_ERROR
+ *     The input length is invalid
  */
 int nghttp2_frame_unpack_rst_stream(nghttp2_rst_stream *frame,
                                     const uint8_t *head, size_t headlen,
@@ -246,8 +246,8 @@ size_t nghttp2_frame_pack_settings_payload(uint8_t *buf,
  * This function returns 0 if it succeeds or one of the following
  * negative error codes:
  *
- * NGHTTP2_ERR_INVALID_FRAME
- *     The input data are invalid.
+ * NGHTTP2_ERR_FRAME_SIZE_ERROR
+ *     The input length is invalid
  * NGHTTP2_ERR_NOMEM
  *     Out of memory.
  */
@@ -317,8 +317,8 @@ ssize_t nghttp2_frame_pack_push_promise(uint8_t **buf_ptr,
  *     The inflate operation failed.
  * NGHTTP2_ERR_INVALID_HEADER_BLOCK
  *     Unpacking succeeds but the header block is invalid.
- * NGHTTP2_ERR_INVALID_FRAME
- *     The input data are invalid.
+ * NGHTTP2_ERR_FRAME_SIZE_ERROR
+ *     The input length is invalid
  * NGHTTP2_ERR_NOMEM
  *     Out of memory.
  */
@@ -335,8 +335,8 @@ int nghttp2_frame_unpack_push_promise(nghttp2_push_promise *frame,
  * This function returns 0 if it succeeds or one of the following
  * negative error codes:
  *
- * NGHTTP2_ERR_INVALID_FRAME
- *     The input data are invalid.
+ * NGHTTP2_ERR_FRAME_SIZE_ERROR
+ *     The input length is invalid
  */
 int nghttp2_frame_unpack_push_promise_without_nv(nghttp2_push_promise *frame,
                                                  const uint8_t *head,
@@ -365,8 +365,8 @@ ssize_t nghttp2_frame_pack_ping(uint8_t **buf_ptr, size_t *buflen_ptr,
  * This function returns 0 if it succeeds or one of the following
  * negative error codes:
  *
- * NGHTTP2_ERR_INVALID_FRAME
- *     The input data are invalid.
+ * NGHTTP2_ERR_FRAME_SIZE_ERROR
+ *     The input length is invalid
  */
 int nghttp2_frame_unpack_ping(nghttp2_ping *frame,
                               const uint8_t *head, size_t headlen,
@@ -393,8 +393,8 @@ ssize_t nghttp2_frame_pack_goaway(uint8_t **buf_ptr, size_t *buflen_ptr,
  * This function returns 0 if it succeeds or one of the following
  * negative error codes:
  *
- * NGHTTP2_ERR_INVALID_FRAME
- *     The input data are invalid.
+ * NGHTTP2_ERR_FRAME_SIZE_ERROR
+ *     The input length is invalid
  * NGHTTP2_ERR_NOMEM
  *     Out of memory.
  */
@@ -423,8 +423,8 @@ ssize_t nghttp2_frame_pack_window_update(uint8_t **buf_ptr, size_t *buflen_ptr,
  * This function returns 0 if it succeeds or one of the following
  * negative error codes:
  *
- * NGHTTP2_ERR_INVALID_FRAME
- *     The input data are invalid.
+ * NGHTTP2_ERR_FRAME_SIZE_ERROR
+ *     The input length is invalid
  */
 int nghttp2_frame_unpack_window_update(nghttp2_window_update *frame,
                                        const uint8_t *head, size_t headlen,

lib/nghttp2_session.c

@@ -2684,6 +2684,8 @@ static int get_error_code_from_lib_error_code(int lib_error_code)
   switch(lib_error_code) {
   case NGHTTP2_ERR_HEADER_COMP:
     return NGHTTP2_COMPRESSION_ERROR;
+  case NGHTTP2_ERR_FRAME_SIZE_ERROR:
+    return NGHTTP2_FRAME_SIZE_ERROR;
   default:
     return NGHTTP2_PROTOCOL_ERROR;
   }
@@ -2764,8 +2766,8 @@ static int nghttp2_session_process_ctrl_frame(nghttp2_session *session)
         nghttp2_frame_priority_free(&frame->priority);
       }
     } else if(nghttp2_is_non_fatal(r)) {
-      r = nghttp2_session_handle_parse_error(session, type, r,
-                                             NGHTTP2_PROTOCOL_ERROR);
+      r = nghttp2_session_handle_parse_error
+        (session, type, r, get_error_code_from_lib_error_code(r));
     }
     break;
   case NGHTTP2_RST_STREAM:
@@ -2780,8 +2782,8 @@ static int nghttp2_session_process_ctrl_frame(nghttp2_session *session)
         nghttp2_frame_rst_stream_free(&frame->rst_stream);
       }
     } else if(nghttp2_is_non_fatal(r)) {
-      r = nghttp2_session_handle_parse_error(session, type, r,
-                                             NGHTTP2_PROTOCOL_ERROR);
+      r = nghttp2_session_handle_parse_error
+        (session, type, r, get_error_code_from_lib_error_code(r));
     }
     break;
   case NGHTTP2_SETTINGS:
@@ -2796,8 +2798,8 @@ static int nghttp2_session_process_ctrl_frame(nghttp2_session *session)
         nghttp2_frame_settings_free(&frame->settings);
       }
     } else if(nghttp2_is_non_fatal(r)) {
-      r = nghttp2_session_handle_parse_error(session, type, r,
-                                             NGHTTP2_PROTOCOL_ERROR);
+      r = nghttp2_session_handle_parse_error
+        (session, type, r, get_error_code_from_lib_error_code(r));
     }
     break;
   case NGHTTP2_PUSH_PROMISE:
@@ -2834,8 +2836,8 @@ static int nghttp2_session_process_ctrl_frame(nghttp2_session *session)
         nghttp2_frame_ping_free(&frame->ping);
       }
     } else if(nghttp2_is_non_fatal(r)) {
-      r = nghttp2_session_handle_parse_error(session, type, r,
-                                             NGHTTP2_PROTOCOL_ERROR);
+      r = nghttp2_session_handle_parse_error
+        (session, type, r, get_error_code_from_lib_error_code(r));
     }
     break;
   case NGHTTP2_GOAWAY:
@@ -2850,8 +2852,8 @@ static int nghttp2_session_process_ctrl_frame(nghttp2_session *session)
         nghttp2_frame_goaway_free(&frame->goaway);
       }
     } else if(nghttp2_is_non_fatal(r)) {
-      r = nghttp2_session_handle_parse_error(session, type, r,
-                                             NGHTTP2_PROTOCOL_ERROR);
+      r = nghttp2_session_handle_parse_error
+        (session, type, r, get_error_code_from_lib_error_code(r));
     }
     break;
   case NGHTTP2_WINDOW_UPDATE:
@@ -2866,8 +2868,8 @@ static int nghttp2_session_process_ctrl_frame(nghttp2_session *session)
         nghttp2_frame_window_update_free(&frame->window_update);
       }
     } else if(nghttp2_is_non_fatal(r)) {
-      r = nghttp2_session_handle_parse_error(session, type, r,
-                                             NGHTTP2_PROTOCOL_ERROR);
+      r = nghttp2_session_handle_parse_error
+        (session, type, r, get_error_code_from_lib_error_code(r));
     }
     break;
   default:

tests/nghttp2_session_test.c

@@ -463,7 +463,7 @@ void test_nghttp2_session_recv(void)
   CU_ASSERT(0 == user_data.frame_recv_cb_called);
   item = nghttp2_session_get_next_ob_item(session);
   CU_ASSERT(NGHTTP2_GOAWAY == OB_CTRL_TYPE(item));
-  CU_ASSERT(NGHTTP2_PROTOCOL_ERROR == OB_CTRL(item)->goaway.error_code);
+  CU_ASSERT(NGHTTP2_FRAME_SIZE_ERROR == OB_CTRL(item)->goaway.error_code);
   CU_ASSERT(0 == nghttp2_session_send(session));
 
   free(framedata);