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

@@ -39,4 +39,4 @@ after_success:
 - bash <(curl -s https://codecov.io/bash)
 
 go:
-- 1.7.5
+- 1.8.3

Dockerfile

@@ -1,4 +1,4 @@
-FROM alpine:3.5
+FROM alpine:3.6
 
 MAINTAINER Minio Inc <dev@minio.io>
 

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

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)

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")},

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 == "" {

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 {

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)

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
 }

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)
 	}
 

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")},
 	}

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,

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
+}

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)},