From fecab4058655c14887b4109b508c7fc640b3b563 Mon Sep 17 00:00:00 2001 From: William Chan Date: Tue, 31 May 2011 14:05:35 -0700 Subject: [PATCH] http/spdy: fix data race in header decompression. flate's reader greedily reads from the shared io.Reader in Framer. This leads to a data race on Framer.r. Fix this by providing a corkedReader to zlib.NewReaderDict(). We uncork the reader and allow it to read the number of bytes in the compressed payload. Fixes #1884. R=bradfitz, rsc, go.peter.90 CC=golang-dev https://golang.org/cl/4530089 --- src/pkg/http/spdy/framer.go | 31 ++++++++++++++++++++++++++----- src/pkg/http/spdy/framer_test.go | 12 ------------ 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/pkg/http/spdy/framer.go b/src/pkg/http/spdy/framer.go index 0ad2989d27..8850c71083 100644 --- a/src/pkg/http/spdy/framer.go +++ b/src/pkg/http/spdy/framer.go @@ -44,6 +44,24 @@ func (e FramerError) String() string { return "Error(" + strconv.Itoa(int(e)) + ")" } +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 +} + // Framer handles serializing/deserializing SPDY frames, including compressing/ // decompressing payloads. type Framer struct { @@ -52,6 +70,7 @@ type Framer struct { headerBuf *bytes.Buffer headerCompressor *zlib.Writer r io.Reader + headerReader corkedReader headerDecompressor io.ReadCloser } @@ -74,11 +93,13 @@ func NewFramer(w io.Writer, r io.Reader) (*Framer, os.Error) { return framer, nil } -func (f *Framer) initHeaderDecompression() os.Error { +func (f *Framer) uncorkHeaderDecompressor(payloadSize int) os.Error { if f.headerDecompressor != nil { + f.headerReader.ch <- payloadSize return nil } - decompressor, err := zlib.NewReaderDict(f.r, []byte(HeaderDictionary)) + f.headerReader = corkedReader{r: f.r, ch: make(chan int, 1), n: payloadSize} + decompressor, err := zlib.NewReaderDict(&f.headerReader, []byte(HeaderDictionary)) if err != nil { return err } @@ -171,7 +192,7 @@ func (f *Framer) readSynStreamFrame(h ControlFrameHeader, frame *SynStreamFrame) reader := f.r if !f.headerCompressionDisabled { - f.initHeaderDecompression() + f.uncorkHeaderDecompressor(int(h.length - 10)) reader = f.headerDecompressor } @@ -194,7 +215,7 @@ func (f *Framer) readSynReplyFrame(h ControlFrameHeader, frame *SynReplyFrame) o } reader := f.r if !f.headerCompressionDisabled { - f.initHeaderDecompression() + f.uncorkHeaderDecompressor(int(h.length - 6)) reader = f.headerDecompressor } frame.Headers, err = parseHeaderValueBlock(reader) @@ -216,7 +237,7 @@ func (f *Framer) readHeadersFrame(h ControlFrameHeader, frame *HeadersFrame) os. } reader := f.r if !f.headerCompressionDisabled { - f.initHeaderDecompression() + f.uncorkHeaderDecompressor(int(h.length - 6)) reader = f.headerDecompressor } frame.Headers, err = parseHeaderValueBlock(reader) diff --git a/src/pkg/http/spdy/framer_test.go b/src/pkg/http/spdy/framer_test.go index 192d688fb6..9100e1ea89 100644 --- a/src/pkg/http/spdy/framer_test.go +++ b/src/pkg/http/spdy/framer_test.go @@ -371,12 +371,6 @@ func TestCreateParseDataFrame(t *testing.T) { } func TestCompressionContextAcrossFrames(t *testing.T) { - { - // TODO(willchan,bradfitz): test is temporarily disabled - t.Logf("test temporarily disabled; http://code.google.com/p/go/issues/detail?id=1884") - return - } - buffer := new(bytes.Buffer) framer, err := NewFramer(buffer, buffer) if err != nil { @@ -430,12 +424,6 @@ func TestCompressionContextAcrossFrames(t *testing.T) { } func TestMultipleSPDYFrames(t *testing.T) { - { - // TODO(willchan,bradfitz): test is temporarily disabled - t.Logf("test temporarily disabled; http://code.google.com/p/go/issues/detail?id=1884") - return - } - // Initialize the framers. pr1, pw1 := io.Pipe() pr2, pw2 := io.Pipe()