Merge branch 'upgrade2'

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,
-                            const uint8_t *settings_payload,
-                            size_t settings_payloadlen,
-                            void *stream_user_data) {
+static int nghttp2_session_upgrade_internal(nghttp2_session *session,
+                                            const uint8_t *settings_payload,
+                                            size_t settings_payloadlen,
+                                            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
-     add new version of nghttp2_session_upgrade with the parameter to
-     pass the request method information so that we can handle this
-     situation properly. */
+     get mismatch because HEAD response body must be empty.  Because
+     of this reason, nghttp2_session_upgrade() was deprecated in favor
+     of nghttp2_session_upgrade2(), which has |head_request| parameter
+     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;

src/nghttp.cc

@@ -392,6 +392,11 @@ int submit_request(HttpClient *client, const Headers &headers, Request *req) {
     nva.push_back(http2::make_nv(kv.name, kv.value, kv.no_index));
   }
 
+  auto method = http2::get_header(build_headers, ":method");
+  assert(method);
+
+  req->method = method->value;
+
   std::string trailer_names;
   if (!config.trailer.empty()) {
     trailer_names = config.trailer[0].name;
@@ -808,6 +813,7 @@ int HttpClient::on_upgrade_connect() {
       base64::encode(std::begin(settings_payload),
                      std::begin(settings_payload) + settings_payloadlen);
   util::to_token68(token68);
+
   std::string req;
   if (reqvec[0]->data_prd) {
     // If the request contains upload data, use OPTIONS * to upgrade
@@ -815,29 +821,34 @@ int HttpClient::on_upgrade_connect() {
   } else {
     auto meth = std::find_if(
         std::begin(config.headers), std::end(config.headers),
-        [](const Header &kv) { return util::strieq_l(":method", kv.name); });
+        [](const Header &kv) { return util::streq_l(":method", kv.name); });
 
     if (meth == std::end(config.headers)) {
       req = "GET ";
+      reqvec[0]->method = "GET";
     } else {
       req = (*meth).value;
       req += " ";
+      reqvec[0]->method = (*meth).value;
     }
     req += reqvec[0]->make_reqpath();
   }
 
-  auto headers = Headers{{"Host", hostport},
-                         {"Connection", "Upgrade, HTTP2-Settings"},
-                         {"Upgrade", NGHTTP2_CLEARTEXT_PROTO_VERSION_ID},
-                         {"HTTP2-Settings", token68},
-                         {"Accept", "*/*"},
-                         {"User-Agent", "nghttp2/" NGHTTP2_VERSION}};
+  auto headers = Headers{{"host", hostport},
+                         {"connection", "Upgrade, HTTP2-Settings"},
+                         {"upgrade", NGHTTP2_CLEARTEXT_PROTO_VERSION_ID},
+                         {"http2-settings", token68},
+                         {"accept", "*/*"},
+                         {"user-agent", "nghttp2/" NGHTTP2_VERSION}};
   auto initial_headerslen = headers.size();
 
   for (auto &kv : config.headers) {
     size_t i;
+    if (kv.name.empty() || kv.name[0] == ':') {
+      continue;
+    }
     for (i = 0; i < initial_headerslen; ++i) {
-      if (util::strieq(kv.name, headers[i].name)) {
+      if (kv.name == headers[i].name) {
         headers[i].value = kv.value;
         break;
       }
@@ -845,9 +856,7 @@ int HttpClient::on_upgrade_connect() {
     if (i < initial_headerslen) {
       continue;
     }
-    if (kv.name.size() != 0 && kv.name[0] != ':') {
-      headers.emplace_back(kv.name, kv.value, kv.no_index);
-    }
+    headers.emplace_back(kv.name, kv.value, kv.no_index);
   }
 
   req += " HTTP/1.1\r\n";
@@ -867,9 +876,10 @@ int HttpClient::on_upgrade_connect() {
     std::cout << " HTTP Upgrade request\n" << req << std::endl;
   }
 
-  // record request time if this is GET request
   if (!reqvec[0]->data_prd) {
+    // record request time if this is a part of real request.
     reqvec[0]->record_request_start_time();
+    reqvec[0]->req_nva = std::move(headers);
   }
 
   on_writefn = &HttpClient::noop;
@@ -988,8 +998,12 @@ int HttpClient::connection_made() {
     if (!reqvec[0]->data_prd) {
       stream_user_data = reqvec[0].get();
     }
-    rv = nghttp2_session_upgrade(session, settings_payload.data(),
-                                 settings_payloadlen, stream_user_data);
+    // If HEAD is used, that is only when user specified it with -H
+    // option.
+    auto head_request = stream_user_data && stream_user_data->method == "HEAD";
+    rv = nghttp2_session_upgrade2(session, settings_payload.data(),
+                                  settings_payloadlen, head_request,
+                                  stream_user_data);
     if (rv != 0) {
       std::cerr << "[ERROR] nghttp2_session_upgrade() returned error: "
                 << nghttp2_strerror(rv) << std::endl;
@@ -1387,13 +1401,6 @@ void HttpClient::output_har(FILE *outfile) {
     auto request = json_object();
     json_object_set_new(entry, "request", request);
 
-    auto method_ptr = http2::get_header(req->req_nva, ":method");
-
-    const char *method = "GET";
-    if (method_ptr) {
-      method = (*method_ptr).value.c_str();
-    }
-
     auto req_headers = json_array();
     json_object_set_new(request, "headers", req_headers);
 
@@ -1405,7 +1412,7 @@ void HttpClient::output_har(FILE *outfile) {
       json_object_set_new(hd, "value", json_string(nv.value.c_str()));
     }
 
-    json_object_set_new(request, "method", json_string(method));
+    json_object_set_new(request, "method", json_string(req->method.c_str()));
     json_object_set_new(request, "url", json_string(req->uri.c_str()));
     json_object_set_new(request, "httpVersion", json_string("HTTP/2.0"));
     json_object_set_new(request, "cookies", json_array());

src/nghttp.h

@@ -137,6 +137,7 @@ struct Request {
 
   Headers res_nva;
   Headers req_nva;
+  std::string method;
   // URI without fragment
   std::string uri;
   http_parser_url u;

src/shrpx_http2_upstream.cc

@@ -108,9 +108,10 @@ int Http2Upstream::upgrade_upstream(HttpsUpstream *http) {
   auto settings_payload =
       base64::decode(std::begin(http2_settings), std::end(http2_settings));
 
-  rv = nghttp2_session_upgrade(
+  rv = nghttp2_session_upgrade2(
       session_, reinterpret_cast<const uint8_t *>(settings_payload.c_str()),
-      settings_payload.size(), nullptr);
+      settings_payload.size(),
+      http->get_downstream()->get_request_method() == HTTP_HEAD, nullptr);
   if (rv != 0) {
     if (LOG_ENABLED(INFO)) {
       ULOG(INFO, this) << "nghttp2_session_upgrade() returned error: "

src/util.h

@@ -404,6 +404,10 @@ bool streq_l(const char (&a)[N], InputIt b, size_t blen) {
   return streq(a, N - 1, b, blen);
 }
 
+template <size_t N> bool streq_l(const char (&a)[N], const std::string &b) {
+  return streq(a, N - 1, std::begin(b), b.size());
+}
+
 bool strifind(const char *a, const char *b);
 
 template <typename InputIt> void inp_strlower(InputIt first, InputIt last) {

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,
-                                         settings_payloadlen, &callbacks));
+  CU_ASSERT(0 == nghttp2_session_upgrade2(session, settings_payload,
+                                          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,
-                                    settings_payloadlen, &callbacks));
+            nghttp2_session_upgrade2(session, settings_payload,
+                                     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,
-                                         settings_payloadlen, &callbacks));
+  CU_ASSERT(0 == nghttp2_session_upgrade2(session, settings_payload,
+                                          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,
-                                    settings_payloadlen, &callbacks));
+            nghttp2_session_upgrade2(session, settings_payload,
+                                     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,
-                                         settings_payloadlen, NULL));
+  CU_ASSERT(0 == nghttp2_session_upgrade2(session, settings_payload,
+                                          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);