From 5af8e53a148165bf1b57952d20969e859a39243d Mon Sep 17 00:00:00 2001 From: William Chan Date: Tue, 14 Jun 2011 11:31:18 -0400 Subject: [PATCH] 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 --- src/pkg/http/spdy/read.go | 90 ++++++++++++++++++++++------------ src/pkg/http/spdy/spdy_test.go | 3 +- src/pkg/http/spdy/types.go | 57 +++++++++++---------- src/pkg/http/spdy/write.go | 3 +- 4 files changed, 93 insertions(+), 60 deletions(-) diff --git a/src/pkg/http/spdy/read.go b/src/pkg/http/spdy/read.go index 159dbc57802..8adec7bd4ff 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 9100e1ea897..cb91e028613 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 5a665f04ff3..41cafb1741f 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 aa1679f1bd3..7d40bbe9fe2 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