net/http: use dynamic type assertion to remove HTTP server code from cmd/go

I was wondering why cmd/go includes the HTTP server implementations.

Dumping the linker's deadcode dependency graph into a file and doing
some graph analysis, I found that the only reason cmd/go included an
HTTP server was because the maxBytesReader type (used by both the HTTP
transport & HTTP server) did a static type assertion to an HTTP server
type.

Changing it to a interface type assertion reduces the size of cmd/go
by 533KB (5.2%)

On linux/amd64, cmd/go goes from 10549200 to 10002624 bytes.

Add a test too so this doesn't regress. The test uses cmd/go as the
binary to test (a binary which needs the HTTP client but not the HTTP
server), but this change and test are equally applicable to any such
program.

Change-Id: I93865f43ec03b06d09241fbd9ea381817c2909c5
Reviewed-on: https://go-review.googlesource.com/20763
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/net/http/http_test.go b/src/net/http/http_test.go
index dead3b0454223325e948e756c52fc52893bf585a..3267d478eea51d8cfa0af5c0800db98a1dcec620 100644 (file)
--- a/src/net/http/http_test.go
+++ b/src/net/http/http_test.go
@@ -2,12 +2,17 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Tests of internal functions with no better homes.
+// Tests of internal functions and things with no better homes.
 
 package http
 
 import (
+       "bytes"
+       "internal/testenv"
+       "os/exec"
+       "path/filepath"
        "reflect"
+       "runtime"
        "testing"
 )
 
@@ -56,3 +61,39 @@ func TestCleanHost(t *testing.T) {
                }
        }
 }
+
+// Test that cmd/go doesn't link in the HTTP server.
+//
+// This catches accidental dependencies between the HTTP transport and
+// server code.
+func TestCmdGoNoHTTPServer(t *testing.T) {
+       testenv.MustHaveGoBuild(t)
+       var exeSuffix string
+       if runtime.GOOS == "windows" {
+               exeSuffix = ".exe"
+       }
+
+       goBin := filepath.Join(runtime.GOROOT(), "bin", "go"+exeSuffix)
+       out, err := exec.Command("go", "tool", "nm", goBin).Output()
+       if err != nil {
+               t.Fatalf("go tool nm: %v", err)
+       }
+       wantSym := map[string]bool{
+               // Verify these exist: (sanity checking this test)
+               "net/http.(*Client).Get":          true,
+               "net/http.(*Transport).RoundTrip": true,
+
+               // Verify these don't exist:
+               "net/http.http2Server":     false,
+               "net/http.(*Server).Serve": false,
+       }
+       for sym, want := range wantSym {
+               got := bytes.Contains(out, []byte(sym))
+               if !want && got {
+                       t.Errorf("cmd/go unexpectedly links in HTTP server code; found symbol %q in cmd/go", sym)
+               }
+               if want && !got {
+                       t.Errorf("expected to find symbol %q in cmd/go; not found", sym)
+               }
+       }
+}

diff --git a/src/net/http/request.go b/src/net/http/request.go
index 9dba0c33b55323f38cd71d991ade8429d79541c7..ba487cfa31ac850f2b97229543dcb5f1ed7b52c1 100644 (file)
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -820,7 +820,18 @@ type maxBytesReader struct {
 func (l *maxBytesReader) tooLarge() (n int, err error) {
        if !l.stopped {
                l.stopped = true
-               if res, ok := l.w.(*response); ok {
+
+               // The server code and client code both use
+               // maxBytesReader. This "requestTooLarge" check is
+               // only used by the server code. To prevent binaries
+               // which only using the HTTP Client code (such as
+               // cmd/go) from also linking in the HTTP server, don't
+               // use a static type assertion to the server
+               // "*response" type. Check this interface instead:
+               type requestTooLarger interface {
+                       requestTooLarge()
+               }
+               if res, ok := l.w.(requestTooLarger); ok {
                        res.requestTooLarge()
                }
        }