nghttpx: Return 503 on hard disconnect in HTTP/2 backend

integration-tests/nghttpx_test.go

@@ -95,6 +95,25 @@ func TestH1H1ConnectFailure(t *testing.T) {
 	}
 }
 
+func TestH1H2ConnectFailure(t *testing.T) {
+	st := newServerTester([]string{"--http2-bridge"}, t, noopHandler)
+	defer st.Close()
+
+	// simulate backend connect attempt failure
+	st.ts.Close()
+
+	res, err := st.http1(requestParam{
+		name: "TestH1H2ConnectFailure",
+	})
+	if err != nil {
+		t.Fatalf("Error st.http1() = %v", err)
+	}
+	want := 503
+	if got := res.status; got != want {
+		t.Errorf("status: %v; want %v", got, want)
+	}
+}
+
 func TestH1H2NoHost(t *testing.T) {
 	st := newServerTester([]string{"--http2-bridge"}, t, func(w http.ResponseWriter, r *http.Request) {
 		t.Errorf("server should not forward bad request")
@@ -409,6 +428,25 @@ func TestH2H2InvalidResponseCL(t *testing.T) {
 	}
 }
 
+func TestH2H2ConnectFailure(t *testing.T) {
+	st := newServerTester([]string{"--http2-bridge"}, t, noopHandler)
+	defer st.Close()
+
+	// simulate backend connect attempt failure
+	st.ts.Close()
+
+	res, err := st.http2(requestParam{
+		name: "TestH2H2ConnectFailure",
+	})
+	if err != nil {
+		t.Fatalf("Error st.http2() = %v", err)
+	}
+	want := 503
+	if got := res.status; got != want {
+		t.Errorf("status: %v; want %v", got, want)
+	}
+}
+
 func TestS3H1PlainGET(t *testing.T) {
 	st := newServerTesterTLS([]string{"--npn-list=spdy/3.1"}, t, noopHandler)
 	defer st.Close()
@@ -492,3 +530,22 @@ func TestS3H1InvalidRequestCL(t *testing.T) {
 		t.Errorf("status: %v; want %v", got, want)
 	}
 }
+
+func TestS3H2ConnectFailure(t *testing.T) {
+	st := newServerTesterTLS([]string{"--npn-list=spdy/3.1", "--http2-bridge"}, t, noopHandler)
+	defer st.Close()
+
+	// simulate backend connect attempt failure
+	st.ts.Close()
+
+	res, err := st.spdy(requestParam{
+		name: "TestS3H2ConnectFailure",
+	})
+	if err != nil {
+		t.Fatalf("Error st.spdy() = %v", err)
+	}
+	want := 503
+	if got := res.status; got != want {
+		t.Errorf("status: %v; want %v", got, want)
+	}
+}

src/shrpx_http2_session.cc

@@ -239,11 +239,7 @@ int Http2Session::disconnect(bool hard) {
     handlers.insert(dc->get_client_handler());
   }
   for (auto h : handlers) {
-    if (hard) {
+    if (h->get_upstream()->on_downstream_reset(hard) != 0) {
-      delete h;
-      continue;
-    }
-    if (h->get_upstream()->on_downstream_reset() != 0) {
       delete h;
     }
   }

src/shrpx_http2_upstream.cc

@@ -1345,7 +1345,7 @@ void Http2Upstream::on_handler_delete() {
   }
 }
 
-int Http2Upstream::on_downstream_reset() {
+int Http2Upstream::on_downstream_reset(bool no_retry) {
   int rv;
 
   for (auto &ent : downstream_queue_.get_active_downstreams()) {
@@ -1358,9 +1358,17 @@ int Http2Upstream::on_downstream_reset() {
       continue;
     }
 
+    downstream->pop_downstream_connection();
+
+    if (no_retry) {
+      if (on_downstream_abort_request(downstream, 503) != 0) {
+        return -1;
+      }
+      continue;
+    }
+
     // downstream connection is clean; we can retry with new
     // downstream connection.
-    downstream->pop_downstream_connection();
 
     rv = downstream->attach_downstream_connection(
         handler_->get_downstream_connection());

src/shrpx_http2_upstream.h

@@ -80,7 +80,7 @@ public:
   virtual int on_downstream_body_complete(Downstream *downstream);
 
   virtual void on_handler_delete();
-  virtual int on_downstream_reset();
+  virtual int on_downstream_reset(bool no_retry);
 
   virtual MemchunkPool *get_mcpool();
 

src/shrpx_https_upstream.cc

@@ -815,7 +815,7 @@ void HttpsUpstream::on_handler_delete() {
   }
 }
 
-int HttpsUpstream::on_downstream_reset() {
+int HttpsUpstream::on_downstream_reset(bool no_retry) {
   int rv;
 
   if ((downstream_->get_request_state() != Downstream::HEADER_COMPLETE &&
@@ -825,6 +825,13 @@ int HttpsUpstream::on_downstream_reset() {
     return -1;
   }
 
+  if (no_retry) {
+    if (on_downstream_abort_request(downstream_.get(), 503) != 0) {
+      return -1;
+    }
+    return 0;
+  }
+
   downstream_->pop_downstream_connection();
 
   rv = downstream_->attach_downstream_connection(

src/shrpx_https_upstream.h

@@ -74,7 +74,7 @@ public:
   virtual int on_downstream_body_complete(Downstream *downstream);
 
   virtual void on_handler_delete();
-  virtual int on_downstream_reset();
+  virtual int on_downstream_reset(bool no_retry);
 
   virtual MemchunkPool *get_mcpool();
 

src/shrpx_spdy_upstream.cc

@@ -1028,7 +1028,7 @@ void SpdyUpstream::on_handler_delete() {
   }
 }
 
-int SpdyUpstream::on_downstream_reset() {
+int SpdyUpstream::on_downstream_reset(bool no_retry) {
   int rv;
 
   for (auto &ent : downstream_queue_.get_active_downstreams()) {
@@ -1041,9 +1041,17 @@ int SpdyUpstream::on_downstream_reset() {
       continue;
     }
 
+    downstream->pop_downstream_connection();
+
+    if (no_retry) {
+      if (on_downstream_abort_request(downstream, 503) != 0) {
+        return -1;
+      }
+      return 0;
+    }
+
     // downstream connection is clean; we can retry with new
     // downstream connection.
-    downstream->pop_downstream_connection();
 
     rv = downstream->attach_downstream_connection(
         handler_->get_downstream_connection());

src/shrpx_spdy_upstream.h

@@ -75,7 +75,7 @@ public:
   virtual int on_downstream_body_complete(Downstream *downstream);
 
   virtual void on_handler_delete();
-  virtual int on_downstream_reset();
+  virtual int on_downstream_reset(bool no_retry);
 
   virtual MemchunkPool *get_mcpool();
 

src/shrpx_upstream.h

@@ -58,8 +58,9 @@ public:
 
   virtual void on_handler_delete() = 0;
   // Called when downstream connection is reset.  Currently this is
-  // only used by Http2Session.
+  // only used by Http2Session.  If |no_retry| is true, another
-  virtual int on_downstream_reset() = 0;
+  // connection attempt using new DownstreamConnection is not allowed.
+  virtual int on_downstream_reset(bool no_retry) = 0;
 
   virtual void pause_read(IOCtrlReason reason) = 0;
   virtual int resume_read(IOCtrlReason reason, Downstream *downstream,