Merge pull request #1833 from nghttp2/integration-h3

Integration h3

integration-tests/Makefile.am

@@ -24,6 +24,7 @@
 GO_FILES = \
 	nghttpx_http1_test.go \
 	nghttpx_http2_test.go \
+	nghttpx_http3_test.go \
 	server_tester.go
 
 EXTRA_DIST = \

integration-tests/nghttpx_http3_test.go

@@ -1,4 +1,5 @@
 //go:build quic
+
 package nghttp2
 
 import (
@@ -6,7 +7,10 @@ import (
 	"crypto/rand"
 	"io"
 	"net/http"
+	"regexp"
 	"testing"
+
+	"golang.org/x/net/http2/hpack"
 )
 
 // TestH3H1PlainGET tests whether simple HTTP/3 GET request works.
@@ -84,3 +88,303 @@ func TestH3H1RequestBody(t *testing.T) {
 		t.Errorf("res.status: %v; want %v", got, want)
 	}
 }
+
+// TestH3H1GenerateVia tests that server generates Via header field to
+// and from backend server.
+func TestH3H1GenerateVia(t *testing.T) {
+	opts := options{
+		handler: func(w http.ResponseWriter, r *http.Request) {
+			if got, want := r.Header.Get("Via"), "3 nghttpx"; got != want {
+				t.Errorf("Via: %v; want %v", got, want)
+			}
+		},
+		quic: true,
+	}
+	st := newServerTester(t, opts)
+	defer st.Close()
+
+	res, err := st.http3(requestParam{
+		name: "TestH3H1GenerateVia",
+	})
+	if err != nil {
+		t.Fatalf("Error st.http3() = %v", err)
+	}
+	if got, want := res.header.Get("Via"), "1.1 nghttpx"; got != want {
+		t.Errorf("Via: %v; want %v", got, want)
+	}
+}
+
+// TestH3H1AppendVia tests that server adds value to existing Via
+// header field to and from backend server.
+func TestH3H1AppendVia(t *testing.T) {
+	opts := options{
+		handler: func(w http.ResponseWriter, r *http.Request) {
+			if got, want := r.Header.Get("Via"), "foo, 3 nghttpx"; got != want {
+				t.Errorf("Via: %v; want %v", got, want)
+			}
+			w.Header().Add("Via", "bar")
+		},
+		quic: true,
+	}
+	st := newServerTester(t, opts)
+	defer st.Close()
+
+	res, err := st.http3(requestParam{
+		name: "TestH3H1AppendVia",
+		header: []hpack.HeaderField{
+			pair("via", "foo"),
+		},
+	})
+	if err != nil {
+		t.Fatalf("Error st.http3() = %v", err)
+	}
+	if got, want := res.header.Get("Via"), "bar, 1.1 nghttpx"; got != want {
+		t.Errorf("Via: %v; want %v", got, want)
+	}
+}
+
+// TestH3H1NoVia tests that server does not add value to existing Via
+// header field to and from backend server.
+func TestH3H1NoVia(t *testing.T) {
+	opts := options{
+		args: []string{"--no-via"},
+		handler: func(w http.ResponseWriter, r *http.Request) {
+			if got, want := r.Header.Get("Via"), "foo"; got != want {
+				t.Errorf("Via: %v; want %v", got, want)
+			}
+			w.Header().Add("Via", "bar")
+		},
+		quic: true,
+	}
+	st := newServerTester(t, opts)
+	defer st.Close()
+
+	res, err := st.http3(requestParam{
+		name: "TestH3H1NoVia",
+		header: []hpack.HeaderField{
+			pair("via", "foo"),
+		},
+	})
+	if err != nil {
+		t.Fatalf("Error st.http3() = %v", err)
+	}
+	if got, want := res.header.Get("Via"), "bar"; got != want {
+		t.Errorf("Via: %v; want %v", got, want)
+	}
+}
+
+// TestH3H1BadResponseCL tests that server returns error when
+// content-length response header field value does not match its
+// response body size.
+func TestH3H1BadResponseCL(t *testing.T) {
+	opts := options{
+		handler: func(w http.ResponseWriter, r *http.Request) {
+			// we set content-length: 1024, but only send 3 bytes.
+			w.Header().Add("Content-Length", "1024")
+			w.Write([]byte("foo"))
+		},
+		quic: true,
+	}
+	st := newServerTester(t, opts)
+	defer st.Close()
+
+	_, err := st.http3(requestParam{
+		name: "TestH3H1BadResponseCL",
+	})
+	if err == nil {
+		t.Fatal("st.http3() should fail")
+	}
+}
+
+// TestH3H1HTTPSRedirect tests that HTTPS redirect should not happen
+// with HTTP/3.
+func TestH3H1HTTPSRedirect(t *testing.T) {
+	opts := options{
+		args: []string{"--redirect-if-not-tls"},
+		quic: true,
+	}
+	st := newServerTester(t, opts)
+	defer st.Close()
+
+	res, err := st.http3(requestParam{
+		name: "TestH3H1HTTPSRedirect",
+	})
+	if err != nil {
+		t.Fatalf("Error st.http3() = %v", err)
+	}
+
+	if got, want := res.status, 200; got != want {
+		t.Errorf("status = %v; want %v", got, want)
+	}
+}
+
+// TestH3H1AffinityCookieTLS tests that affinity cookie is sent back
+// in https.
+func TestH3H1AffinityCookieTLS(t *testing.T) {
+	opts := options{
+		args: []string{"--affinity-cookie"},
+		quic: true,
+	}
+	st := newServerTester(t, opts)
+	defer st.Close()
+
+	res, err := st.http3(requestParam{
+		name:   "TestH3H1AffinityCookieTLS",
+		scheme: "https",
+	})
+	if err != nil {
+		t.Fatalf("Error st.http3() = %v", err)
+	}
+
+	if got, want := res.status, 200; got != want {
+		t.Errorf("status = %v; want %v", got, want)
+	}
+
+	const pattern = `affinity=[0-9a-f]{8}; Path=/foo/bar; Secure`
+	validCookie := regexp.MustCompile(pattern)
+	if got := res.header.Get("Set-Cookie"); !validCookie.MatchString(got) {
+		t.Errorf("Set-Cookie: %v; want pattern %v", got, pattern)
+	}
+}
+
+// TestH3H2ReqPhaseReturn tests mruby request phase hook returns
+// custom response.
+func TestH3H2ReqPhaseReturn(t *testing.T) {
+	opts := options{
+		args: []string{
+			"--http2-bridge",
+			"--mruby-file=" + testDir + "/req-return.rb",
+		},
+		handler: func(w http.ResponseWriter, r *http.Request) {
+			t.Fatalf("request should not be forwarded")
+		},
+		quic: true,
+	}
+	st := newServerTester(t, opts)
+	defer st.Close()
+
+	res, err := st.http3(requestParam{
+		name: "TestH3H2ReqPhaseReturn",
+	})
+	if err != nil {
+		t.Fatalf("Error st.http3() = %v", err)
+	}
+
+	if got, want := res.status, 404; got != want {
+		t.Errorf("status = %v; want %v", got, want)
+	}
+
+	hdtests := []struct {
+		k, v string
+	}{
+		{"content-length", "20"},
+		{"from", "mruby"},
+	}
+	for _, tt := range hdtests {
+		if got, want := res.header.Get(tt.k), tt.v; got != want {
+			t.Errorf("%v = %v; want %v", tt.k, got, want)
+		}
+	}
+
+	if got, want := string(res.body), "Hello World from req"; got != want {
+		t.Errorf("body = %v; want %v", got, want)
+	}
+}
+
+// TestH3H2RespPhaseReturn tests mruby response phase hook returns
+// custom response.
+func TestH3H2RespPhaseReturn(t *testing.T) {
+	opts := options{
+		args: []string{
+			"--http2-bridge",
+			"--mruby-file=" + testDir + "/resp-return.rb",
+		},
+		quic: true,
+	}
+	st := newServerTester(t, opts)
+	defer st.Close()
+
+	res, err := st.http3(requestParam{
+		name: "TestH3H2RespPhaseReturn",
+	})
+	if err != nil {
+		t.Fatalf("Error st.http3() = %v", err)
+	}
+
+	if got, want := res.status, 404; got != want {
+		t.Errorf("status = %v; want %v", got, want)
+	}
+
+	hdtests := []struct {
+		k, v string
+	}{
+		{"content-length", "21"},
+		{"from", "mruby"},
+	}
+	for _, tt := range hdtests {
+		if got, want := res.header.Get(tt.k), tt.v; got != want {
+			t.Errorf("%v = %v; want %v", tt.k, got, want)
+		}
+	}
+
+	if got, want := string(res.body), "Hello World from resp"; got != want {
+		t.Errorf("body = %v; want %v", got, want)
+	}
+}
+
+// TestH3ResponseBeforeRequestEnd tests the situation where response
+// ends before request body finishes.
+func TestH3ResponseBeforeRequestEnd(t *testing.T) {
+	opts := options{
+		args: []string{"--mruby-file=" + testDir + "/req-return.rb"},
+		handler: func(w http.ResponseWriter, r *http.Request) {
+			t.Fatal("request should not be forwarded")
+		},
+		quic: true,
+	}
+	st := newServerTester(t, opts)
+	defer st.Close()
+
+	res, err := st.http3(requestParam{
+		name:        "TestH3ResponseBeforeRequestEnd",
+		noEndStream: true,
+	})
+	if err != nil {
+		t.Fatalf("Error st.http3() = %v", err)
+	}
+	if got, want := res.status, 404; got != want {
+		t.Errorf("res.status: %v; want %v", got, want)
+	}
+}
+
+// TestH3H1ChunkedEndsPrematurely tests that a stream is reset if the
+// backend chunked encoded response ends prematurely.
+func TestH3H1ChunkedEndsPrematurely(t *testing.T) {
+	opts := options{
+		handler: func(w http.ResponseWriter, r *http.Request) {
+			hj, ok := w.(http.Hijacker)
+			if !ok {
+				http.Error(w, "Could not hijack the connection", http.StatusInternalServerError)
+				return
+			}
+			conn, bufrw, err := hj.Hijack()
+			if err != nil {
+				http.Error(w, err.Error(), http.StatusInternalServerError)
+				return
+			}
+			defer conn.Close()
+			bufrw.WriteString("HTTP/1.1 200\r\nTransfer-Encoding: chunked\r\n\r\n")
+			bufrw.Flush()
+		},
+		quic: true,
+	}
+	st := newServerTester(t, opts)
+	defer st.Close()
+
+	_, err := st.http3(requestParam{
+		name: "TestH3H1ChunkedEndsPrematurely",
+	})
+	if err == nil {
+		t.Fatal("st.http3() should fail")
+	}
+}

