mime/multipart: misc code/doc fixes

R=rsc
CC=golang-dev
https://golang.org/cl/4532089

src/pkg/mime/multipart/writer.go

@@ -6,24 +6,18 @@ package multipart
 
 import (
 	"bytes"
+	"crypto/rand"
 	"fmt"
 	"io"
 	"net/textproto"
 	"os"
-	"rand"
 	"strings"
 )
 
-// Writer is used to generate multipart messages.
+// A Writer generates multipart messages.
 type Writer struct {
-	// Boundary is the random boundary string between
-	// parts. NewWriter will generate this but it must
-	// not be changed after a part has been created.
-	// Setting this to an invalid value will generate
-	// malformed messages.
-	Boundary string
-
 	w        io.Writer
+	boundary string
 	lastpart *part
 }
 
@@ -32,38 +26,42 @@ type Writer struct {
 func NewWriter(w io.Writer) *Writer {
 	return &Writer{
 		w:        w,
-		Boundary: randomBoundary(),
+		boundary: randomBoundary(),
 	}
 }
 
+// Boundary returns the Writer's randomly selected boundary string.
+func (w *Writer) Boundary() string {
+	return w.boundary
+}
+
 // FormDataContentType returns the Content-Type for an HTTP
 // multipart/form-data with this Writer's Boundary.
 func (w *Writer) FormDataContentType() string {
-	return "multipart/form-data; boundary=" + w.Boundary
+	return "multipart/form-data; boundary=" + w.boundary
 }
 
-const randChars = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
-
 func randomBoundary() string {
-	var buf [60]byte
+	var buf [30]byte
-	for i := range buf {
+	_, err := io.ReadFull(rand.Reader, buf[:])
-		buf[i] = randChars[rand.Intn(len(randChars))]
+	if err != nil {
+		panic(err)
 	}
-	return string(buf[:])
+	return fmt.Sprintf("%x", buf[:])
 }
 
 // CreatePart creates a new multipart section with the provided
-// header. The previous part, if still open, is closed. The body of
+// header. The body of the part should be written to the returned
-// the part should be written to the returned WriteCloser. Closing the
+// Writer. After calling CreatePart, any previous part may no longer
-// returned WriteCloser after writing is optional.
+// be written to.
-func (w *Writer) CreatePart(header textproto.MIMEHeader) (io.WriteCloser, os.Error) {
+func (w *Writer) CreatePart(header textproto.MIMEHeader) (io.Writer, os.Error) {
 	if w.lastpart != nil {
-		if err := w.lastpart.Close(); err != nil {
+		if err := w.lastpart.close(); err != nil {
 			return nil, err
 		}
 	}
 	var b bytes.Buffer
-	fmt.Fprintf(&b, "\r\n--%s\r\n", w.Boundary)
+	fmt.Fprintf(&b, "\r\n--%s\r\n", w.boundary)
 	// TODO(bradfitz): move this to textproto.MimeHeader.Write(w), have it sort
 	// and clean, like http.Header.Write(w) does.
 	for k, vv := range header {
@@ -91,7 +89,7 @@ func escapeQuotes(s string) string {
 
 // CreateFormFile is a convenience wrapper around CreatePart. It creates
 // a new form-data header with the provided field name and file name.
-func (w *Writer) CreateFormFile(fieldname, filename string) (io.WriteCloser, os.Error) {
+func (w *Writer) CreateFormFile(fieldname, filename string) (io.Writer, os.Error) {
 	h := make(textproto.MIMEHeader)
 	h.Set("Content-Disposition",
 		fmt.Sprintf(`form-data; name="%s"; filename="%s"`,
@@ -100,40 +98,35 @@ func (w *Writer) CreateFormFile(fieldname, filename string) (io.WriteCloser, os.
 	return w.CreatePart(h)
 }
 
-// CreateFormField is a convenience wrapper around CreatePart. It creates
+// CreateFormField calls calls CreatePart with a header using the
-// a new form-data header with the provided field name.
+// given field name.
-func (w *Writer) CreateFormField(fieldname string) (io.WriteCloser, os.Error) {
+func (w *Writer) CreateFormField(fieldname string) (io.Writer, os.Error) {
 	h := make(textproto.MIMEHeader)
 	h.Set("Content-Disposition",
 		fmt.Sprintf(`form-data; name="%s"`, escapeQuotes(fieldname)))
 	return w.CreatePart(h)
 }
 
-// WriteField is a convenience wrapper around CreateFormField. It creates and
+// WriteField calls CreateFormField and then writes the given value.
-// writes a part with the provided name and value.
 func (w *Writer) WriteField(fieldname, value string) os.Error {
 	p, err := w.CreateFormField(fieldname)
 	if err != nil {
 		return err
 	}
 	_, err = p.Write([]byte(value))
-	if err != nil {
+	return err
-		return err
-	}
-	return p.Close()
 }
 
-// Close finishes the multipart message. It closes the previous part,
+// Close finishes the multipart message and writes the trailing
-// if still open, and writes the trailing boundary end line to the
+// boundary end line to the output.
-// output.
 func (w *Writer) Close() os.Error {
 	if w.lastpart != nil {
-		if err := w.lastpart.Close(); err != nil {
+		if err := w.lastpart.close(); err != nil {
 			return err
 		}
 		w.lastpart = nil
 	}
-	_, err := fmt.Fprintf(w.w, "\r\n--%s--\r\n", w.Boundary)
+	_, err := fmt.Fprintf(w.w, "\r\n--%s--\r\n", w.boundary)
 	return err
 }
 
@@ -143,14 +136,14 @@ type part struct {
 	we     os.Error // last error that occurred writing
 }
 
-func (p *part) Close() os.Error {
+func (p *part) close() os.Error {
 	p.closed = true
 	return p.we
 }
 
 func (p *part) Write(d []byte) (n int, err os.Error) {
 	if p.closed {
-		return 0, os.NewError("multipart: Write after Close")
+		return 0, os.NewError("multipart: can't write to finished part")
 	}
 	n, err = p.mw.w.Write(d)
 	if err != nil {

src/pkg/mime/multipart/writer_test.go

@@ -32,7 +32,7 @@ func TestWriter(t *testing.T) {
 		}
 	}
 
-	r := NewReader(&b, w.Boundary)
+	r := NewReader(&b, w.Boundary())
 
 	part, err := r.NextPart()
 	if err != nil {