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.
This commit is contained in:
Harshavardhana 2018-10-04 10:13:12 -07:00 committed by kannappanr
parent ea9408ccbb
commit 02ad9d6072
2 changed files with 17 additions and 26 deletions

View File

@ -35,9 +35,7 @@ import (
var sslRequiredErrMsg = []byte("HTTP/1.0 403 Forbidden\r\n\r\nSSL required") var sslRequiredErrMsg = []byte("HTTP/1.0 403 Forbidden\r\n\r\nSSL required")
var malformedErrMsgFn = func(data interface{}) string { var badRequestMsg = []byte("HTTP/1.0 400 Bad Request\r\n\r\n")
return fmt.Sprintf("HTTP/1.0 400 Bad Request\r\n\r\n%s", data)
}
// HTTP methods. // HTTP methods.
var methods = []string{ var methods = []string{
@ -97,7 +95,7 @@ func getPlainText(bufConn *BufConn) (bool, error) {
return false, nil 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() defer bufConn.setReadTimeout()
var data []byte var data []byte
@ -120,11 +118,15 @@ func getResourceHost(bufConn *BufConn, maxHeaderBytes int) (resource string, met
if method == "" && resource == "" { if method == "" && resource == "" {
if i := strings.IndexByte(tokens[0], ' '); i == -1 { 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) httpTokens := strings.SplitN(tokens[0], " ", 3)
if len(httpTokens) < 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] method = httpTokens[0]
@ -141,7 +143,7 @@ func getResourceHost(bufConn *BufConn, maxHeaderBytes int) (resource string, met
token = strings.ToLower(token) token = strings.ToLower(token)
if strings.HasPrefix(token, "host: ") { if strings.HasPrefix(token, "host: ") {
host = strings.TrimPrefix(strings.TrimSuffix(token, "\r"), "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() bufconn.Close()
return return
} }
if ok { if ok {
// As TLS is configured and we got plain text HTTP request, // As TLS is configured and we got plain text HTTP request,
// return 403 (forbidden) error. // return 403 (forbidden) error.
@ -241,8 +244,10 @@ func (listener *httpListener) start() {
bufconn.Close() bufconn.Close()
return return
} }
// As the listener is configured with TLS, try to do TLS handshake, drop the connection if it fails. // As the listener is configured with TLS, try to do TLS handshake, drop the connection if it fails.
tlsConn := tls.Server(bufconn, listener.tlsConfig) tlsConn := tls.Server(bufconn, listener.tlsConfig)
if err := tlsConn.Handshake(); err != nil { if err := tlsConn.Handshake(); err != nil {
reqInfo := (&logger.ReqInfo{}).AppendTags("remoteAddr", bufconn.RemoteAddr().String()) reqInfo := (&logger.ReqInfo{}).AppendTags("remoteAddr", bufconn.RemoteAddr().String())
reqInfo.AppendTags("localAddr", bufconn.LocalAddr().String()) reqInfo.AppendTags("localAddr", bufconn.LocalAddr().String())
@ -251,10 +256,11 @@ func (listener *httpListener) start() {
bufconn.Close() bufconn.Close()
return return
} }
bufconn = newBufConn(tlsConn, listener.readTimeout, listener.writeTimeout) 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 { if err != nil {
// Peek could fail legitimately when clients abruptly close // Peek could fail legitimately when clients abruptly close
// connection. E.g. Chrome browser opens connections speculatively to // connection. E.g. Chrome browser opens connections speculatively to
@ -266,20 +272,8 @@ func (listener *httpListener) start() {
reqInfo.AppendTags("localAddr", bufconn.LocalAddr().String()) reqInfo.AppendTags("localAddr", bufconn.LocalAddr().String())
ctx := logger.SetReqInfo(context.Background(), reqInfo) ctx := logger.SetReqInfo(context.Background(), reqInfo)
logger.LogIf(ctx, err) 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() bufconn.Close()
return return
} }

View File

@ -657,11 +657,8 @@ func TestHTTPListenerAcceptError(t *testing.T) {
continue continue
} }
s, err := bufio.NewReader(conn).ReadString('\n') if _, err = bufio.NewReader(conn).ReadString('\n'); err != io.EOF {
if err != nil { t.Errorf("Test %d: reply read: expected = io.EOF, got = %s", i+1, err)
t.Errorf("Test %d: reply read: expected = <nil>, 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)
} }
conn.Close() conn.Close()