nghttp: time out on long Expect/Continue waits

To deal with servers that don't conform to RFC 7231 (or, potentially,
connections with a large round-trip time), don't wait forever for a 100
Continue status to come back. Currently, the timeout is hard-coded to
one second.

A ContinueTimer encapsulates the handshake timeout logic for a single
request. Somewhat counterintuitively, ContinueTimers are owned by the
HttpClient instead of the Request object, because their lifetime must be
bound to the life of the connection (which is owned by the HttpClient
and not the Requests). A Request is associated with its corresponding
ContinueTimer through a std::weak_ptr.

src/nghttp.cc

@@ -160,8 +160,7 @@ Request::Request(const std::string &uri, const http_parser_url &u,
       stream_id(-1),
       status(0),
       level(level),
-      expect_final_response(false),
+      expect_final_response(false) {
-      expect_continue(false) {
   http2::init_hdidx(res_hdidx);
   http2::init_hdidx(req_hdidx);
 }
@@ -305,6 +304,49 @@ void Request::record_response_end_time() {
   timing.response_end_time = get_time();
 }
 
+namespace {
+void continue_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) {
+  auto client = static_cast<HttpClient *>(ev_userdata(loop));
+  auto req = static_cast<Request *>(w->data);
+  int error;
+
+  error = nghttp2_submit_data(client->session, NGHTTP2_FLAG_END_STREAM,
+                              req->stream_id, req->data_prd);
+
+  if (error) {
+    std::cerr << "[ERROR] nghttp2_submit_data() returned error: "
+              << nghttp2_strerror(error) << std::endl;
+    nghttp2_submit_rst_stream(client->session, NGHTTP2_FLAG_NONE,
+                              req->stream_id, NGHTTP2_INTERNAL_ERROR);
+  }
+
+  client->signal_write();
+}
+} // namespace
+
+ContinueTimer::ContinueTimer(struct ev_loop *loop, Request *req)
+    : loop(loop),
+      req(req) {
+  ev_timer_init(&timer, continue_timeout_cb, 1., 0.);
+}
+
+ContinueTimer::~ContinueTimer() {
+  stop();
+}
+
+void ContinueTimer::start() {
+  timer.data = req;
+  ev_timer_start(loop, &timer);
+}
+
+void ContinueTimer::stop() {
+  ev_timer_stop(loop, &timer);
+}
+
+void ContinueTimer::dispatch_continue() {
+  ev_feed_event(loop, &timer, 0);
+}
+
 namespace {
 int htp_msg_begincb(http_parser *htp) {
   if (config.verbose) {
@@ -355,6 +397,8 @@ int submit_request(HttpClient *client, const Headers &headers, Request *req) {
                                {"accept", "*/*"},
                                {"accept-encoding", "gzip, deflate"},
                                {"user-agent", "nghttp2/" NGHTTP2_VERSION}};
+  bool expect_continue = false;
+
   if (config.continuation) {
     for (size_t i = 0; i < 6; ++i) {
       build_headers.emplace_back("continuation-test-" + util::utos(i + 1),
@@ -369,7 +413,7 @@ int submit_request(HttpClient *client, const Headers &headers, Request *req) {
       build_headers.emplace_back("content-length", util::utos(req->data_length));
     }
     if (config.expect_continue) {
-      req->expect_continue = true;
+      expect_continue = true;
       build_headers.emplace_back("expect", "100-continue");
     }
   }
@@ -413,7 +457,7 @@ int submit_request(HttpClient *client, const Headers &headers, Request *req) {
 
   int32_t stream_id;
 
-  if (req->expect_continue) {
+  if (expect_continue) {
     stream_id = nghttp2_submit_headers(client->session, 0, -1, &req->pri_spec,
                                nva.data(), nva.size(), req);
   } else {
@@ -424,7 +468,7 @@ int submit_request(HttpClient *client, const Headers &headers, Request *req) {
 
   if (stream_id < 0) {
     std::cerr << "[ERROR] nghttp2_submit_"
-              << (req->expect_continue ? "headers" : "request")
+              << (expect_continue ? "headers" : "request")
               << "() returned error: "
               << nghttp2_strerror(stream_id) << std::endl;
     return -1;
@@ -435,6 +479,14 @@ int submit_request(HttpClient *client, const Headers &headers, Request *req) {
 
   req->req_nva = std::move(build_headers);
 
+  if (expect_continue) {
+    auto timer = std::make_shared<ContinueTimer>(client->loop, req);
+    timer->start();
+
+    req->continue_timer = timer;
+    client->continue_timers.push_back(timer);
+  }
+
   return 0;
 }
 } // namespace
@@ -635,6 +687,8 @@ int HttpClient::initiate_connection() {
 void HttpClient::disconnect() {
   state = ClientState::IDLE;
 
+  continue_timers.clear();
+
   ev_timer_stop(loop, &settings_timer);
 
   ev_timer_stop(loop, &rt);
@@ -1639,15 +1693,11 @@ void check_response_header(nghttp2_session *session, Request *req) {
   }
 
   if (req->status / 100 == 1) {
-    if (req->expect_continue && (req->status == 100)) {
+    if (req->status == 100) {
-      int error = nghttp2_submit_data(session, NGHTTP2_FLAG_END_STREAM,
+      // If the request is waiting for a 100 Continue, complete the handshake.
-                                      req->stream_id, req->data_prd);
+      std::shared_ptr<ContinueTimer> timer = req->continue_timer.lock();
-      if (error) {
+      if (timer) {
-        std::cerr << "[ERROR] nghttp2_submit_data() returned error: "
+        timer->dispatch_continue();
-                  << nghttp2_strerror(error) << std::endl;
-        nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, req->stream_id,
-                                  NGHTTP2_INTERNAL_ERROR);
-        return;
       }
     }
 
@@ -2173,7 +2223,10 @@ int communicate(
                 << std::endl;
       goto fin;
     }
+
+    ev_set_userdata(loop, &client);
     ev_run(loop, 0);
+    ev_set_userdata(loop, nullptr);
 
 #ifdef HAVE_JANSSON
     if (!config.harfile.empty()) {

src/nghttp.h

@@ -110,6 +110,23 @@ struct RequestTiming {
   RequestTiming() : state(RequestState::INITIAL) {}
 };
 
+struct Request; // forward declaration for ContinueTimer
+
+struct ContinueTimer {
+  ContinueTimer(struct ev_loop *loop, Request *req);
+  ~ContinueTimer();
+
+  void start();
+  void stop();
+
+  // Schedules an immediate run of the continue callback on the loop
+  void dispatch_continue();
+
+  struct ev_loop *loop;
+  Request *req;
+  ev_timer timer;
+};
+
 struct Request {
   // For pushed request, |uri| is empty and |u| is zero-cleared.
   Request(const std::string &uri, const http_parser_url &u,
@@ -157,7 +174,8 @@ struct Request {
   // used for incoming PUSH_PROMISE
   http2::HeaderIndex req_hdidx;
   bool expect_final_response;
-  bool expect_continue;
+  // only alive if this request is using Expect/Continue
+  std::weak_ptr<ContinueTimer> continue_timer;
 };
 
 struct SessionTiming {
@@ -266,6 +284,8 @@ struct HttpClient {
   Buffer<64_k> wb;
   // SETTINGS payload sent as token68 in HTTP Upgrade
   std::array<uint8_t, 128> settings_payload;
+  // List of timers for outstanding expect/continue handshakes
+  std::vector<std::shared_ptr<ContinueTimer>> continue_timers;
 
   enum { ERR_CONNECT_FAIL = -100 };
 };