Add nghttp2_session_upgrade2(), deprecate nghttp2_session_upgrade()

To validate actual response body length against the value declared in
content-length response header field, we first check request method.
If request method is HEAD, respose body must be 0 regardless of the
value in content-length.  nghttp2_session_upgrade() has no parameter
to indicate the request method is HEAD, so we failed to validate
response body if HEAD is used with HTTP Upgrade.  New
nghttp2_session_upgrade2() accepts new parameter to indicate that
request method is HEAD or not to fix this issue.  Although, this issue
affects client side only, we deprecate nghttp2_session_upgrade() in
favor of nghttp2_session_upgrade2() for both client and server side.

doc/Makefile.am

@@ -116,6 +116,7 @@ APIDOCS= \
 	nghttp2_session_terminate_session.rst \
 	nghttp2_session_terminate_session2.rst \
 	nghttp2_session_upgrade.rst \
+	nghttp2_session_upgrade2.rst \
 	nghttp2_session_want_read.rst \
 	nghttp2_session_want_write.rst \
 	nghttp2_stream_get_first_child.rst \

lib/includes/nghttp2/nghttp2.h

@@ -2858,6 +2858,17 @@ NGHTTP2_EXTERN int nghttp2_session_consume_stream(nghttp2_session *session,
  * be called from both client and server, but the behavior is very
  * different in each other.
  *
+ * .. warning::
+ *
+ *   This function is deprecated in favor of
+ *   `nghttp2_session_upgrade2()`, because this function lacks the
+ *   parameter to tell the library the request method used in the
+ *   original HTTP request.  This information is required for client
+ *   to validate actual response body length against content-length
+ *   header field (see `nghttp2_option_set_no_http_messaging()`).  If
+ *   HEAD is used in request, the length of response body must be 0
+ *   regardless of value included in content-length header field.
+ *
  * If called from client side, the |settings_payload| must be the
  * value sent in ``HTTP2-Settings`` header field and must be decoded
  * by base64url decoder.  The |settings_payloadlen| is the length of
@@ -2892,6 +2903,51 @@ NGHTTP2_EXTERN int nghttp2_session_upgrade(nghttp2_session *session,
                                            size_t settings_payloadlen,
                                            void *stream_user_data);
 
+/**
+ * @function
+ *
+ * Performs post-process of HTTP Upgrade request.  This function can
+ * be called from both client and server, but the behavior is very
+ * different in each other.
+ *
+ * If called from client side, the |settings_payload| must be the
+ * value sent in ``HTTP2-Settings`` header field and must be decoded
+ * by base64url decoder.  The |settings_payloadlen| is the length of
+ * |settings_payload|.  The |settings_payload| is unpacked and its
+ * setting values will be submitted using `nghttp2_submit_settings()`.
+ * This means that the client application code does not need to submit
+ * SETTINGS by itself.  The stream with stream ID=1 is opened and the
+ * |stream_user_data| is used for its stream_user_data.  The opened
+ * stream becomes half-closed (local) state.
+ *
+ * If called from server side, the |settings_payload| must be the
+ * value received in ``HTTP2-Settings`` header field and must be
+ * decoded by base64url decoder.  The |settings_payloadlen| is the
+ * length of |settings_payload|.  It is treated as if the SETTINGS
+ * frame with that payload is received.  Thus, callback functions for
+ * the reception of SETTINGS frame will be invoked.  The stream with
+ * stream ID=1 is opened.  The |stream_user_data| is ignored.  The
+ * opened stream becomes half-closed (remote).
+ *
+ * If the request method is HEAD, pass nonzero value to
+ * |head_request|.  Otherwise, pass 0.
+ *
+ * This function returns 0 if it succeeds, or one of the following
+ * negative error codes:
+ *
+ * :enum:`NGHTTP2_ERR_NOMEM`
+ *     Out of memory.
+ * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT`
+ *     The |settings_payload| is badly formed.
+ * :enum:`NGHTTP2_ERR_PROTO`
+ *     The stream ID 1 is already used or closed; or is not available.
+ */
+NGHTTP2_EXTERN int nghttp2_session_upgrade2(nghttp2_session *session,
+                                            const uint8_t *settings_payload,
+                                            size_t settings_payloadlen,
+                                            int head_request,
+                                            void *stream_user_data);
+
 /**
  * @function
  *

lib/nghttp2_session.c

@@ -6502,10 +6502,10 @@ uint32_t nghttp2_session_get_remote_settings(nghttp2_session *session,
   assert(0);
 }
 
-int nghttp2_session_upgrade(nghttp2_session *session,
+static int nghttp2_session_upgrade_internal(nghttp2_session *session,
-                            const uint8_t *settings_payload,
+                                            const uint8_t *settings_payload,
-                            size_t settings_payloadlen,
+                                            size_t settings_payloadlen,
-                            void *stream_user_data) {
+                                            void *stream_user_data) {
   nghttp2_stream *stream;
   nghttp2_frame frame;
   nghttp2_settings_entry *iv;
@@ -6559,19 +6559,61 @@ int nghttp2_session_upgrade(nghttp2_session *session,
     nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_WR);
     session->next_stream_id += 2;
   }
+  return 0;
+}
+
+int nghttp2_session_upgrade(nghttp2_session *session,
+                            const uint8_t *settings_payload,
+                            size_t settings_payloadlen,
+                            void *stream_user_data) {
+  int rv;
+  nghttp2_stream *stream;
+
+  rv = nghttp2_session_upgrade_internal(session, settings_payload,
+                                        settings_payloadlen, stream_user_data);
+  if (rv != 0) {
+    return rv;
+  }
+
+  stream = nghttp2_session_get_stream(session, 1);
+  assert(stream);
+
   /* We have no information about request header fields when Upgrade
      was happened.  So we don't know the request method here.  If
      request method is HEAD, we have a trouble because we may have
      nonzero content-length header field in response headers, and we
      will going to check it against the actual DATA frames, but we may
-     get mismatch because HEAD response body must be empty.  We will
+     get mismatch because HEAD response body must be empty.  Because
-     add new version of nghttp2_session_upgrade with the parameter to
+     of this reason, nghttp2_session_upgrade() was deprecated in favor
-     pass the request method information so that we can handle this
+     of nghttp2_session_upgrade2(), which has |head_request| parameter
-     situation properly. */
+     to indicate that request method is HEAD or not. */
   stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_UPGRADE_WORKAROUND;
   return 0;
 }
 
