http/spdy: improve error handling.

Create a new spdy.Error type that includes the enumerated error type and
the associated stream id (0 if not associated with a specific stream).
This will let users handle errors differently (RST_STREAM vs GOAWAY).

R=bradfitz, rsc, rogpeppe
CC=golang-dev
https://golang.org/cl/4532131

4 files changed, 93 insertions, 60 deletions

diff --git a/src/pkg/http/spdy/read.go b/src/pkg/http/spdy/read.go
index 159dbc5780..8adec7bd4f 100644
--- a/src/pkg/http/spdy/read.go
+++ b/src/pkg/http/spdy/read.go
@@ -80,7 +80,7 @@ func (frame *HeadersFrame) read(h ControlFrameHeader, f *Framer) os.Error {
 func newControlFrame(frameType ControlFrameType) (controlFrame, os.Error) {
 	ctor, ok := cframeCtor[frameType]
 	if !ok {
-		return nil, InvalidControlFrame
+		return nil, &Error{Err: InvalidControlFrame}
 	}
 	return ctor(), nil
 }
@@ -97,30 +97,12 @@ var cframeCtor = map[ControlFrameType]func() controlFrame{
 	// TODO(willchan): Add TypeWindowUpdate
 }
 
-type corkedReader struct {
-	r  io.Reader
-	ch chan int
-	n  int
-}
-
-func (cr *corkedReader) Read(p []byte) (int, os.Error) {
-	if cr.n == 0 {
-		cr.n = <-cr.ch
-	}
-	if len(p) > cr.n {
-		p = p[:cr.n]
-	}
-	n, err := cr.r.Read(p)
-	cr.n -= n
-	return n, err
-}
-
-func (f *Framer) uncorkHeaderDecompressor(payloadSize int) os.Error {
+func (f *Framer) uncorkHeaderDecompressor(payloadSize int64) os.Error {
 	if f.headerDecompressor != nil {
-		f.headerReader.ch <- payloadSize
+		f.headerReader.N = payloadSize
 		return nil
 	}
-	f.headerReader = corkedReader{r: f.r, ch: make(chan int, 1), n: payloadSize}
+	f.headerReader = io.LimitedReader{R: f.r, N: payloadSize}
 	decompressor, err := zlib.NewReaderDict(&f.headerReader, []byte(HeaderDictionary))
 	if err != nil {
 		return err
@@ -161,11 +143,12 @@ func (f *Framer) parseControlFrame(version uint16, frameType ControlFrameType) (
 	return cframe, nil
 }
 
-func parseHeaderValueBlock(r io.Reader) (http.Header, os.Error) {
+func parseHeaderValueBlock(r io.Reader, streamId uint32) (http.Header, os.Error) {
 	var numHeaders uint16
 	if err := binary.Read(r, binary.BigEndian, &numHeaders); err != nil {
 		return nil, err
 	}
+	var e os.Error
 	h := make(http.Header, int(numHeaders))
 	for i := 0; i < int(numHeaders); i++ {
 		var length uint16
@@ -178,10 +161,11 @@ func parseHeaderValueBlock(r io.Reader) (http.Header, os.Error) {
 		}
 		name := string(nameBytes)
 		if name != strings.ToLower(name) {
-			return nil, UnlowercasedHeaderName
+			e = &Error{UnlowercasedHeaderName, streamId}
+			name = strings.ToLower(name)
 		}
 		if h[name] != nil {
-			return nil, DuplicateHeaders
+			e = &Error{DuplicateHeaders, streamId}
 		}
 		if err := binary.Read(r, binary.BigEndian, &length); err != nil {
 			return nil, err
@@ -195,6 +179,9 @@ func parseHeaderValueBlock(r io.Reader) (http.Header, os.Error) {
 			h.Add(name, v)
 		}
 	}
+	if e != nil {
+		return h, e
+	}
 	return h, nil
 }
 
@@ -214,14 +201,25 @@ func (f *Framer) readSynStreamFrame(h ControlFrameHeader, frame *SynStreamFrame)
 
 	reader := f.r
 	if !f.headerCompressionDisabled {
-		f.uncorkHeaderDecompressor(int(h.length - 10))
+		f.uncorkHeaderDecompressor(int64(h.length - 10))
 		reader = f.headerDecompressor
 	}
 
-	frame.Headers, err = parseHeaderValueBlock(reader)
+	frame.Headers, err = parseHeaderValueBlock(reader, frame.StreamId)
+	if !f.headerCompressionDisabled && ((err == os.EOF && f.headerReader.N == 0) || f.headerReader.N != 0) {
+		err = &Error{WrongCompressedPayloadSize, 0}
+	}
 	if err != nil {
 		return err
 	}
+	// Remove this condition when we bump Version to 3.
+	if Version >= 3 {
+		for h, _ := range frame.Headers {
+			if invalidReqHeaders[h] {
+				return &Error{InvalidHeaderPresent, frame.StreamId}
+			}
+		}
+	}
 	return nil
 }
 
@@ -237,13 +235,24 @@ func (f *Framer) readSynReplyFrame(h ControlFrameHeader, frame *SynReplyFrame) o
 	}
 	reader := f.r
 	if !f.headerCompressionDisabled {
-		f.uncorkHeaderDecompressor(int(h.length - 6))
+		f.uncorkHeaderDecompressor(int64(h.length - 6))
 		reader = f.headerDecompressor
 	}
-	frame.Headers, err = parseHeaderValueBlock(reader)
+	frame.Headers, err = parseHeaderValueBlock(reader, frame.StreamId)
+	if !f.headerCompressionDisabled && ((err == os.EOF && f.headerReader.N == 0) || f.headerReader.N != 0) {
+		err = &Error{WrongCompressedPayloadSize, 0}
+	}
 	if err != nil {
 		return err
 	}
+	// Remove this condition when we bump Version to 3.
+	if Version >= 3 {
+		for h, _ := range frame.Headers {
+			if invalidRespHeaders[h] {
+				return &Error{InvalidHeaderPresent, frame.StreamId}
+			}
+		}
+	}
 	return nil
 }
 
@@ -259,13 +268,31 @@ func (f *Framer) readHeadersFrame(h ControlFrameHeader, frame *HeadersFrame) os.
 	}
 	reader := f.r
 	if !f.headerCompressionDisabled {
-		f.uncorkHeaderDecompressor(int(h.length - 6))
+		f.uncorkHeaderDecompressor(int64(h.length - 6))
 		reader = f.headerDecompressor
 	}
-	frame.Headers, err = parseHeaderValueBlock(reader)
+	frame.Headers, err = parseHeaderValueBlock(reader, frame.StreamId)
+	if !f.headerCompressionDisabled && ((err == os.EOF && f.headerReader.N == 0) || f.headerReader.N != 0) {
+		err = &Error{WrongCompressedPayloadSize, 0}
+	}
 	if err != nil {
 		return err
 	}
+
+	// Remove this condition when we bump Version to 3.
+	if Version >= 3 {
+		var invalidHeaders map[string]bool
+		if frame.StreamId%2 == 0 {
+			invalidHeaders = invalidReqHeaders
+		} else {
+			invalidHeaders = invalidRespHeaders
+		}
+		for h, _ := range frame.Headers {
+			if invalidHeaders[h] {
+				return &Error{InvalidHeaderPresent, frame.StreamId}
+			}
+		}
+	}
 	return nil
 }
 
@@ -279,7 +306,6 @@ func (f *Framer) parseDataFrame(streamId uint32) (*DataFrame, os.Error) {
 	frame.Flags = DataFlags(length >> 24)
 	length &= 0xffffff
 	frame.Data = make([]byte, length)
-	// TODO(willchan): Support compressed data frames.
 	if _, err := io.ReadFull(f.r, frame.Data); err != nil {
 		return nil, err
 	}
diff --git a/src/pkg/http/spdy/spdy_test.go b/src/pkg/http/spdy/spdy_test.go
index 9100e1ea89..cb91e02861 100644
--- a/src/pkg/http/spdy/spdy_test.go
+++ b/src/pkg/http/spdy/spdy_test.go
@@ -21,7 +21,8 @@ func TestHeaderParsing(t *testing.T) {
 	var headerValueBlockBuf bytes.Buffer
 	writeHeaderValueBlock(&headerValueBlockBuf, headers)
 
-	newHeaders, err := parseHeaderValueBlock(&headerValueBlockBuf)
+	const bogusStreamId = 1
+	newHeaders, err := parseHeaderValueBlock(&headerValueBlockBuf, bogusStreamId)
 	if err != nil {
 		t.Fatal("parseHeaderValueBlock:", err)
 	}
diff --git a/src/pkg/http/spdy/types.go b/src/pkg/http/spdy/types.go
index 5a665f04ff..41cafb1741 100644
--- a/src/pkg/http/spdy/types.go
+++ b/src/pkg/http/spdy/types.go
@@ -10,7 +10,6 @@ import (
 	"http"
 	"io"
 	"os"
-	"strconv"
 )
 
 //  Data Frame Format
@@ -302,33 +301,41 @@ const HeaderDictionary = "optionsgetheadpostputdeletetrace" +
 	"chunkedtext/htmlimage/pngimage/jpgimage/gifapplication/xmlapplication/xhtmltext/plainpublicmax-age" +
 	"charset=iso-8859-1utf-8gzipdeflateHTTP/1.1statusversionurl\x00"
 
-type FramerError int
+// A SPDY specific error.
+type ErrorCode string
 
 const (
-	Internal FramerError = iota
-	InvalidControlFrame
-	UnlowercasedHeaderName
-	DuplicateHeaders
-	UnknownFrameType
-	InvalidDataFrame
+	UnlowercasedHeaderName     ErrorCode = "header was not lowercased"
+	DuplicateHeaders           ErrorCode = "multiple headers with same name"
+	WrongCompressedPayloadSize ErrorCode = "compressed payload size was incorrect"
+	UnknownFrameType           ErrorCode = "unknown frame type"
+	InvalidControlFrame        ErrorCode = "invalid control frame"
+	InvalidDataFrame           ErrorCode = "invalid data frame"
+	InvalidHeaderPresent       ErrorCode = "frame contained invalid header"
 )
 
-func (e FramerError) String() string {
-	switch e {
-	case Internal:
-		return "Internal"
-	case InvalidControlFrame:
-		return "InvalidControlFrame"
-	case UnlowercasedHeaderName:
-		return "UnlowercasedHeaderName"
-	case DuplicateHeaders:
-		return "DuplicateHeaders"
-	case UnknownFrameType:
-		return "UnknownFrameType"
-	case InvalidDataFrame:
-		return "InvalidDataFrame"
-	}
-	return "Error(" + strconv.Itoa(int(e)) + ")"
+// Error contains both the type of error and additional values. StreamId is 0
+// if Error is not associated with a stream.
+type Error struct {
+	Err      ErrorCode
+	StreamId uint32
+}
+
+func (e *Error) String() string {
+	return string(e.Err)
+}
+
+var invalidReqHeaders = map[string]bool{
+	"Connection":        true,
+	"Keep-Alive":        true,
+	"Proxy-Connection":  true,
+	"Transfer-Encoding": true,
+}
+
+var invalidRespHeaders = map[string]bool{
+	"Connection":        true,
+	"Keep-Alive":        true,
+	"Transfer-Encoding": true,
 }
 
 // Framer handles serializing/deserializing SPDY frames, including compressing/
@@ -339,7 +346,7 @@ type Framer struct {
 	headerBuf                 *bytes.Buffer
 	headerCompressor          *zlib.Writer
 	r                         io.Reader
-	headerReader              corkedReader
+	headerReader              io.LimitedReader
 	headerDecompressor        io.ReadCloser
 }
 
diff --git a/src/pkg/http/spdy/write.go b/src/pkg/http/spdy/write.go
index aa1679f1bd..7d40bbe9fe 100644
--- a/src/pkg/http/spdy/write.go
+++ b/src/pkg/http/spdy/write.go
@@ -267,10 +267,9 @@ func (f *Framer) writeHeadersFrame(frame *HeadersFrame) (err os.Error) {
 func (f *Framer) writeDataFrame(frame *DataFrame) (err os.Error) {
 	// Validate DataFrame
 	if frame.StreamId&0x80000000 != 0 || len(frame.Data) >= 0x0f000000 {
-		return InvalidDataFrame
+		return &Error{InvalidDataFrame, frame.StreamId}
 	}
 
-	// TODO(willchan): Support data compression.
 	// Serialize frame to Writer
 	if err = binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil {
 		return