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 = \ diff --git a/integration-tests/nghttpx_http3_test.go b/integration-tests/nghttpx_http3_test.go index bd1e2e91..ee4a8ebf 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 ( @@ -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") + } +} 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);