src/shrpx_http3_upstream.cc

@@ -984,15 +984,8 @@ int Http3Upstream::on_downstream_abort_request(Downstream *downstream,
 
 int Http3Upstream::on_downstream_abort_request_with_https_redirect(
     Downstream *downstream) {
-  int rv;
+  assert(0);
-
+  abort();
-  rv = redirect_to_https(downstream);
-  if (rv != 0) {
-    return -1;
-  }
-
-  handler_->signal_write();
-  return 0;
 }
 
 namespace {
@@ -1604,10 +1597,11 @@ int Http3Upstream::on_downstream_reset(Downstream *downstream, bool no_retry) {
 
 fail:
   if (rv == SHRPX_ERR_TLS_REQUIRED) {
-    rv = on_downstream_abort_request_with_https_redirect(downstream);
+    assert(0);
-  } else {
+    abort();
-    rv = on_downstream_abort_request(downstream, 502);
   }
+
+  rv = on_downstream_abort_request(downstream, 502);
   if (rv != 0) {
     shutdown_stream(downstream, NGHTTP3_H3_INTERNAL_ERROR);
   }
@@ -2318,10 +2312,11 @@ void Http3Upstream::initiate_downstream(Downstream *downstream) {
     auto dconn = handler_->get_downstream_connection(rv, downstream);
     if (!dconn) {
       if (rv == SHRPX_ERR_TLS_REQUIRED) {
-        rv = redirect_to_https(downstream);
+        assert(0);
-      } else {
+        abort();
-        rv = error_reply(downstream, 502);
       }
+
+      rv = error_reply(downstream, 502);
       if (rv != 0) {
         shutdown_stream(downstream, NGHTTP3_H3_INTERNAL_ERROR);
       }
@@ -2731,39 +2726,6 @@ int Http3Upstream::shutdown_stream_read(int64_t stream_id,
   return 0;
 }
 
-int Http3Upstream::redirect_to_https(Downstream *downstream) {
-  auto &req = downstream->request();
-  if (req.regular_connect_method() || req.scheme != "http") {
-    return error_reply(downstream, 400);
-  }
-
-  auto authority = util::extract_host(req.authority);
-  if (authority.empty()) {
-    return error_reply(downstream, 400);
-  }
-
-  auto &balloc = downstream->get_block_allocator();
-  auto config = get_config();
-  auto &httpconf = config->http;
-
-  StringRef loc;
-  if (httpconf.redirect_https_port == StringRef::from_lit("443")) {
-    loc = concat_string_ref(balloc, StringRef::from_lit("https://"), authority,
-                            req.path);
-  } else {
-    loc = concat_string_ref(balloc, StringRef::from_lit("https://"), authority,
-                            StringRef::from_lit(":"),
-                            httpconf.redirect_https_port, req.path);
-  }
-
-  auto &resp = downstream->response();
-  resp.http_status = 308;
-  resp.fs.add_header_token(StringRef::from_lit("location"), loc, false,
-                           http2::HD_LOCATION);
-
-  return send_reply(downstream, nullptr, 0);
-}
-
 void Http3Upstream::consume(int64_t stream_id, size_t nconsumed) {
   ngtcp2_conn_extend_max_stream_offset(conn_, stream_id, nconsumed);
   ngtcp2_conn_extend_max_offset(conn_, nconsumed);

src/shrpx_http3_upstream.h

@@ -120,7 +120,6 @@ public:
   void initiate_downstream(Downstream *downstream);
   int shutdown_stream(Downstream *downstream, uint64_t app_error_code);
   int shutdown_stream_read(int64_t stream_id, uint64_t app_error_code);
-  int redirect_to_https(Downstream *downstream);
   int http_stream_close(Downstream *downstream, uint64_t app_error_code);
   void consume(int64_t stream_id, size_t nconsumed);
   void remove_downstream(Downstream *downstream);