From 126b5f9d2b64a87b8da8c0b723bc6ffcffdad84c Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 20 Nov 2022 15:03:41 +0900 Subject: [PATCH 1/9] Add the missing file --- integration-tests/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/integration-tests/Makefile.am b/integration-tests/Makefile.am index c2d39dfa..38cd1197 100644 --- a/integration-tests/Makefile.am +++ b/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 = \ From 20d95edc5738a0b7b6a5000b1ed39289e5e62895 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 20 Nov 2022 15:04:01 +0900 Subject: [PATCH 2/9] integration: Add http3 via tests --- integration-tests/nghttpx_http3_test.go | 87 +++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/integration-tests/nghttpx_http3_test.go b/integration-tests/nghttpx_http3_test.go index bd1e2e91..7a151946 100644 --- a/integration-tests/nghttpx_http3_test.go +++ b/integration-tests/nghttpx_http3_test.go @@ -1,4 +1,5 @@ //go:build quic + package nghttp2 import ( @@ -7,6 +8,8 @@ import ( "io" "net/http" "testing" + + "golang.org/x/net/http2/hpack" ) // TestH3H1PlainGET tests whether simple HTTP/3 GET request works. @@ -84,3 +87,87 @@ 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) + } +} From 17a5ba4969911987cc0adae834b8079033c4aaa8 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 20 Nov 2022 15:12:19 +0900 Subject: [PATCH 3/9] integration: Add http3 response content-length test --- integration-tests/nghttpx_http3_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/integration-tests/nghttpx_http3_test.go b/integration-tests/nghttpx_http3_test.go index 7a151946..5765c464 100644 --- a/integration-tests/nghttpx_http3_test.go +++ b/integration-tests/nghttpx_http3_test.go @@ -171,3 +171,26 @@ func TestH3H1NoVia(t *testing.T) { 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") + } +} From 5b0cbb489248415a250bd4306829ad2cbc9406f5 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 20 Nov 2022 16:28:16 +0900 Subject: [PATCH 4/9] integration: Add http3 HTTPS redirect test --- integration-tests/nghttpx_http3_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/integration-tests/nghttpx_http3_test.go b/integration-tests/nghttpx_http3_test.go index 5765c464..87f28e05 100644 --- a/integration-tests/nghttpx_http3_test.go +++ b/integration-tests/nghttpx_http3_test.go @@ -194,3 +194,25 @@ func TestH3H1BadResponseCL(t *testing.T) { 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) + } +} From 5e1b1a088309b3475a86f632f216727374a008b2 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 20 Nov 2022 16:31:24 +0900 Subject: [PATCH 5/9] integration: Add http3 affinity cookie test --- integration-tests/nghttpx_http3_test.go | 30 +++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/integration-tests/nghttpx_http3_test.go b/integration-tests/nghttpx_http3_test.go index 87f28e05..9bac1652 100644 --- a/integration-tests/nghttpx_http3_test.go +++ b/integration-tests/nghttpx_http3_test.go @@ -7,6 +7,7 @@ import ( "crypto/rand" "io" "net/http" + "regexp" "testing" "golang.org/x/net/http2/hpack" @@ -216,3 +217,32 @@ func TestH3H1HTTPSRedirect(t *testing.T) { 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) + } +} From 17e3bb4ec5d1aaac14441b24a849b6c50590a252 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 20 Nov 2022 16:35:54 +0900 Subject: [PATCH 6/9] integration: Add http3 mruby tests --- integration-tests/nghttpx_http3_test.go | 85 +++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/integration-tests/nghttpx_http3_test.go b/integration-tests/nghttpx_http3_test.go index 9bac1652..28478365 100644 --- a/integration-tests/nghttpx_http3_test.go +++ b/integration-tests/nghttpx_http3_test.go @@ -246,3 +246,88 @@ func TestH3H1AffinityCookieTLS(t *testing.T) { 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) + } +} From 2bef60a6f4ae42cecc2cfe72a67284322f98c738 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 20 Nov 2022 16:53:34 +0900 Subject: [PATCH 7/9] integration: Add http3 test which verifies response ends before request --- integration-tests/nghttpx_http3_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/integration-tests/nghttpx_http3_test.go b/integration-tests/nghttpx_http3_test.go index 28478365..11b36c6c 100644 --- a/integration-tests/nghttpx_http3_test.go +++ b/integration-tests/nghttpx_http3_test.go @@ -331,3 +331,28 @@ func TestH3H2RespPhaseReturn(t *testing.T) { 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) + } +} From 2d790edac596da5da361475c949aec3e4ffedbb1 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 20 Nov 2022 16:55:58 +0900 Subject: [PATCH 8/9] integration: Add http3 test which verifies chunked encoding ends prematurely --- integration-tests/nghttpx_http3_test.go | 32 +++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/integration-tests/nghttpx_http3_test.go b/integration-tests/nghttpx_http3_test.go index 11b36c6c..ee4a8ebf 100644 --- a/integration-tests/nghttpx_http3_test.go +++ b/integration-tests/nghttpx_http3_test.go @@ -356,3 +356,35 @@ func TestH3ResponseBeforeRequestEnd(t *testing.T) { 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") + } +} From babeddb6499f3baae5bf3befa5f503e615cdacf9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 20 Nov 2022 17:03:30 +0900 Subject: [PATCH 9/9] nghttpx: HTTPS redirect should not happen with HTTP/3 upstream --- src/shrpx_http3_upstream.cc | 58 +++++++------------------------------ src/shrpx_http3_upstream.h | 1 - 2 files changed, 10 insertions(+), 49 deletions(-) diff --git a/src/shrpx_http3_upstream.cc b/src/shrpx_http3_upstream.cc index ac3399d7..c92f195c 100644 --- a/src/shrpx_http3_upstream.cc +++ b/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; - - rv = redirect_to_https(downstream); - if (rv != 0) { - return -1; - } - - handler_->signal_write(); - return 0; + assert(0); + abort(); } 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); - } else { - rv = on_downstream_abort_request(downstream, 502); + assert(0); + abort(); } + + 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); - } else { - rv = error_reply(downstream, 502); + assert(0); + abort(); } + + 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); diff --git a/src/shrpx_http3_upstream.h b/src/shrpx_http3_upstream.h index 23874850..cc0d2353 100644 --- a/src/shrpx_http3_upstream.h +++ b/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);