Fix http client handling of status messages with spaces (e.g. "HTTP/1.1 400 Bad

Request".
Use chunked Transfer-Encoding for all POSTs.
Implement chunked reading.
Change http.Request.write to be HTTP/1.1 only.

R=rsc
APPROVED=rsc
DELTA=178  (123 added, 26 deleted, 29 changed)
OCL=30563
CL=30673

src/pkg/http/client.go

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Primitive HTTP client.  See RFC 2616.
+// Primitive HTTP client. See RFC 2616.
 
 package http
 
@@ -14,8 +14,8 @@ import (
 	"log";
 	"net";
 	"os";
-	"strings";
 	"strconv";
+	"strings";
 )
 
 // Response represents the response from an HTTP request.
@@ -73,24 +73,6 @@ type readClose struct {
 
 // Send issues an HTTP request.  Caller should close resp.Body when done reading it.
 //
-// This method consults the following fields of req:
-//
-//	Url
-//	Method (defaults to "GET")
-//	Proto (defaults to "HTTP/1.0")
-//	UserAgent (if empty, currently defaults to http.Client; may change)
-//	Referer (if empty, no Referer header will be supplied)
-//	Header
-//	Body (if nil, defaults to empty body)
-//
-// The following fields are redundant and are ignored:
-//
-//	RawUrl
-//	ProtoMajor
-//	ProtoMinor
-//	Close
-//	Host
-//
 // TODO: support persistent connections (multiple requests on a single connection).
 // send() method is nonpublic because, when we refactor the code for persistent
 // connections, it may no longer make sense to have a method with this signature.
@@ -126,14 +108,15 @@ func send(req *Request) (resp *Response, err os.Error) {
 	if err != nil {
 		return nil, err;
 	}
-	ss := strings.Split(line, " ");
-	if len(ss) != 3 {
+	i := strings.Index(line, " ");
+	j := strings.Index(line[i+1:len(line)], " ") + i+1;
+	if i < 0 || j < 0 {
 		return nil, os.ErrorString(fmt.Sprintf("Invalid first line in HTTP response: %q", line));
 	}
-	resp.Status = ss[1] + " " + ss[2];
-	resp.StatusCode, err = strconv.Atoi(ss[1]);
+	resp.Status = line[i+1:len(line)];
+	resp.StatusCode, err = strconv.Atoi(line[i+1:j]);
 	if err != nil {
-		return nil, os.ErrorString(fmt.Sprintf("Invalid status code in HTTP response %q", line));
+		return nil, os.ErrorString(fmt.Sprintf("Invalid status code in HTTP response: %q", line));
 	}
 
 	// Parse the response headers.
@@ -148,7 +131,14 @@ func send(req *Request) (resp *Response, err os.Error) {
 		resp.AddHeader(key, value);
 	}
 
-	resp.Body = readClose{reader, conn};
+	// TODO(rsc): Make this work:
+	//   r := io.Reader(reader);
+	var r io.Reader = reader;
+	if v := resp.GetHeader("Transfer-Encoding"); v == "chunked" {
+		r = newChunkedReader(reader);
+	}
+	resp.Body = readClose{ r, conn };
+
 	conn = nil; // so that defered func won't close it
 	err = nil;
 	return;
@@ -209,14 +199,16 @@ func Get(url string) (r *Response, finalUrl string, err os.Error) {
 
 // Post issues a POST to the specified URL.
 //
-// Caller should close resp.Body when done reading it.
-func Post(url string, requestBody io.Reader) (r *Response, err os.Error) {
-	// NOTE TO REVIEWER: this could share more code with Get, waiting for API to settle
-	// down before cleaning up that detail.
-
+// Caller should close r.Body when done reading it.
+func Post(url string, bodyType string, body io.Reader) (r *Response, err os.Error) {
 	var req Request;
 	req.Method = "POST";
-	req.Body = requestBody;
+	req.Body = body;
+	req.Header = map[string] string{
+		"Content-Type": bodyType,
+		"Transfer-Encoding": "chunked",
+	};
+
 	req.Url, err = ParseURL(url);
 	if err != nil {
 		return nil, err;
@@ -224,4 +216,3 @@ func Post(url string, requestBody io.Reader) (r *Response, err os.Error) {
 
 	return send(&req);
 }
-

src/pkg/http/request.go

@@ -24,6 +24,7 @@ const (
 	maxLineLength = 1024;	// assumed < bufio.DefaultBufSize
 	maxValueLength = 1024;
 	maxHeaderLines = 1024;
+	chunkSize = 4 << 10;  // 4 KB chunks
 )
 
 // HTTP request parsing errors.
@@ -41,6 +42,7 @@ var (
 	BadRequest = &ProtocolError{"invalid http request"};
 	BadHTTPVersion = &ProtocolError{"unsupported http version"};
 	UnknownContentType = &ProtocolError{"unknown content type"};
+	BadChunkedEncoding = &ProtocolError{"bad chunked encoding"};
 )
 
 // A Request represents a parsed HTTP request header.
@@ -122,22 +124,35 @@ func valueOrDefault(value, def string) string {
 // TODO(rsc): Change default UserAgent before open-source release.
 const defaultUserAgent = "http.Client";
 
-// Write an HTTP request -- header and body -- in wire format.
-// See Send for a list of which Request fields we use.
+// Write an HTTP/1.1 request -- header and body -- in wire format.
+// This method consults the following fields of req:
+//	Url
+//	Method (defaults to "GET")
+//	UserAgent (defaults to defaultUserAgent)
+//	Referer
+//	Header
+//	Body
+//
+// If Body is present, "Transfer-Encoding: chunked" is forced as a header.
 func (req *Request) write(w io.Writer) os.Error {
 	uri := URLEscape(req.Url.Path);
 	if req.Url.RawQuery != "" {
 		uri += "?" + req.Url.RawQuery;
 	}
 
-	fmt.Fprintf(w, "%s %s %s\r\n", valueOrDefault(req.Method, "GET"), uri, valueOrDefault(req.Proto, "HTTP/1.0"));
+	fmt.Fprintf(w, "%s %s HTTP/1.1\r\n", valueOrDefault(req.Method, "GET"), uri);
 	fmt.Fprintf(w, "Host: %s\r\n", req.Url.Host);
 	fmt.Fprintf(w, "User-Agent: %s\r\n", valueOrDefault(req.UserAgent, defaultUserAgent));
 
-	if (req.Referer != "") {
+	if req.Referer != "" {
 		fmt.Fprintf(w, "Referer: %s\r\n", req.Referer);
 	}
 
+	if req.Body != nil {
+		// Force chunked encoding
+		req.Header["Transfer-Encoding"] = "chunked";
+	}
+
 	// TODO: split long values?  (If so, should share code with Conn.Write)
 	// TODO: if Header includes values for Host, User-Agent, or Referer, this
 	// may conflict with the User-Agent or Referer headers we add manually.
@@ -152,10 +167,32 @@ func (req *Request) write(w io.Writer) os.Error {
 	io.WriteString(w, "\r\n");
 
 	if req.Body != nil {
-		_, err := io.Copy(req.Body, w);
-		if err != nil {
-			return err;
+		buf := make([]byte, chunkSize);
+	Loop:
+		for {
+			var nr, nw int;
+			var er, ew os.Error
+			if nr, er = req.Body.Read(buf); nr > 0 {
+				if er == nil || er == os.EOF {
+					fmt.Fprintf(w, "%x\r\n", nr);
+					nw, ew = w.Write(buf[0:nr]);
+					fmt.Fprint(w, "\r\n");
+				}
+			}
+			switch {
+			case er != nil:
+				if er == os.EOF {
+					break Loop
+				}
+				return er;
+			case ew != nil:
+				return ew;
+			case nw < nr:
+				return io.ErrShortWrite;
+			}
 		}
+		// last-chunk CRLF
+		fmt.Fprint(w, "0\r\n\r\n");
 	}
 
 	return nil;
@@ -330,6 +367,70 @@ func CanonicalHeaderKey(s string) string {
 	return t;
 }
 
+type chunkedReader struct {
+	r *bufio.Reader;
+	n uint64;  // unread bytes in chunk
+	err os.Error;
+}
+
+func newChunkedReader(r *bufio.Reader) *chunkedReader {
+	return &chunkedReader{ r: r }
+}
+
+func (cr *chunkedReader) beginChunk() {
+	// chunk-size CRLF
+	var line string;
+	line, cr.err = readLine(cr.r);
+	if cr.err != nil {
+		return
+	}
+	cr.n, cr.err = strconv.Btoui64(line, 16);
+	if cr.err != nil {
+		return
+	}
+	if cr.n == 0 {
+		// trailer CRLF
+		for {
+			line, cr.err = readLine(cr.r);
+			if cr.err != nil {
+				return
+			}
+			if line == "" {
+				break
+			}
+		}
+		cr.err = os.EOF;
+	}
+}
+
+func (cr *chunkedReader) Read(b []uint8) (n int, err os.Error) {
+	if cr.err != nil {
+		return 0, cr.err
+	}
+	if cr.n == 0 {
+		cr.beginChunk();
+		if cr.err != nil {
+			return 0, cr.err
+		}
+	}
+	if uint64(len(b)) > cr.n {
+		b = b[0:cr.n];
+	}
+	n, cr.err = cr.r.Read(b);
+	cr.n -= uint64(n);
+	if cr.n == 0 && cr.err == nil {
+		// end of chunk (CRLF)
+		b := make([]byte, 2);
+		var nb int;
+		if nb, cr.err = io.ReadFull(cr.r, b); cr.err == nil {
+			if b[0] != '\r' || b[1] != '\n' {
+				cr.err = BadChunkedEncoding;
+			}
+		}
+	}
+	return n, cr.err
+}
+
 // ReadRequest reads and parses a request from b.
 func ReadRequest(b *bufio.Reader) (req *Request, err os.Error) {
 	req = new(Request);
@@ -449,8 +550,10 @@ func ReadRequest(b *bufio.Reader) (req *Request, err os.Error) {
 	//	Warning
 
 	// A message body exists when either Content-Length or Transfer-Encoding
-	// headers are present. TODO: Handle Transfer-Encoding.
-	if v, present := req.Header["Content-Length"]; present {
+	// headers are present. Transfer-Encoding trumps Content-Length.
+	if v, present := req.Header["Transfer-Encoding"]; present && v == "chunked" {
+		req.Body = newChunkedReader(b);
+	} else if v, present := req.Header["Content-Length"]; present {
 		length, err := strconv.Btoui64(v, 10);
 		if err != nil {
 			return nil, BadContentLength
@@ -493,6 +596,7 @@ func parseForm(body string) (data map[string] *vector.StringVector, err os.Error
 }
 
 // ParseForm parses the request body as a form.
+// TODO(dsymonds): Parse r.Url.RawQuery instead for GET requests.
 func (r *Request) ParseForm() (err os.Error) {
 	if r.Body == nil {
 		return NoEntityBody