Bläddra i källkod

http2: change how Server.IdleTimeout is initialized from http.Server

This is an alternate implementation of https://golang.org/cl/32230
which is nicer to the dead code elimination in Go's linker.

The old implementation causes a test in the net/http package to fail:
https://storage.googleapis.com/go-build-log/2c24cf88/linux-amd64_39728ac9.log
... since it caused the cmd/go binary to link in the whole http1 & http2 Server
code, due to the init func and slice which referenced those symbols.

Instead, use an explicit func.

This aso includes the tests meant to be in CL 32230 but which I'd
failed to git add earlier.

Updates golang/go#14204

Change-Id: I13dc138bf0c4df65bc282133ee94036b7f84ab70
Reviewed-on: https://go-review.googlesource.com/32323
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <martisch@uos.de>
Brad Fitzpatrick 9 år sedan
förälder
incheckning
76c1a11e06
4 ändrade filer med 82 tillägg och 8 borttagningar
  1. 0 4
      http2/go18.go
  2. 66 0
      http2/go18_test.go
  3. 14 0
      http2/not_go18.go
  4. 2 4
      http2/server.go

+ 0 - 4
http2/go18.go

@@ -25,10 +25,6 @@ func (w *responseWriter) Push(target string, opts *http.PushOptions) error {
 	return w.push(target, internalOpts)
 }
 
-func init() {
-	configServerFuncs = append(configServerFuncs, configureServer18)
-}
-
 func configureServer18(h1 *http.Server, h2 *Server) error {
 	if h2.IdleTimeout == 0 {
 		if h1.IdleTimeout != 0 {

+ 66 - 0
http2/go18_test.go

@@ -0,0 +1,66 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build go1.8
+
+package http2
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+// Tests that http2.Server.IdleTimeout is initialized from
+// http.Server.{Idle,Read}Timeout. http.Server.IdleTimeout was
+// added in Go 1.8.
+func TestConfigureServerIdleTimeout_Go18(t *testing.T) {
+	const timeout = 5 * time.Second
+	const notThisOne = 1 * time.Second
+
+	// With a zero http2.Server, verify that it copies IdleTimeout:
+	{
+		s1 := &http.Server{
+			IdleTimeout: timeout,
+			ReadTimeout: notThisOne,
+		}
+		s2 := &Server{}
+		if err := ConfigureServer(s1, s2); err != nil {
+			t.Fatal(err)
+		}
+		if s2.IdleTimeout != timeout {
+			t.Errorf("s2.IdleTimeout = %v; want %v", s2.IdleTimeout, timeout)
+		}
+	}
+
+	// And that it falls back to ReadTimeout:
+	{
+		s1 := &http.Server{
+			ReadTimeout: timeout,
+		}
+		s2 := &Server{}
+		if err := ConfigureServer(s1, s2); err != nil {
+			t.Fatal(err)
+		}
+		if s2.IdleTimeout != timeout {
+			t.Errorf("s2.IdleTimeout = %v; want %v", s2.IdleTimeout, timeout)
+		}
+	}
+
+	// Verify that s1's IdleTimeout doesn't overwrite an existing setting:
+	{
+		s1 := &http.Server{
+			IdleTimeout: notThisOne,
+		}
+		s2 := &Server{
+			IdleTimeout: timeout,
+		}
+		if err := ConfigureServer(s1, s2); err != nil {
+			t.Fatal(err)
+		}
+		if s2.IdleTimeout != timeout {
+			t.Errorf("s2.IdleTimeout = %v; want %v", s2.IdleTimeout, timeout)
+		}
+	}
+}

+ 14 - 0
http2/not_go18.go

@@ -0,0 +1,14 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build !go1.8
+
+package http2
+
+import "net/http"
+
+func configureServer18(h1 *http.Server, h2 *Server) error {
+	// No IdleTimeout to sync prior to Go 1.8.
+	return nil
+}

+ 2 - 4
http2/server.go

@@ -145,10 +145,8 @@ func ConfigureServer(s *http.Server, conf *Server) error {
 	if conf == nil {
 		conf = new(Server)
 	}
-	for _, fn := range configServerFuncs {
-		if err := fn(s, conf); err != nil {
-			return err
-		}
+	if err := configureServer18(s, conf); err != nil {
+		return err
 	}
 
 	if s.TLSConfig == nil {