http: buffer Request.Write

Fixes #1996

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/4639068

diff --git a/src/pkg/http/client_test.go b/src/pkg/http/client_test.go
index d6a9dec3513b6ea0d78da94496460c3fdadbda25..3b85585353f622a1502405e90fac1bfbc7b738d9 100644 (file)
--- a/src/pkg/http/client_test.go
+++ b/src/pkg/http/client_test.go
@@ -12,6 +12,7 @@ import (
        "http/httptest"
        "io"
        "io/ioutil"
+       "net"
        "os"
        "strconv"
        "strings"
@@ -243,3 +244,48 @@ func TestStreamingGet(t *testing.T) {
                t.Fatalf("at end expected EOF, got %v", err)
        }
 }
+
+type writeCountingConn struct {
+       net.Conn
+       count *int
+}
+
+func (c *writeCountingConn) Write(p []byte) (int, os.Error) {
+       *c.count++
+       return c.Conn.Write(p)
+}
+
+// TestClientWrites verifies that client requests are buffered and we
+// don't send a TCP packet per line of the http request + body.
+func TestClientWrites(t *testing.T) {
+       ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+       }))
+       defer ts.Close()
+
+       writes := 0
+       dialer := func(netz string, addr string) (net.Conn, os.Error) {
+               c, err := net.Dial(netz, addr)
+               if err == nil {
+                       c = &writeCountingConn{c, &writes}
+               }
+               return c, err
+       }
+       c := &Client{Transport: &Transport{Dial: dialer}}
+
+       _, err := c.Get(ts.URL)
+       if err != nil {
+               t.Fatal(err)
+       }
+       if writes != 1 {
+               t.Errorf("Get request did %d Write calls, want 1", writes)
+       }
+
+       writes = 0
+       _, err = c.PostForm(ts.URL, Values{"foo": {"bar"}})
+       if err != nil {
+               t.Fatal(err)
+       }
+       if writes != 1 {
+               t.Errorf("Post request did %d Write calls, want 1", writes)
+       }
+}

diff --git a/src/pkg/http/request.go b/src/pkg/http/request.go
index 40ed5b2b6025e3b291a808ef3e8bc95c7ce91f90..183a35c712d70ac5dac34210f1cd2db15c290c04 100644 (file)
--- a/src/pkg/http/request.go
+++ b/src/pkg/http/request.go
@@ -304,10 +304,11 @@ func (req *Request) write(w io.Writer, usingProxy bool) os.Error {
                }
        }
 
-       fmt.Fprintf(w, "%s %s HTTP/1.1\r\n", valueOrDefault(req.Method, "GET"), uri)
+       bw := bufio.NewWriter(w)
+       fmt.Fprintf(bw, "%s %s HTTP/1.1\r\n", valueOrDefault(req.Method, "GET"), uri)
 
        // Header lines
-       fmt.Fprintf(w, "Host: %s\r\n", host)
+       fmt.Fprintf(bw, "Host: %s\r\n", host)
 
        // Use the defaultUserAgent unless the Header contains one, which
        // may be blank to not send the header.
@@ -318,7 +319,7 @@ func (req *Request) write(w io.Writer, usingProxy bool) os.Error {
                }
        }
        if userAgent != "" {
-               fmt.Fprintf(w, "User-Agent: %s\r\n", userAgent)
+               fmt.Fprintf(bw, "User-Agent: %s\r\n", userAgent)
        }
 
        // Process Body,ContentLength,Close,Trailer
@@ -326,25 +327,25 @@ func (req *Request) write(w io.Writer, usingProxy bool) os.Error {
        if err != nil {
                return err
        }
-       err = tw.WriteHeader(w)
+       err = tw.WriteHeader(bw)
        if err != nil {
                return err
        }
 
        // TODO: split long values?  (If so, should share code with Conn.Write)
-       err = req.Header.WriteSubset(w, reqWriteExcludeHeader)
+       err = req.Header.WriteSubset(bw, reqWriteExcludeHeader)
        if err != nil {
                return err
        }
 
-       io.WriteString(w, "\r\n")
+       io.WriteString(bw, "\r\n")
 
        // Write body and trailer
-       err = tw.WriteBody(w)
+       err = tw.WriteBody(bw)
        if err != nil {
                return err
        }
-
+       bw.Flush()
        return nil
 }