]> Cypherpunks repositories - gostls13.git/commitdiff
http/spdy: improve error handling.
authorWilliam Chan <willchan@chromium.org>
Tue, 14 Jun 2011 15:31:18 +0000 (11:31 -0400)
committerRuss Cox <rsc@golang.org>
Tue, 14 Jun 2011 15:31:18 +0000 (11:31 -0400)
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
src/pkg/http/spdy/spdy_test.go
src/pkg/http/spdy/types.go
src/pkg/http/spdy/write.go

index 159dbc57802e914860ad4baecb5fa82a0e77b5e3..8adec7bd4ffe1c6493798a44bb68bdc09097e3e3 100644 (file)
@@ -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
        }
index 9100e1ea8978a7da60a63f41c260d4b9a9cb1940..cb91e028613c5be13b57dd9a413df8211b4951c8 100644 (file)
@@ -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)
        }
index 5a665f04ff3a4e51eb1e477348bd75ae4ec82949..41cafb1741f2a3723f88ef93f809d5bab3f6b285 100644 (file)
@@ -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
 }
 
index aa1679f1bd31e349cbe14960ca61b19c5af643c8..7d40bbe9fe2b6a030a1c8003e62947c057b2b5e2 100644 (file)
@@ -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