From a96884cf6c76a5d409ec4b193b6cc52534b80bad Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 16 Mar 2016 18:26:43 +0000 Subject: [PATCH] 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 Reviewed-by: Josh Bleecher Snyder Run-TryBot: Brad Fitzpatrick Reviewed-by: David Crawshaw TryBot-Result: Gobot Gobot --- src/net/http/http_test.go | 43 ++++++++++++++++++++++++++++++++++++++- src/net/http/request.go | 13 +++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/net/http/http_test.go b/src/net/http/http_test.go index dead3b0454..3267d478ee 100644 --- 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 9dba0c33b5..ba487cfa31 100644 --- 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() } } -- 2.48.1