From daec7c16d3fccec6cd623b03feeb93bc3ba25d0b Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 20 Jan 2015 22:19:28 +0900 Subject: [PATCH] nghttpx: Don't discard data on HTTP/1 backend EOF Also HTTP/1 frontend testing method was added. --- integration-tests/nghttpx_test.go | 37 ++++++++++++++++++++++++ integration-tests/server_tester.go | 46 ++++++++++++++++++++++++++++++ src/shrpx_https_upstream.cc | 12 ++++++-- 3 files changed, 92 insertions(+), 3 deletions(-) diff --git a/integration-tests/nghttpx_test.go b/integration-tests/nghttpx_test.go index f646967b..189df0ad 100644 --- a/integration-tests/nghttpx_test.go +++ b/integration-tests/nghttpx_test.go @@ -9,6 +9,43 @@ import ( "testing" ) +func TestH1H1PlainGET(t *testing.T) { + st := newServerTester(nil, t, noopHandler) + defer st.Close() + + res, err := st.http1(requestParam{ + name: "TestH1H1PlainGET", + }) + if err != nil { + t.Errorf("Error st.http1() = %v", err) + } + + want := 200 + if got := res.status; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + +func TestH1H1PlainGETClose(t *testing.T) { + st := newServerTester(nil, t, noopHandler) + defer st.Close() + + res, err := st.http1(requestParam{ + name: "TestH1H1PlainGET", + header: []hpack.HeaderField{ + pair("Connection", "close"), + }, + }) + if err != nil { + t.Fatalf("Error st.http1() = %v", err) + } + + want := 200 + if got := res.status; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + func TestH2H1PlainGET(t *testing.T) { st := newServerTester(nil, t, noopHandler) defer st.Close() diff --git a/integration-tests/server_tester.go b/integration-tests/server_tester.go index e4197e2d..6b859222 100644 --- a/integration-tests/server_tester.go +++ b/integration-tests/server_tester.go @@ -1,6 +1,7 @@ package nghttp2 import ( + "bufio" "bytes" "crypto/tls" "errors" @@ -8,6 +9,8 @@ import ( "github.com/bradfitz/http2" "github.com/bradfitz/http2/hpack" "github.com/tatsuhiro-t/go-nghttp2" + "io" + "io/ioutil" "net" "net/http" "net/http/httptest" @@ -34,6 +37,7 @@ func pair(name, value string) hpack.HeaderField { type serverTester struct { args []string // command-line arguments cmd *exec.Cmd // test frontend server process, which is test subject + url string // test frontend server URL t *testing.T ts *httptest.Server // backend server conn net.Conn // connection to frontend server @@ -87,6 +91,7 @@ func newServerTester(args []string, t *testing.T, handler http.HandlerFunc) *ser cmd: exec.Command(serverBin, args...), t: t, ts: ts, + url: fmt.Sprintf("http://127.0.0.1:%v", serverPort), nextStreamID: 1, authority: u.Host, frCh: make(chan http2.Frame), @@ -165,6 +170,47 @@ type requestParam struct { body []byte // request body } +func (st *serverTester) http1(rp requestParam) (*serverResponse, error) { + method := "GET" + if rp.method != "" { + method = rp.method + } + + var body io.Reader + if rp.body != nil { + body = bytes.NewBuffer(rp.body) + } + req, err := http.NewRequest(method, st.url, body) + if err != nil { + return nil, err + } + for _, h := range rp.header { + req.Header.Add(h.Name, h.Value) + } + req.Header.Add("Test-Case", rp.name) + + if err := req.Write(st.conn); err != nil { + return nil, err + } + resp, err := http.ReadResponse(bufio.NewReader(st.conn), req) + if err != nil { + return nil, err + } + respBody, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, err + } + resp.Body.Close() + + res := &serverResponse{ + status: resp.StatusCode, + header: resp.Header, + body: respBody, + } + + return res, nil +} + func (st *serverTester) http2(rp requestParam) (*serverResponse, error) { res := &serverResponse{} st.headerBlkBuf.Reset() diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 7f6cdbd6..a51df58e 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -506,6 +506,11 @@ int HttpsUpstream::downstream_eof(DownstreamConnection *dconn) { if (LOG_ENABLED(INFO)) { DCLOG(INFO, dconn) << "EOF"; } + + if (downstream->get_response_state() == Downstream::MSG_COMPLETE) { + goto end; + } + if (downstream->get_response_state() == Downstream::HEADER_COMPLETE) { // Server may indicate the end of the request by EOF if (LOG_ENABLED(INFO)) { @@ -518,10 +523,11 @@ int HttpsUpstream::downstream_eof(DownstreamConnection *dconn) { goto end; } - if (downstream->get_response_state() != Downstream::MSG_COMPLETE) { - // error + if (downstream->get_response_state() == Downstream::INITIAL) { + // we did not send any response headers, so we can reply error + // message. if (LOG_ENABLED(INFO)) { - DCLOG(INFO, dconn) << "Treated as error"; + DCLOG(INFO, dconn) << "Return error reply"; } if (error_reply(502) != 0) { return -1;