+int nghttp2_session_upgrade2(nghttp2_session *session,
+                             const uint8_t *settings_payload,
+                             size_t settings_payloadlen, int head_request,
+                             void *stream_user_data) {
+  int rv;
+  nghttp2_stream *stream;
+
+  rv = nghttp2_session_upgrade_internal(session, settings_payload,
+                                        settings_payloadlen, stream_user_data);
+  if (rv != 0) {
+    return rv;
+  }
+
+  stream = nghttp2_session_get_stream(session, 1);
+  assert(stream);
+
+  if (head_request) {
+    stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_HEAD;
+  }
+
+  return 0;
+}
+
 int nghttp2_session_get_stream_local_close(nghttp2_session *session,
                                            int32_t stream_id) {
   nghttp2_stream *stream;

tests/main.c

@@ -138,7 +138,7 @@ int main(int argc _U_, char *argv[] _U_) {
                    test_nghttp2_session_send_push_promise) ||
       !CU_add_test(pSuite, "session_is_my_stream_id",
                    test_nghttp2_session_is_my_stream_id) ||
-      !CU_add_test(pSuite, "session_upgrade", test_nghttp2_session_upgrade) ||
+      !CU_add_test(pSuite, "session_upgrade2", test_nghttp2_session_upgrade2) ||
       !CU_add_test(pSuite, "session_reprioritize_stream",
                    test_nghttp2_session_reprioritize_stream) ||
       !CU_add_test(

tests/nghttp2_session_test.c

@@ -3331,7 +3331,7 @@ void test_nghttp2_session_is_my_stream_id(void) {
   nghttp2_session_del(session);
 }
 
-void test_nghttp2_session_upgrade(void) {
+void test_nghttp2_session_upgrade2(void) {
   nghttp2_session *session;
   nghttp2_session_callbacks callbacks;
   uint8_t settings_payload[128];
@@ -3351,8 +3351,8 @@ void test_nghttp2_session_upgrade(void) {
 
   /* Check client side */
   nghttp2_session_client_new(&session, &callbacks, NULL);
-  CU_ASSERT(0 == nghttp2_session_upgrade(session, settings_payload,
+  CU_ASSERT(0 == nghttp2_session_upgrade2(session, settings_payload,
-                                         settings_payloadlen, &callbacks));
+                                          settings_payloadlen, 0, &callbacks));
   stream = nghttp2_session_get_stream(session, 1);
   CU_ASSERT(stream != NULL);
   CU_ASSERT(&callbacks == stream->stream_user_data);
@@ -3367,16 +3367,16 @@ void test_nghttp2_session_upgrade(void) {
             item->frame.settings.iv[1].settings_id);
   CU_ASSERT(4095 == item->frame.settings.iv[1].value);
 
-  /* Call nghttp2_session_upgrade() again is error */
+  /* Call nghttp2_session_upgrade2() again is error */
   CU_ASSERT(NGHTTP2_ERR_PROTO ==
-            nghttp2_session_upgrade(session, settings_payload,
+            nghttp2_session_upgrade2(session, settings_payload,
-                                    settings_payloadlen, &callbacks));
+                                     settings_payloadlen, 0, &callbacks));
   nghttp2_session_del(session);
 
   /* Check server side */
   nghttp2_session_server_new(&session, &callbacks, NULL);
-  CU_ASSERT(0 == nghttp2_session_upgrade(session, settings_payload,
+  CU_ASSERT(0 == nghttp2_session_upgrade2(session, settings_payload,
-                                         settings_payloadlen, &callbacks));
+                                          settings_payloadlen, 0, &callbacks));
   stream = nghttp2_session_get_stream(session, 1);
   CU_ASSERT(stream != NULL);
   CU_ASSERT(NULL == stream->stream_user_data);
@@ -3384,10 +3384,10 @@ void test_nghttp2_session_upgrade(void) {
   CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(session));
   CU_ASSERT(1 == session->remote_settings.max_concurrent_streams);
   CU_ASSERT(4095 == session->remote_settings.initial_window_size);
-  /* Call nghttp2_session_upgrade() again is error */
+  /* Call nghttp2_session_upgrade2() again is error */
   CU_ASSERT(NGHTTP2_ERR_PROTO ==
-            nghttp2_session_upgrade(session, settings_payload,
+            nghttp2_session_upgrade2(session, settings_payload,
-                                    settings_payloadlen, &callbacks));
+                                     settings_payloadlen, 0, &callbacks));
   nghttp2_session_del(session);
 
   /* Empty SETTINGS is OK */
@@ -3395,8 +3395,8 @@ void test_nghttp2_session_upgrade(void) {
       settings_payload, sizeof(settings_payload), NULL, 0);
 
   nghttp2_session_client_new(&session, &callbacks, NULL);
-  CU_ASSERT(0 == nghttp2_session_upgrade(session, settings_payload,
+  CU_ASSERT(0 == nghttp2_session_upgrade2(session, settings_payload,
-                                         settings_payloadlen, NULL));
+                                          settings_payloadlen, 0, NULL));
   nghttp2_session_del(session);
 }
 

tests/nghttp2_session_test.h

@@ -59,7 +59,7 @@ void test_nghttp2_session_send_headers_push_reply(void);
 void test_nghttp2_session_send_rst_stream(void);
 void test_nghttp2_session_send_push_promise(void);
 void test_nghttp2_session_is_my_stream_id(void);
-void test_nghttp2_session_upgrade(void);
+void test_nghttp2_session_upgrade2(void);
 void test_nghttp2_session_reprioritize_stream(void);
 void test_nghttp2_session_reprioritize_stream_with_idle_stream_dep(void);
 void test_nghttp2_submit_data(void);