From b4dc6df35c69210dde2c8355fcd137b240985a5b Mon Sep 17 00:00:00 2001 From: "A. Elleuch" Date: Sun, 6 Aug 2017 19:27:33 +0100 Subject: [PATCH] go1.8: Changes to support golang 1.8 (#4759) QuirkConn is added to replace net.Conn as a workaround to a golang bug: https://github.com/golang/go/issues/21133 --- .travis.yml | 2 +- Dockerfile | 2 +- appveyor.yml | 4 ++-- buildscripts/checkdeps.sh | 2 +- cmd/endpoint_test.go | 6 +---- cmd/net.go | 16 +++++++------ cmd/net_test.go | 7 +++--- cmd/notify-webhook_test.go | 2 +- cmd/test-utils_test.go | 43 ++++++++++++++-------------------- main.go | 6 ++--- main_test.go | 6 ++--- pkg/http/bufconn.go | 6 ++--- pkg/http/conn_bug_21133.go | 48 ++++++++++++++++++++++++++++++++++++++ pkg/http/listener_test.go | 5 +--- 14 files changed, 96 insertions(+), 59 deletions(-) create mode 100644 pkg/http/conn_bug_21133.go diff --git a/.travis.yml b/.travis.yml index e37f4da01..4e1ad3d15 100644 --- a/.travis.yml +++ b/.travis.yml @@ -39,4 +39,4 @@ after_success: - bash <(curl -s https://codecov.io/bash) go: -- 1.7.5 +- 1.8.3 diff --git a/Dockerfile b/Dockerfile index 6e193a5e5..9f1febfdd 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM alpine:3.5 +FROM alpine:3.6 MAINTAINER Minio Inc diff --git a/appveyor.yml b/appveyor.yml index 1e3f3b8e8..7a4e15041 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -11,12 +11,12 @@ clone_folder: c:\gopath\src\github.com\minio\minio # Environment variables environment: - GOROOT: c:\go17 + GOVERSION: 1.8.3 GOPATH: c:\gopath # scripts that run after cloning repository install: - - set PATH=%GOPATH%\bin;c:\go17\bin;%PATH% + - set PATH=%GOPATH%\bin;c:\go\bin;%PATH% - go version - go env - python --version diff --git a/buildscripts/checkdeps.sh b/buildscripts/checkdeps.sh index c843b546d..645158cb7 100644 --- a/buildscripts/checkdeps.sh +++ b/buildscripts/checkdeps.sh @@ -21,7 +21,7 @@ _init() { ## Minimum required versions for build dependencies GIT_VERSION="1.0" - GO_VERSION="1.7.1" + GO_VERSION="1.8.3" OSX_VERSION="10.8" KNAME=$(uname -s) ARCH=$(uname -m) diff --git a/cmd/endpoint_test.go b/cmd/endpoint_test.go index c01fd7a6c..ed3fdff8d 100644 --- a/cmd/endpoint_test.go +++ b/cmd/endpoint_test.go @@ -20,7 +20,6 @@ import ( "fmt" "net/url" "reflect" - "runtime" "sort" "strings" "testing" @@ -33,9 +32,6 @@ func TestNewEndpoint(t *testing.T) { u4, _ := url.Parse("http://192.168.253.200/path") errMsg := ": no such host" - if runtime.GOOS == "windows" { - errMsg = ": No such host is known." - } testCases := []struct { arg string @@ -232,7 +228,7 @@ func TestCreateEndpoints(t *testing.T) { expectedSetupType SetupType expectedErr error }{ - {"localhost", []string{}, "", EndpointList{}, -1, fmt.Errorf("missing port in address localhost")}, + {"localhost", []string{}, "", EndpointList{}, -1, fmt.Errorf("address localhost: missing port in address")}, // FS Setup {"localhost:9000", []string{"http://localhost/d1"}, "", EndpointList{}, -1, fmt.Errorf("use path style endpoint for FS setup")}, diff --git a/cmd/net.go b/cmd/net.go index d846773cc..4c73ecc81 100644 --- a/cmd/net.go +++ b/cmd/net.go @@ -198,18 +198,20 @@ func extractHostPort(hostAddr string) (string, string, error) { return "", "", errors.New("unable to process empty address") } + // Simplify the work of url.Parse() and always send a url with + if !strings.HasPrefix(hostAddr, "http://") && !strings.HasPrefix(hostAddr, "https://") { + hostAddr = "//" + hostAddr + } + // Parse address to extract host and scheme field u, err := url.Parse(hostAddr) if err != nil { - // Ignore scheme not present error - if !strings.Contains(err.Error(), "missing protocol scheme") { - return "", "", err - } - } else { - addr = u.Host - scheme = u.Scheme + return "", "", err } + addr = u.Host + scheme = u.Scheme + // Use the given parameter again if url.Parse() // didn't return any useful result. if addr == "" { diff --git a/cmd/net_test.go b/cmd/net_test.go index f020c27f4..50d3c1818 100644 --- a/cmd/net_test.go +++ b/cmd/net_test.go @@ -223,7 +223,7 @@ func TestCheckLocalServerAddr(t *testing.T) { {"localhost:54321", nil}, {"0.0.0.0:9000", nil}, {"", fmt.Errorf("missing port in address")}, - {"localhost", fmt.Errorf("missing port in address localhost")}, + {"localhost", fmt.Errorf("address localhost: missing port in address")}, {"example.org:54321", fmt.Errorf("host in server address should be this server")}, {":0", fmt.Errorf("port number must be between 1 to 65535")}, {":-10", fmt.Errorf("port number must be between 1 to 65535")}, @@ -251,7 +251,6 @@ func TestExtractHostPort(t *testing.T) { expectedErr error }{ {"", "", "", errors.New("unable to process empty address")}, - {"localhost", "localhost", "80", nil}, {"localhost:9000", "localhost", "9000", nil}, {"http://:9000/", "", "9000", nil}, {"http://8.8.8.8:9000/", "8.8.8.8", "9000", nil}, @@ -294,7 +293,9 @@ func TestSameLocalAddrs(t *testing.T) { {":9000", ":9000", true, nil}, {"localhost:9000", ":9000", true, nil}, {"localhost:9000", "http://localhost:9000", true, nil}, - {"8.8.8.8:9000", "http://localhost:9000", false, nil}, + {"http://localhost:9000", ":9000", true, nil}, + {"http://localhost:9000", "http://localhost:9000", true, nil}, + {"http://8.8.8.8:9000", "http://localhost:9000", false, nil}, } for i, testCase := range testCases { diff --git a/cmd/notify-webhook_test.go b/cmd/notify-webhook_test.go index 4462b330f..c42b5e4e8 100644 --- a/cmd/notify-webhook_test.go +++ b/cmd/notify-webhook_test.go @@ -58,7 +58,7 @@ func TestNewWebHookNotify(t *testing.T) { t.Fatal("Unexpected should fail") } - serverConfig.Notify.SetWebhookByID("10", webhookNotify{Enable: true, Endpoint: "http://127.0.0.1:xxx"}) + serverConfig.Notify.SetWebhookByID("10", webhookNotify{Enable: true, Endpoint: "http://127.0.0.1:80"}) _, err = newWebhookNotify("10") if err != nil { t.Fatal("Unexpected should not fail with lookupEndpoint", err) diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index b81148a20..fc6347794 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -939,14 +939,10 @@ func signRequestV2(req *http.Request, accessKey, secretKey string) error { req.Header.Set("Date", d.Format(http.TimeFormat)) } - // url.RawPath will be valid if path has any encoded characters, if not it will - // be empty - in which case we need to consider url.Path (bug in net/http?) - encodedResource := req.URL.RawPath - if encodedResource == "" { - splits := strings.Split(req.URL.Path, "?") - if len(splits) > 0 { - encodedResource = getURLEncodedName(splits[0]) - } + splits := strings.Split(req.URL.Path, "?") + var encodedResource string + if len(splits) > 0 { + encodedResource = getURLEncodedName(splits[0]) } encodedQuery := req.URL.Query().Encode() @@ -1088,16 +1084,9 @@ func newTestRequest(method, urlStr string, contentLength int64, body io.ReadSeek method = "POST" } - req, err := http.NewRequest(method, urlStr, nil) - if err != nil { - return nil, err - } - - // Add Content-Length - req.ContentLength = contentLength - // Save for subsequent use var hashedPayload string + var md5Base64 string switch { case body == nil: hashedPayload = getSHA256Hash([]byte{}) @@ -1107,21 +1096,25 @@ func newTestRequest(method, urlStr string, contentLength int64, body io.ReadSeek return nil, err } hashedPayload = getSHA256Hash(payloadBytes) - md5Base64 := getMD5HashBase64(payloadBytes) - req.Header.Set("Content-Md5", md5Base64) + md5Base64 = getMD5HashBase64(payloadBytes) } - req.Header.Set("x-amz-content-sha256", hashedPayload) // Seek back to beginning. if body != nil { body.Seek(0, 0) - // Add body - req.Body = ioutil.NopCloser(body) } else { - // this is added to avoid panic during ioutil.ReadAll(req.Body). - // th stack trace can be found here https://github.com/minio/minio/pull/2074 . - // This is very similar to https://github.com/golang/go/issues/7527. - req.Body = ioutil.NopCloser(bytes.NewReader([]byte(""))) + body = bytes.NewReader([]byte("")) } + req, err := http.NewRequest(method, urlStr, body) + if err != nil { + return nil, err + } + if md5Base64 != "" { + req.Header.Set("Content-Md5", md5Base64) + } + req.Header.Set("x-amz-content-sha256", hashedPayload) + + // Add Content-Length + req.ContentLength = contentLength return req, nil } diff --git a/main.go b/main.go index 892305e51..6c296030d 100644 --- a/main.go +++ b/main.go @@ -33,8 +33,8 @@ import ( ) const ( - // Minio requires at least Go v1.7 - minGoVersion = "1.7" + // Minio requires at least Go v1.8.3 + minGoVersion = "1.8.3" goVersionConstraint = ">= " + minGoVersion ) @@ -51,7 +51,7 @@ func checkGoVersion(goVersionStr string) error { } if !constraint.Check(goVersion) { - return fmt.Errorf("Minio is not compiled by Go %s. Please recompile accordingly.", + return fmt.Errorf("Minio is not compiled by Go %s. Please recompile accordingly", goVersionConstraint) } diff --git a/main_test.go b/main_test.go index 85b9b2e8e..ba9fbdbd0 100644 --- a/main_test.go +++ b/main_test.go @@ -28,9 +28,9 @@ func TestCheckGoVersion(t *testing.T) { expectedErr error }{ {minGoVersion, nil}, - {minGoVersion + ".10", nil}, - {"1.6.8", fmt.Errorf("Minio is not compiled by Go >= 1.7. Please recompile accordingly.")}, - {"0.1", fmt.Errorf("Minio is not compiled by Go >= 1.7. Please recompile accordingly.")}, + {"1.6.8", fmt.Errorf("Minio is not compiled by Go >= 1.8.3. Please recompile accordingly")}, + {"1.5", fmt.Errorf("Minio is not compiled by Go >= 1.8.3. Please recompile accordingly")}, + {"0.1", fmt.Errorf("Minio is not compiled by Go >= 1.8.3. Please recompile accordingly")}, {".1", fmt.Errorf("Malformed version: .1")}, {"somejunk", fmt.Errorf("Malformed version: somejunk")}, } diff --git a/pkg/http/bufconn.go b/pkg/http/bufconn.go index cf6679502..b21c1ac2c 100644 --- a/pkg/http/bufconn.go +++ b/pkg/http/bufconn.go @@ -24,7 +24,7 @@ import ( // BufConn - is a generic stream-oriented network connection supporting buffered reader and read/write timeout. type BufConn struct { - net.Conn + QuirkConn bufReader *bufio.Reader // buffered reader wraps reader in net.Conn. readTimeout time.Duration // sets the read timeout in the connection. writeTimeout time.Duration // sets the write timeout in the connection. @@ -34,7 +34,7 @@ type BufConn struct { // Sets read timeout func (c *BufConn) setReadTimeout() { - if c.readTimeout != 0 { + if c.readTimeout != 0 && c.canSetReadDeadline() { c.SetReadDeadline(time.Now().UTC().Add(c.readTimeout)) } } @@ -91,7 +91,7 @@ func (c *BufConn) Write(b []byte) (n int, err error) { func newBufConn(c net.Conn, readTimeout, writeTimeout time.Duration, updateBytesReadFunc, updateBytesWrittenFunc func(int)) *BufConn { return &BufConn{ - Conn: c, + QuirkConn: QuirkConn{Conn: c}, bufReader: bufio.NewReader(c), readTimeout: readTimeout, writeTimeout: writeTimeout, diff --git a/pkg/http/conn_bug_21133.go b/pkg/http/conn_bug_21133.go new file mode 100644 index 000000000..8c59332c0 --- /dev/null +++ b/pkg/http/conn_bug_21133.go @@ -0,0 +1,48 @@ +/* + * Minio Cloud Storage, (C) 2017 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package http + +import ( + "net" + "sync/atomic" + "time" +) + +// QuirkConn - similar to golang net.Conn struct, but contains a workaround of the +// following the go bug reported here https://github.com/golang/go/issues/21133. +// Once the bug will be fixed, we can remove this structure and replaces it with +// the standard net.Conn +type QuirkConn struct { + net.Conn + hadReadDeadlineInPast int32 // atomic +} + +// SetReadDeadline - implements a workaround of SetReadDeadline go bug +func (q *QuirkConn) SetReadDeadline(t time.Time) error { + inPast := int32(0) + if t.Before(time.Now()) { + inPast = 1 + } + atomic.StoreInt32(&q.hadReadDeadlineInPast, inPast) + return q.Conn.SetReadDeadline(t) +} + +// canSetReadDeadline - returns if it is safe to set a new +// read deadline without triggering golang/go#21133 issue. +func (q *QuirkConn) canSetReadDeadline() bool { + return atomic.LoadInt32(&q.hadReadDeadlineInPast) != 1 +} diff --git a/pkg/http/listener_test.go b/pkg/http/listener_test.go index 149e405fb..08aa10460 100644 --- a/pkg/http/listener_test.go +++ b/pkg/http/listener_test.go @@ -180,9 +180,6 @@ func TestIsHTTPMethod(t *testing.T) { func TestNewHTTPListener(t *testing.T) { errMsg := ": no such host" - if runtime.GOOS == "windows" { - errMsg = ": No such host is known." - } remoteAddrErrMsg := "listen tcp 93.184.216.34:9000: bind: cannot assign requested address" if runtime.GOOS == "windows" { @@ -204,7 +201,7 @@ func TestNewHTTPListener(t *testing.T) { }{ {[]string{"93.184.216.34:9000"}, nil, time.Duration(0), time.Duration(0), time.Duration(0), nil, nil, nil, errors.New(remoteAddrErrMsg)}, {[]string{"example.org:9000"}, nil, time.Duration(0), time.Duration(0), time.Duration(0), nil, nil, nil, errors.New(remoteAddrErrMsg)}, - {[]string{"unknown-host"}, nil, time.Duration(0), time.Duration(0), time.Duration(0), nil, nil, nil, errors.New("listen tcp: missing port in address unknown-host")}, + {[]string{"unknown-host"}, nil, time.Duration(0), time.Duration(0), time.Duration(0), nil, nil, nil, errors.New("listen tcp: address unknown-host: missing port in address")}, {[]string{"unknown-host:9000"}, nil, time.Duration(0), time.Duration(0), time.Duration(0), nil, nil, nil, errors.New("listen tcp: lookup unknown-host" + errMsg)}, {[]string{"localhost:9000", "93.184.216.34:9000"}, nil, time.Duration(0), time.Duration(0), time.Duration(0), nil, nil, nil, errors.New(remoteAddrErrMsg)}, {[]string{"localhost:9000", "unknown-host:9000"}, nil, time.Duration(0), time.Duration(0), time.Duration(0), nil, nil, nil, errors.New("listen tcp: lookup unknown-host" + errMsg)},