From 02ad9d6072f65325f87d85b527c389986e3b713d Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 4 Oct 2018 10:13:12 -0700 Subject: [PATCH] Avoid unsolicited response over idle http client (#6562) Without this PR minio server is writing an erroneous response to clients on an idle connections which ends up printing following message ``` Unsolicited response received on idle HTTP channel ``` This PR would avoid sending responses on idle connections .i.e routine network errors. --- cmd/http/listener.go | 36 +++++++++++++++--------------------- cmd/http/listener_test.go | 7 ++----- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/cmd/http/listener.go b/cmd/http/listener.go index 46986ee03..0c3ab658c 100644 --- a/cmd/http/listener.go +++ b/cmd/http/listener.go @@ -35,9 +35,7 @@ import ( var sslRequiredErrMsg = []byte("HTTP/1.0 403 Forbidden\r\n\r\nSSL required") -var malformedErrMsgFn = func(data interface{}) string { - return fmt.Sprintf("HTTP/1.0 400 Bad Request\r\n\r\n%s", data) -} +var badRequestMsg = []byte("HTTP/1.0 400 Bad Request\r\n\r\n") // HTTP methods. var methods = []string{ @@ -97,7 +95,7 @@ func getPlainText(bufConn *BufConn) (bool, error) { return false, nil } -func getResourceHost(bufConn *BufConn, maxHeaderBytes int) (resource string, method string, host string, err error) { +func getMethodResourceHost(bufConn *BufConn, maxHeaderBytes int) (method string, resource string, host string, err error) { defer bufConn.setReadTimeout() var data []byte @@ -120,11 +118,15 @@ func getResourceHost(bufConn *BufConn, maxHeaderBytes int) (resource string, met if method == "" && resource == "" { if i := strings.IndexByte(tokens[0], ' '); i == -1 { - return "", "", "", fmt.Errorf("malformed HTTP request %s, from %s", tokens[0], bufConn.LocalAddr()) + return "", "", "", fmt.Errorf("malformed HTTP request from '%s'", bufConn.LocalAddr()) } httpTokens := strings.SplitN(tokens[0], " ", 3) if len(httpTokens) < 3 { - return "", "", "", fmt.Errorf("malformed HTTP request %s, from %s", tokens[0], bufConn.LocalAddr()) + return "", "", "", fmt.Errorf("malformed HTTP request from '%s'", bufConn.LocalAddr()) + } + if !isHTTPMethod(httpTokens[0]) { + return "", "", "", fmt.Errorf("malformed HTTP request, invalid HTTP method '%s' from '%s'", + httpTokens[0], bufConn.LocalAddr()) } method = httpTokens[0] @@ -141,7 +143,7 @@ func getResourceHost(bufConn *BufConn, maxHeaderBytes int) (resource string, met token = strings.ToLower(token) if strings.HasPrefix(token, "host: ") { host = strings.TrimPrefix(strings.TrimSuffix(token, "\r"), "host: ") - return resource, method, host, nil + return method, resource, host, nil } } @@ -234,6 +236,7 @@ func (listener *httpListener) start() { bufconn.Close() return } + if ok { // As TLS is configured and we got plain text HTTP request, // return 403 (forbidden) error. @@ -241,8 +244,10 @@ func (listener *httpListener) start() { bufconn.Close() return } + // As the listener is configured with TLS, try to do TLS handshake, drop the connection if it fails. tlsConn := tls.Server(bufconn, listener.tlsConfig) + if err := tlsConn.Handshake(); err != nil { reqInfo := (&logger.ReqInfo{}).AppendTags("remoteAddr", bufconn.RemoteAddr().String()) reqInfo.AppendTags("localAddr", bufconn.LocalAddr().String()) @@ -251,10 +256,11 @@ func (listener *httpListener) start() { bufconn.Close() return } + bufconn = newBufConn(tlsConn, listener.readTimeout, listener.writeTimeout) } - resource, method, host, err := getResourceHost(bufconn, listener.maxHeaderBytes) + method, resource, host, err := getMethodResourceHost(bufconn, listener.maxHeaderBytes) if err != nil { // Peek could fail legitimately when clients abruptly close // connection. E.g. Chrome browser opens connections speculatively to @@ -266,20 +272,8 @@ func (listener *httpListener) start() { reqInfo.AppendTags("localAddr", bufconn.LocalAddr().String()) ctx := logger.SetReqInfo(context.Background(), reqInfo) logger.LogIf(ctx, err) + bufconn.Write(badRequestMsg) } - bufconn.Write([]byte(malformedErrMsgFn(err))) - bufconn.Close() - return - } - - // Return bufconn if read data is a valid HTTP method. - if !isHTTPMethod(method) { - reqInfo := (&logger.ReqInfo{}).AppendTags("remoteAddr", bufconn.RemoteAddr().String()) - reqInfo.AppendTags("localAddr", bufconn.LocalAddr().String()) - ctx := logger.SetReqInfo(context.Background(), reqInfo) - err = fmt.Errorf("malformed HTTP invalid HTTP method %s", method) - logger.LogIf(ctx, err) - bufconn.Write([]byte(malformedErrMsgFn(err))) bufconn.Close() return } diff --git a/cmd/http/listener_test.go b/cmd/http/listener_test.go index 41d2de3b3..8c639185f 100644 --- a/cmd/http/listener_test.go +++ b/cmd/http/listener_test.go @@ -657,11 +657,8 @@ func TestHTTPListenerAcceptError(t *testing.T) { continue } - s, err := bufio.NewReader(conn).ReadString('\n') - if err != nil { - t.Errorf("Test %d: reply read: expected = , got = %s", i+1, err) - } else if !strings.Contains(s, "HTTP/1.0 400 Bad Request") { - t.Errorf("Test %d: reply read: expected = 'HTTP/1.0 400 Bad Request' got = %s", i+1, s) + if _, err = bufio.NewReader(conn).ReadString('\n'); err != io.EOF { + t.Errorf("Test %d: reply read: expected = io.EOF, got = %s", i+1, err) } conn.Close()