diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 0b08eed..27debbb 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -13,6 +13,21 @@ env: GOTOOLCHAIN: local jobs: + lint: + name: Golang-CI Lint + timeout-minutes: 10 + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v6 + - name: Install Go + uses: actions/setup-go@v6 + with: + go-version: stable + - name: Install golangci-lint + uses: golangci/golangci-lint-action@v9 + with: + args: --verbose --show-stats test: name: Unit test timeout-minutes: 10 diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..b5e3cb1 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,65 @@ +version: "2" + +linters: + enable: + - bodyclose + - canonicalheader + - errcheck + - errorlint + - forcetypeassert + - gocritic + - godoclint + - nakedret + - perfsprint + - staticcheck + - thelper + - unconvert + - unparam + - whitespace + exclusions: + generated: disable + presets: + - comments + - std-error-handling + rules: + - path: spdy/ + linters: + - nakedret + - nonamedreturns + - path: connection.go + linters: + - unparam + text: "result 0 \\(error\\) is always nil" # TODO(thaJeztah): check if errors should be returned + settings: + revive: + rules: + # https://github.com/mgechev/revive/blob/HEAD/RULES_DESCRIPTIONS.md#var-naming + - name: var-naming + severity: warning + disabled: false + exclude: [""] + arguments: + - ["Id"] # AllowList + - [] # DenyList + - - skip-initialism-name-checks: true + upper-case-const: true + staticcheck: + # Enable all options, with some exceptions. + # For defaults, see https://golangci-lint.run/usage/linters/#staticcheck + checks: + - all + - -QF1008 # Omit embedded fields from selector expression; https://staticcheck.dev/docs/checks/#QF1008 + - -ST1003 # Poorly chosen identifier; https://staticcheck.dev/docs/checks/#ST1003 + +formatters: + enable: + - gofumpt + exclusions: + generated: disable + +issues: + # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + max-issues-per-linter: 0 + + # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. + max-same-issues: 0 diff --git a/connection.go b/connection.go index 1394d0a..b268887 100644 --- a/connection.go +++ b/connection.go @@ -29,10 +29,10 @@ import ( ) var ( - ErrInvalidStreamId = errors.New("Invalid stream id") - ErrTimeout = errors.New("Timeout occurred") - ErrReset = errors.New("Stream reset") - ErrWriteClosedStream = errors.New("Write on closed stream") + ErrInvalidStreamId = errors.New("invalid stream id") // ErrInvalidStreamId is returned when an operation refers to a stream with an invalid or unknown stream ID. + ErrTimeout = errors.New("timeout occurred") // ErrTimeout is returned when a stream or connection operation exceeds its configured timeout. + ErrReset = errors.New("stream reset") // ErrReset is returned when a stream is reset by the peer or by the local endpoint. + ErrWriteClosedStream = errors.New("write on closed stream") // ErrWriteClosedStream is returned when attempting to write to a stream that has already been closed. ) const ( @@ -101,9 +101,9 @@ Loop: i.conn.streamCond.L.Unlock() go func() { for _, stream := range streams { - stream.resetStream() + _ = stream.resetStream() } - i.conn.Close() + _ = i.conn.Close() }() case <-i.conn.closeChan: if timer != nil { @@ -277,9 +277,9 @@ func (s *Connection) Ping() (time.Duration, error) { pid := s.pingId s.pingLock.Lock() if s.pingId > 0x7ffffffe { - s.pingId = s.pingId - 0x7ffffffe + s.pingId -= 0x7ffffffe } else { - s.pingId = s.pingId + 2 + s.pingId += 2 } pingChan := make(chan error) s.pingChans[pid] = pingChan @@ -309,14 +309,14 @@ func (s *Connection) Ping() (time.Duration, error) { } // Serve handles frames sent from the server, including reply frames -// which are needed to fully initiate connections. Both clients and servers +// which are needed to fully initiate connections. Both clients and servers // should call Serve in a separate goroutine before creating streams. func (s *Connection) Serve(newHandler StreamHandler) { // use a WaitGroup to wait for all frames to be drained after receiving // go-away. var wg sync.WaitGroup - // Parition queues to ensure stream frames are handled + // Partition queues to ensure stream frames are handled // by the same worker, ensuring order is maintained frameQueues := make([]*PriorityFrameQueue, FRAME_WORKERS) for i := 0; i < FRAME_WORKERS; i++ { @@ -345,7 +345,7 @@ Loop: for { readFrame, err := s.framer.ReadFrame() if err != nil { - if err != io.EOF { + if !errors.Is(err, io.EOF) { debugMessage("frame read error: %s", err) } else { debugMessage("(%p) EOF received", s) @@ -399,7 +399,7 @@ Loop: wg.Wait() if goAwayFrame != nil { - s.handleGoAwayFrame(goAwayFrame) + _ = s.handleGoAwayFrame(goAwayFrame) } // now it's safe to close remote channels and empty s.streams @@ -506,7 +506,7 @@ func (s *Connection) checkStreamFrame(frame *spdy.SynStreamFrame) bool { func (s *Connection) handleStreamFrame(frame *spdy.SynStreamFrame, newHandler StreamHandler) error { stream, ok := s.getStream(frame.StreamId) if !ok { - return fmt.Errorf("Missing stream: %d", frame.StreamId) + return fmt.Errorf("missing stream: %d", frame.StreamId) } newHandler(stream) @@ -666,9 +666,9 @@ func (s *Connection) remoteStreamFinish(stream *Stream) { } // CreateStream creates a new spdy stream using the parameters for -// creating the stream frame. The stream frame will be sent upon +// creating the stream frame. The stream frame will be sent upon // calling this function, however this function does not wait for -// the reply frame. If waiting for the reply is desired, use +// the reply frame. If waiting for the reply is desired, use // the stream Wait or WaitTimeout function on the stream returned // by this function. func (s *Connection) CreateStream(headers http.Header, parent *Stream, fin bool) (*Stream, error) { @@ -679,7 +679,7 @@ func (s *Connection) CreateStream(headers http.Header, parent *Stream, fin bool) streamId := s.getNextStreamId() if streamId == 0 { - return nil, fmt.Errorf("Unable to get new stream id") + return nil, errors.New("unable to get new stream id") } stream := &Stream{ @@ -762,7 +762,8 @@ func (s *Connection) shutdown(closeTimeout time.Duration) { close(s.shutdownChan) } -// Closes spdy connection by sending GoAway frame and initiating shutdown +// Close closes the spdy connection by sending a [spdy.GoAwayFrame] frame +// with status [spdy.GoAwayOK] and initiating shutdown. func (s *Connection) Close() error { s.receiveIdLock.Lock() if s.goneAway { @@ -792,7 +793,7 @@ func (s *Connection) Close() error { } // CloseWait closes the connection and waits for shutdown -// to finish. Note the underlying network Connection +// to finish. Note the underlying network Connection // is not closed until the end of shutdown. func (s *Connection) CloseWait() error { closeErr := s.Close() @@ -807,11 +808,11 @@ func (s *Connection) CloseWait() error { } // Wait waits for the connection to finish shutdown or for -// the wait timeout duration to expire. This needs to be -// called either after Close has been called or the GOAWAYFRAME -// has been received. If the wait timeout is 0, this function -// will block until shutdown finishes. If wait is never called -// and a shutdown error occurs, that error will be logged as an +// the wait timeout duration to expire. This needs to be +// called either after Close has been called or the GOAWAY frame +// has been received. If the wait timeout is 0, this function +// blocks until shutdown finishes. If wait is never called +// and a shutdown error occurs, that error is logged as an // unhandled error. func (s *Connection) Wait(waitTimeout time.Duration) error { var timeout <-chan time.Time @@ -833,9 +834,9 @@ func (s *Connection) Wait(waitTimeout time.Duration) error { } // NotifyClose registers a channel to be called when the remote -// peer inidicates connection closure. The last stream to be -// received by the remote will be sent on the channel. The notify -// timeout will determine the duration between go away received +// peer indicates connection closure. The last stream to be +// received by the remote will be sent on the channel. The notify +// timeout determines the duration between go away received // and the connection being closed. func (s *Connection) NotifyClose(c chan<- *Stream, timeout time.Duration) { s.goAwayTimeout = timeout @@ -844,7 +845,7 @@ func (s *Connection) NotifyClose(c chan<- *Stream, timeout time.Duration) { // SetCloseTimeout sets the amount of time close will wait for // streams to finish before terminating the underlying network -// connection. Setting the timeout to 0 will cause close to +// connection. Setting the timeout to 0 will cause close to // wait forever, which is the default. func (s *Connection) SetCloseTimeout(timeout time.Duration) { s.closeTimeout = timeout @@ -912,8 +913,8 @@ func (s *Connection) sendStream(stream *Stream, fin bool) error { } streamFrame := &spdy.SynStreamFrame{ - StreamId: spdy.StreamId(stream.streamId), - AssociatedToStreamId: spdy.StreamId(parentId), + StreamId: stream.streamId, + AssociatedToStreamId: parentId, Headers: stream.headers, CFHeader: spdy.ControlFrameHeader{Flags: flags}, } @@ -928,7 +929,7 @@ func (s *Connection) getNextStreamId() spdy.StreamId { if sid > 0x7fffffff { return 0 } - s.nextStreamId = s.nextStreamId + 2 + s.nextStreamId += 2 return sid } @@ -966,7 +967,7 @@ func (s *Connection) getStream(streamId spdy.StreamId) (stream *Stream, ok bool) s.streamLock.RLock() stream, ok = s.streams[streamId] s.streamLock.RUnlock() - return + return stream, ok } // FindStream looks up the given stream id and either waits for the diff --git a/handlers.go b/handlers.go index d68f61f..5b2ea44 100644 --- a/handlers.go +++ b/handlers.go @@ -29,8 +29,8 @@ func MirrorStreamHandler(stream *Stream) { } go func() { - io.Copy(stream, stream) - stream.Close() + _, _ = io.Copy(stream, stream) + _ = stream.Close() }() go func() { for { @@ -46,7 +46,7 @@ func MirrorStreamHandler(stream *Stream) { }() } -// NoopStreamHandler does nothing when stream connects. +// NoOpStreamHandler does nothing when stream connects. func NoOpStreamHandler(stream *Stream) { - stream.SendReply(http.Header{}, false) + _ = stream.SendReply(http.Header{}, false) } diff --git a/priority.go b/priority.go index d8eb351..5af2825 100644 --- a/priority.go +++ b/priority.go @@ -47,7 +47,7 @@ func (fq frameQueue) Swap(i, j int) { } func (fq *frameQueue) Push(x interface{}) { - *fq = append(*fq, x.(*prioritizedFrame)) + *fq = append(*fq, x.(*prioritizedFrame)) //nolint:forcetypeassert } func (fq *frameQueue) Pop() interface{} { @@ -87,7 +87,7 @@ func (q *PriorityFrameQueue) Push(frame spdy.Frame, priority uint8) { priority: priority, insertId: q.nextInsertId, } - q.nextInsertId = q.nextInsertId + 1 + q.nextInsertId++ heap.Push(q.queue, pFrame) q.c.Signal() } @@ -101,7 +101,7 @@ func (q *PriorityFrameQueue) Pop() spdy.Frame { } q.c.Wait() } - frame := heap.Pop(q.queue).(*prioritizedFrame).frame + frame := heap.Pop(q.queue).(*prioritizedFrame).frame //nolint:forcetypeassert q.c.Signal() return frame } diff --git a/priority_test.go b/priority_test.go index ead4b6a..51ebce0 100644 --- a/priority_test.go +++ b/priority_test.go @@ -62,9 +62,9 @@ func TestPriorityQueueOrdering(t *testing.T) { } for i := spdy.StreamId(0); i < 150; i++ { - frame := queue.Pop() - if frame.(*spdy.DataFrame).StreamId != i { - t.Fatalf("Wrong frame\nActual: %d\nExpecting: %d", frame.(*spdy.DataFrame).StreamId, i) + frame := queue.Pop().(*spdy.DataFrame) //nolint:forcetypeassert + if frame.StreamId != i { + t.Fatalf("Wrong frame\nActual: %d\nExpecting: %d", frame.StreamId, i) } } } @@ -85,9 +85,9 @@ func TestPriorityQueueSync(t *testing.T) { wg.Wait() for i := spdy.StreamId(0); i < 150; i++ { - frame := queue.Pop() - if frame.(*spdy.DataFrame).StreamId != i { - t.Fatalf("Wrong frame\nActual: %d\nExpecting: %d", frame.(*spdy.DataFrame).StreamId, i) + frame := queue.Pop().(*spdy.DataFrame) //nolint:forcetypeassert + if frame.StreamId != i { + t.Fatalf("Wrong frame\nActual: %d\nExpecting: %d", frame.StreamId, i) } } } diff --git a/spdy/read.go b/spdy/read.go index 9359a95..09c6e7e 100644 --- a/spdy/read.go +++ b/spdy/read.go @@ -7,6 +7,7 @@ package spdy import ( "compress/zlib" "encoding/binary" + "errors" "io" "net/http" "strings" @@ -134,7 +135,7 @@ func (f *Framer) uncorkHeaderDecompressor(payloadSize int64) error { return nil } f.headerReader = io.LimitedReader{R: f.r, N: payloadSize} - decompressor, err := zlib.NewReaderDict(&f.headerReader, []byte(headerDictionary)) + decompressor, err := zlib.NewReaderDict(&f.headerReader, headerDictionary) if err != nil { return err } @@ -241,7 +242,7 @@ func (f *Framer) readSynStreamFrame(h ControlFrameHeader, frame *SynStreamFrame) reader = f.headerDecompressor } frame.Headers, err = parseHeaderValueBlock(reader, frame.StreamId) - if !f.headerCompressionDisabled && (err == io.EOF && f.headerReader.N == 0 || f.headerReader.N != 0) { + if !f.headerCompressionDisabled && ((errors.Is(err, io.EOF) && f.headerReader.N == 0) || f.headerReader.N != 0) { err = &Error{WrongCompressedPayloadSize, 0} } if err != nil { @@ -273,7 +274,7 @@ func (f *Framer) readSynReplyFrame(h ControlFrameHeader, frame *SynReplyFrame) e reader = f.headerDecompressor } frame.Headers, err = parseHeaderValueBlock(reader, frame.StreamId) - if !f.headerCompressionDisabled && (err == io.EOF && f.headerReader.N == 0 || f.headerReader.N != 0) { + if !f.headerCompressionDisabled && ((errors.Is(err, io.EOF) && f.headerReader.N == 0) || f.headerReader.N != 0) { err = &Error{WrongCompressedPayloadSize, 0} } if err != nil { @@ -305,7 +306,7 @@ func (f *Framer) readHeadersFrame(h ControlFrameHeader, frame *HeadersFrame) err reader = f.headerDecompressor } frame.Headers, err = parseHeaderValueBlock(reader, frame.StreamId) - if !f.headerCompressionDisabled && (err == io.EOF && f.headerReader.N == 0 || f.headerReader.N != 0) { + if !f.headerCompressionDisabled && ((errors.Is(err, io.EOF) && f.headerReader.N == 0) || f.headerReader.N != 0) { err = &Error{WrongCompressedPayloadSize, 0} } if err != nil { diff --git a/spdy/spdy_test.go b/spdy/spdy_test.go index 073b049..7815dc5 100644 --- a/spdy/spdy_test.go +++ b/spdy/spdy_test.go @@ -10,6 +10,7 @@ import ( "bytes" "compress/zlib" "encoding/base64" + "errors" "io" "io/ioutil" "net/http" @@ -25,7 +26,7 @@ var HeadersFixture = http.Header{ func TestHeaderParsing(t *testing.T) { var headerValueBlockBuf bytes.Buffer - writeHeaderValueBlock(&headerValueBlockBuf, HeadersFixture) + _, _ = writeHeaderValueBlock(&headerValueBlockBuf, HeadersFixture) const bogusStreamId = 1 newHeaders, err := parseHeaderValueBlock(&headerValueBlockBuf, bogusStreamId) if err != nil { @@ -575,7 +576,7 @@ func TestReadMalformedZlibHeader(t *testing.T) { t.Fatalf("NewFramer: %v", err) } _, err = reader.ReadFrame() - if err != zlib.ErrHeader { + if !errors.Is(err, zlib.ErrHeader) { t.Errorf("Frame %s, expected: %#v, actual: %#v", name, zlib.ErrHeader, err) } } @@ -637,11 +638,13 @@ func TestNoZeroStreamId(t *testing.T) { } func checkZeroStreamId(t *testing.T, frame string, method string, err error) { + t.Helper() if err == nil { t.Errorf("%s ZeroStreamId, no error on %s", method, frame) return } - eerr, ok := err.(*Error) + eerr := &Error{} + ok := errors.As(err, &eerr) if !ok || eerr.Err != ZeroStreamId { t.Errorf("%s ZeroStreamId, incorrect error %#v, frame %s", method, eerr, frame) } diff --git a/spdy/types.go b/spdy/types.go index 74d3d0d..8579991 100644 --- a/spdy/types.go +++ b/spdy/types.go @@ -51,7 +51,7 @@ const ( // MaxDataLength is the maximum number of bytes that can be stored in one frame. const MaxDataLength = 1<<24 - 1 -// headerValueSepator separates multiple header values. +// headerValueSeparator separates multiple header values. const headerValueSeparator = "\x00" // Frame is a single SPDY frame in its unpacked in-memory representation. Use @@ -129,7 +129,7 @@ const ( FlagSettingsPersisted SettingsFlag = 0x2 ) -// SettingsFlag represents the id of an id/value pair in a SETTINGS frame. +// SettingsId represents the id of an id/value pair in a SETTINGS frame. type SettingsId uint32 const ( @@ -205,7 +205,7 @@ type DataFrame struct { Data []byte // payload data of this frame } -// A SPDY specific error. +// ErrorCode represents a SPDY specific error. type ErrorCode string const ( @@ -263,7 +263,7 @@ type Framer struct { // buffered implementation to optimize performance. func NewFramer(w io.Writer, r io.Reader) (*Framer, error) { compressBuf := new(bytes.Buffer) - compressor, err := zlib.NewWriterLevelDict(compressBuf, zlib.BestCompression, []byte(headerDictionary)) + compressor, err := zlib.NewWriterLevelDict(compressBuf, zlib.BestCompression, headerDictionary) if err != nil { return nil, err } diff --git a/spdy/write.go b/spdy/write.go index b212f66..586fa2c 100644 --- a/spdy/write.go +++ b/spdy/write.go @@ -19,7 +19,7 @@ func (frame *SynReplyFrame) write(f *Framer) error { return f.writeSynReplyFrame(frame) } -func (frame *RstStreamFrame) write(f *Framer) (err error) { +func (frame *RstStreamFrame) write(f *Framer) error { if frame.StreamId == 0 { return &Error{ZeroStreamId, 0} } @@ -29,46 +29,46 @@ func (frame *RstStreamFrame) write(f *Framer) (err error) { frame.CFHeader.length = 8 // Serialize frame to Writer. - if err = writeControlFrameHeader(f.w, frame.CFHeader); err != nil { - return + if err := writeControlFrameHeader(f.w, frame.CFHeader); err != nil { + return err } - if err = binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil { + return err } if frame.Status == 0 { return &Error{InvalidControlFrame, frame.StreamId} } - if err = binary.Write(f.w, binary.BigEndian, frame.Status); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, frame.Status); err != nil { + return err } - return + return nil } -func (frame *SettingsFrame) write(f *Framer) (err error) { +func (frame *SettingsFrame) write(f *Framer) error { frame.CFHeader.version = Version frame.CFHeader.frameType = TypeSettings frame.CFHeader.length = uint32(len(frame.FlagIdValues)*8 + 4) // Serialize frame to Writer. - if err = writeControlFrameHeader(f.w, frame.CFHeader); err != nil { - return + if err := writeControlFrameHeader(f.w, frame.CFHeader); err != nil { + return err } - if err = binary.Write(f.w, binary.BigEndian, uint32(len(frame.FlagIdValues))); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, uint32(len(frame.FlagIdValues))); err != nil { + return err } for _, flagIdValue := range frame.FlagIdValues { flagId := uint32(flagIdValue.Flag)<<24 | uint32(flagIdValue.Id) - if err = binary.Write(f.w, binary.BigEndian, flagId); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, flagId); err != nil { + return err } - if err = binary.Write(f.w, binary.BigEndian, flagIdValue.Value); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, flagIdValue.Value); err != nil { + return err } } - return + return nil } -func (frame *PingFrame) write(f *Framer) (err error) { +func (frame *PingFrame) write(f *Framer) error { if frame.Id == 0 { return &Error{ZeroStreamId, 0} } @@ -78,30 +78,30 @@ func (frame *PingFrame) write(f *Framer) (err error) { frame.CFHeader.length = 4 // Serialize frame to Writer. - if err = writeControlFrameHeader(f.w, frame.CFHeader); err != nil { - return + if err := writeControlFrameHeader(f.w, frame.CFHeader); err != nil { + return err } - if err = binary.Write(f.w, binary.BigEndian, frame.Id); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, frame.Id); err != nil { + return err } - return + return nil } -func (frame *GoAwayFrame) write(f *Framer) (err error) { +func (frame *GoAwayFrame) write(f *Framer) error { frame.CFHeader.version = Version frame.CFHeader.frameType = TypeGoAway frame.CFHeader.Flags = 0 frame.CFHeader.length = 8 // Serialize frame to Writer. - if err = writeControlFrameHeader(f.w, frame.CFHeader); err != nil { - return + if err := writeControlFrameHeader(f.w, frame.CFHeader); err != nil { + return err } - if err = binary.Write(f.w, binary.BigEndian, frame.LastGoodStreamId); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, frame.LastGoodStreamId); err != nil { + return err } - if err = binary.Write(f.w, binary.BigEndian, frame.Status); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, frame.Status); err != nil { + return err } return nil } @@ -110,21 +110,21 @@ func (frame *HeadersFrame) write(f *Framer) error { return f.writeHeadersFrame(frame) } -func (frame *WindowUpdateFrame) write(f *Framer) (err error) { +func (frame *WindowUpdateFrame) write(f *Framer) error { frame.CFHeader.version = Version frame.CFHeader.frameType = TypeWindowUpdate frame.CFHeader.Flags = 0 frame.CFHeader.length = 8 // Serialize frame to Writer. - if err = writeControlFrameHeader(f.w, frame.CFHeader); err != nil { - return + if err := writeControlFrameHeader(f.w, frame.CFHeader); err != nil { + return err } - if err = binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil { + return err } - if err = binary.Write(f.w, binary.BigEndian, frame.DeltaWindowSize); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, frame.DeltaWindowSize); err != nil { + return err } return nil } @@ -152,36 +152,37 @@ func writeControlFrameHeader(w io.Writer, h ControlFrameHeader) error { return nil } -func writeHeaderValueBlock(w io.Writer, h http.Header) (n int, err error) { - n = 0 - if err = binary.Write(w, binary.BigEndian, uint32(len(h))); err != nil { - return +//nolint:unparam // result n is never used +func writeHeaderValueBlock(w io.Writer, h http.Header) (int, error) { + n := 0 + if err := binary.Write(w, binary.BigEndian, uint32(len(h))); err != nil { + return n, err } n += 2 for name, values := range h { - if err = binary.Write(w, binary.BigEndian, uint32(len(name))); err != nil { - return + if err := binary.Write(w, binary.BigEndian, uint32(len(name))); err != nil { + return n, err } n += 2 name = strings.ToLower(name) - if _, err = io.WriteString(w, name); err != nil { - return + if _, err := io.WriteString(w, name); err != nil { + return n, err } n += len(name) v := strings.Join(values, headerValueSeparator) - if err = binary.Write(w, binary.BigEndian, uint32(len(v))); err != nil { - return + if err := binary.Write(w, binary.BigEndian, uint32(len(v))); err != nil { + return n, err } n += 2 - if _, err = io.WriteString(w, v); err != nil { - return + if _, err := io.WriteString(w, v); err != nil { + return n, err } n += len(v) } - return + return n, nil } -func (f *Framer) writeSynStreamFrame(frame *SynStreamFrame) (err error) { +func (f *Framer) writeSynStreamFrame(frame *SynStreamFrame) error { if frame.StreamId == 0 { return &Error{ZeroStreamId, 0} } @@ -190,11 +191,11 @@ func (f *Framer) writeSynStreamFrame(frame *SynStreamFrame) (err error) { if !f.headerCompressionDisabled { writer = f.headerCompressor } - if _, err = writeHeaderValueBlock(writer, frame.Headers); err != nil { - return + if _, err := writeHeaderValueBlock(writer, frame.Headers); err != nil { + return err } if !f.headerCompressionDisabled { - f.headerCompressor.Flush() + _ = f.headerCompressor.Flush() } // Set ControlFrameHeader. @@ -203,29 +204,29 @@ func (f *Framer) writeSynStreamFrame(frame *SynStreamFrame) (err error) { frame.CFHeader.length = uint32(len(f.headerBuf.Bytes()) + 10) // Serialize frame to Writer. - if err = writeControlFrameHeader(f.w, frame.CFHeader); err != nil { + if err := writeControlFrameHeader(f.w, frame.CFHeader); err != nil { return err } - if err = binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil { + if err := binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil { return err } - if err = binary.Write(f.w, binary.BigEndian, frame.AssociatedToStreamId); err != nil { + if err := binary.Write(f.w, binary.BigEndian, frame.AssociatedToStreamId); err != nil { return err } - if err = binary.Write(f.w, binary.BigEndian, frame.Priority<<5); err != nil { + if err := binary.Write(f.w, binary.BigEndian, frame.Priority<<5); err != nil { return err } - if err = binary.Write(f.w, binary.BigEndian, frame.Slot); err != nil { + if err := binary.Write(f.w, binary.BigEndian, frame.Slot); err != nil { return err } - if _, err = f.w.Write(f.headerBuf.Bytes()); err != nil { + if _, err := f.w.Write(f.headerBuf.Bytes()); err != nil { return err } f.headerBuf.Reset() return nil } -func (f *Framer) writeSynReplyFrame(frame *SynReplyFrame) (err error) { +func (f *Framer) writeSynReplyFrame(frame *SynReplyFrame) error { if frame.StreamId == 0 { return &Error{ZeroStreamId, 0} } @@ -234,11 +235,11 @@ func (f *Framer) writeSynReplyFrame(frame *SynReplyFrame) (err error) { if !f.headerCompressionDisabled { writer = f.headerCompressor } - if _, err = writeHeaderValueBlock(writer, frame.Headers); err != nil { - return + if _, err := writeHeaderValueBlock(writer, frame.Headers); err != nil { + return err } if !f.headerCompressionDisabled { - f.headerCompressor.Flush() + _ = f.headerCompressor.Flush() } // Set ControlFrameHeader. @@ -247,20 +248,20 @@ func (f *Framer) writeSynReplyFrame(frame *SynReplyFrame) (err error) { frame.CFHeader.length = uint32(len(f.headerBuf.Bytes()) + 4) // Serialize frame to Writer. - if err = writeControlFrameHeader(f.w, frame.CFHeader); err != nil { - return + if err := writeControlFrameHeader(f.w, frame.CFHeader); err != nil { + return err } - if err = binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil { + return err } - if _, err = f.w.Write(f.headerBuf.Bytes()); err != nil { - return + if _, err := f.w.Write(f.headerBuf.Bytes()); err != nil { + return err } f.headerBuf.Reset() - return + return nil } -func (f *Framer) writeHeadersFrame(frame *HeadersFrame) (err error) { +func (f *Framer) writeHeadersFrame(frame *HeadersFrame) error { if frame.StreamId == 0 { return &Error{ZeroStreamId, 0} } @@ -269,11 +270,11 @@ func (f *Framer) writeHeadersFrame(frame *HeadersFrame) (err error) { if !f.headerCompressionDisabled { writer = f.headerCompressor } - if _, err = writeHeaderValueBlock(writer, frame.Headers); err != nil { - return + if _, err := writeHeaderValueBlock(writer, frame.Headers); err != nil { + return err } if !f.headerCompressionDisabled { - f.headerCompressor.Flush() + _ = f.headerCompressor.Flush() } // Set ControlFrameHeader. @@ -282,20 +283,20 @@ func (f *Framer) writeHeadersFrame(frame *HeadersFrame) (err error) { frame.CFHeader.length = uint32(len(f.headerBuf.Bytes()) + 4) // Serialize frame to Writer. - if err = writeControlFrameHeader(f.w, frame.CFHeader); err != nil { - return + if err := writeControlFrameHeader(f.w, frame.CFHeader); err != nil { + return err } - if err = binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil { + return err } - if _, err = f.w.Write(f.headerBuf.Bytes()); err != nil { - return + if _, err := f.w.Write(f.headerBuf.Bytes()); err != nil { + return err } f.headerBuf.Reset() - return + return nil } -func (f *Framer) writeDataFrame(frame *DataFrame) (err error) { +func (f *Framer) writeDataFrame(frame *DataFrame) error { if frame.StreamId == 0 { return &Error{ZeroStreamId, 0} } @@ -304,15 +305,15 @@ func (f *Framer) writeDataFrame(frame *DataFrame) (err error) { } // Serialize frame to Writer. - if err = binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, frame.StreamId); err != nil { + return err } flagsAndLength := uint32(frame.Flags)<<24 | uint32(len(frame.Data)) - if err = binary.Write(f.w, binary.BigEndian, flagsAndLength); err != nil { - return + if err := binary.Write(f.w, binary.BigEndian, flagsAndLength); err != nil { + return err } - if _, err = f.w.Write(frame.Data); err != nil { - return + if _, err := f.w.Write(frame.Data); err != nil { + return err } return nil } diff --git a/spdy_bench_test.go b/spdy_bench_test.go index 4bf7bf7..c332fac 100644 --- a/spdy_bench_test.go +++ b/spdy_bench_test.go @@ -41,7 +41,7 @@ func BenchmarkDial10000(b *testing.B) { server, addr, wg := configureServer() defer func() { - server.Close() + _ = server.Close() wg.Wait() }() @@ -50,7 +50,7 @@ func BenchmarkDial10000(b *testing.B) { if dialErr != nil { panic(fmt.Sprintf("Error dialing server: %s", dialErr)) } - conn.Close() + _ = conn.Close() } } @@ -58,7 +58,7 @@ func BenchmarkDialWithSPDYStream10000(b *testing.B) { server, addr, wg := configureServer() defer func() { - server.Close() + _ = server.Close() wg.Wait() }() @@ -81,11 +81,12 @@ func BenchmarkDialWithSPDYStream10000(b *testing.B) { } } -func benchmarkStreamWithDataAndSize(size uint64, b *testing.B) { +func benchmarkStreamWithDataAndSize(b *testing.B, size uint64) { + b.Helper() server, addr, wg := configureServer() defer func() { - server.Close() + _ = server.Close() wg.Wait() }() @@ -103,19 +104,17 @@ func benchmarkStreamWithDataAndSize(size uint64, b *testing.B) { go spdyConn.Serve(MirrorStreamHandler) stream, err := spdyConn.CreateStream(http.Header{}, nil, false) - - writer := make([]byte, size) - - stream.Write(writer) - if err != nil { panic(err) } - reader := make([]byte, size) - stream.Read(reader) + writer := make([]byte, size) - stream.Close() + _, _ = stream.Write(writer) + + reader := make([]byte, size) + _, _ = stream.Read(reader) + _ = stream.Close() closeErr := spdyConn.Close() if closeErr != nil { @@ -124,6 +123,6 @@ func benchmarkStreamWithDataAndSize(size uint64, b *testing.B) { } } -func BenchmarkStreamWith1Byte10000(b *testing.B) { benchmarkStreamWithDataAndSize(1, b) } -func BenchmarkStreamWith1KiloByte10000(b *testing.B) { benchmarkStreamWithDataAndSize(1024, b) } -func BenchmarkStreamWith1Megabyte10000(b *testing.B) { benchmarkStreamWithDataAndSize(1024*1024, b) } +func BenchmarkStreamWith1Byte10000(b *testing.B) { benchmarkStreamWithDataAndSize(b, 1) } +func BenchmarkStreamWith1KiloByte10000(b *testing.B) { benchmarkStreamWithDataAndSize(b, 1024) } +func BenchmarkStreamWith1Megabyte10000(b *testing.B) { benchmarkStreamWithDataAndSize(b, 1024*1024) } diff --git a/spdy_test.go b/spdy_test.go index 598604e..1a8577d 100644 --- a/spdy_test.go +++ b/spdy_test.go @@ -19,6 +19,7 @@ package spdystream import ( "bufio" "bytes" + "errors" "fmt" "io" "io/ioutil" @@ -80,7 +81,7 @@ func TestSpdyStreams(t *testing.T) { } headers := http.Header{ - "TestKey": []string{"TestVal"}, + "Test-Key": []string{"TestVal"}, } sendErr := stream.SendHeader(headers, false) if sendErr != nil { @@ -93,7 +94,7 @@ func TestSpdyStreams(t *testing.T) { if len(receiveHeaders) != 1 { t.Fatalf("Unexpected number of headers:\nActual: %d\nExpecting:%d", len(receiveHeaders), 1) } - testVal := receiveHeaders.Get("TestKey") + testVal := receiveHeaders.Get("Test-Key") if testVal != "TestVal" { t.Fatalf("Wrong test value:\nActual: %q\nExpecting: %q", testVal, "TestVal") } @@ -126,7 +127,7 @@ func TestSpdyStreams(t *testing.T) { } n, readErr = stream.Read(buf) - if readErr != io.EOF { + if !errors.Is(readErr, io.EOF) { t.Fatalf("Expected EOF reading from finished stream, read %d bytes", n) } @@ -135,7 +136,7 @@ func TestSpdyStreams(t *testing.T) { if streamCloseErr == nil { t.Fatalf("No error closing finished stream") } - if streamCloseErr != ErrWriteClosedStream { + if !errors.Is(streamCloseErr, ErrWriteClosedStream) { t.Fatalf("Unexpected error closing stream: %s", streamCloseErr) } @@ -154,7 +155,7 @@ func TestSpdyStreams(t *testing.T) { if waitErr == nil { t.Fatalf("Did not receive error creating stream") } - if waitErr != ErrReset { + if !errors.Is(waitErr, ErrReset) { t.Fatalf("Unexpected error creating stream: %s", waitErr) } streamCloseErr = badStream.Close() @@ -296,7 +297,7 @@ func TestUnexpectedRemoteConnectionClosed(t *testing.T) { serverSpdyConn, _ := NewConnection(serverConn, true) go serverSpdyConn.Serve(func(stream *Stream) { - stream.SendReply(http.Header{}, tc.closeSender) + _ = stream.SendReply(http.Header{}, tc.closeSender) }) }() @@ -324,7 +325,7 @@ func TestUnexpectedRemoteConnectionClosed(t *testing.T) { if tc.closeReceiver { // make stream half closed, receive only - stream.Close() + _ = stream.Close() } streamch := make(chan error, 1) @@ -340,7 +341,7 @@ func TestUnexpectedRemoteConnectionClosed(t *testing.T) { } e := <-streamch - if e == nil || e != io.EOF { + if e == nil || !errors.Is(e, io.EOF) { t.Fatalf("(%d) Expected to get an EOF stream error", tix) } @@ -436,7 +437,7 @@ func TestIdleShutdownRace(t *testing.T) { spdyConn.SetIdleTimeout(5 * time.Millisecond) go func() { time.Sleep(5 * time.Millisecond) - stream.Reset() + _ = stream.Reset() }() select { @@ -662,7 +663,7 @@ func TestHalfClosedIdleTimeout(t *testing.T) { t.Errorf("Error creating server connection: %v", err) } go serverSpdyConn.Serve(func(s *Stream) { - s.SendReply(http.Header{}, true) + _ = s.SendReply(http.Header{}, true) }) serverSpdyConn.SetIdleTimeout(10 * time.Millisecond) }() @@ -685,7 +686,7 @@ func TestHalfClosedIdleTimeout(t *testing.T) { time.Sleep(20 * time.Millisecond) - stream.Reset() + _ = stream.Reset() err = spdyConn.Close() if err != nil { @@ -824,20 +825,20 @@ func TestFramingAfterRemoteConnectionClosed(t *testing.T) { w.WriteHeader(http.StatusSwitchingProtocols) - netconn, _, _ := w.(http.Hijacker).Hijack() + netconn, _, _ := w.(http.Hijacker).Hijack() //nolint:forcetypeassert conn, _ := NewConnection(netconn, true) go conn.Serve(func(s *Stream) { - s.SendReply(http.Header{}, false) + _ = s.SendReply(http.Header{}, false) streamCh <- s }) stream := <-streamCh - io.Copy(stream, stream) + _, _ = io.Copy(stream, stream) closeChan := make(chan struct{}) go func() { - stream.Reset() - conn.Close() + _ = stream.Reset() + _ = conn.Close() close(closeChan) }() @@ -847,7 +848,7 @@ func TestFramingAfterRemoteConnectionClosed(t *testing.T) { server.Start() defer server.Close() - req, err := http.NewRequest("GET", server.URL, nil) + req, err := http.NewRequest(http.MethodGet, server.URL, nil) if err != nil { t.Fatalf("Error creating request: %s", err) } @@ -855,10 +856,11 @@ func TestFramingAfterRemoteConnectionClosed(t *testing.T) { rt := &roundTripper{} client := &http.Client{Transport: rt} - _, err = client.Do(req) + r, err := client.Do(req) if err != nil { t.Fatalf("unexpected error from client.Do: %s", err) } + defer func() { _ = r.Body.Close() }() conn, err := NewConnection(rt.conn, false) if err != nil { @@ -891,8 +893,8 @@ func TestFramingAfterRemoteConnectionClosed(t *testing.T) { t.Fatalf("expected '%s', got '%s'", e, a) } - stream.Reset() - conn.Close() + _ = stream.Reset() + _ = conn.Close() } func TestGoAwayRace(t *testing.T) { @@ -931,7 +933,7 @@ func TestGoAwayRace(t *testing.T) { streamCh := make(chan *Stream) go serverSpdyConn.Serve(func(s *Stream) { - s.SendReply(http.Header{}, false) + _ = s.SendReply(http.Header{}, false) streamCh <- s }) @@ -939,7 +941,7 @@ func TestGoAwayRace(t *testing.T) { if !ok { t.Errorf("didn't get a stream") } - stream.Close() + _ = stream.Close() data, err := ioutil.ReadAll(stream) if err != nil { t.Error(err) @@ -967,14 +969,14 @@ func TestGoAwayRace(t *testing.T) { t.Fatalf("error waiting for stream creation: %v", err) } - fmt.Fprint(stream, "hello1") - fmt.Fprint(stream, "hello2") - fmt.Fprint(stream, "hello3") - fmt.Fprint(stream, "hello4") - fmt.Fprint(stream, "hello5") + _, _ = fmt.Fprint(stream, "hello1") + _, _ = fmt.Fprint(stream, "hello2") + _, _ = fmt.Fprint(stream, "hello3") + _, _ = fmt.Fprint(stream, "hello4") + _, _ = fmt.Fprint(stream, "hello5") - stream.Close() - conn.Close() + _ = stream.Close() + _ = conn.Close() // wait for the server to get the go away frame <-serverClosed @@ -1022,7 +1024,7 @@ func TestSetIdleTimeoutAfterRemoteConnectionClosed(t *testing.T) { } serverConn := <-serverConns - defer serverConn.Close() + defer func() { _ = serverConn.Close() }() <-serverConn.closeChan serverConn.SetIdleTimeout(10 * time.Second) @@ -1078,7 +1080,7 @@ func TestClientConnectionStopsServingAfterGoAway(t *testing.T) { }() serverConn := <-serverConns - serverConn.Close() + _ = serverConn.Close() // make sure the client conn breaks out of the main loop in Serve() <-spdyConn.closeChan @@ -1136,7 +1138,7 @@ func TestStreamReadUnblocksAfterCloseThenReset(t *testing.T) { }() serverConn := <-serverConns - defer serverConn.Close() + defer func() { _ = serverConn.Close() }() if err := stream.Close(); err != nil { t.Fatal(err) @@ -1153,8 +1155,10 @@ func TestStreamReadUnblocksAfterCloseThenReset(t *testing.T) { } } -var authLock sync.RWMutex -var authenticated bool +var ( + authLock sync.RWMutex + authenticated bool +) func setAuthenticated(b bool) { authLock.Lock() @@ -1166,7 +1170,7 @@ func authStreamHandler(stream *Stream) { authLock.RLock() defer authLock.RUnlock() if !authenticated { - stream.Refuse() + _ = stream.Refuse() return } MirrorStreamHandler(stream) @@ -1187,7 +1191,6 @@ func runServer(wg *sync.WaitGroup) (io.Closer, string, error) { spdyConn, _ := NewConnection(conn, true) go spdyConn.Serve(authStreamHandler) - } wg.Done() }() diff --git a/stream.go b/stream.go index 171c1e9..e8bcd98 100644 --- a/stream.go +++ b/stream.go @@ -28,9 +28,9 @@ import ( "github.com/moby/spdystream/spdy" ) -var ( - ErrUnreadPartialData = errors.New("unread partial data") -) +// ErrUnreadPartialData is returned by ReadData when it is called while there is +// still unread data buffered from a previous Read call. +var ErrUnreadPartialData = errors.New("unread partial data") type Stream struct { streamId spdy.StreamId @@ -85,7 +85,7 @@ func (s *Stream) Write(data []byte) (n int, err error) { if err == nil { n = len(data) } - return + return n, err } // Read reads bytes from a stream, a single read will never get more @@ -109,12 +109,12 @@ func (s *Stream) Read(p []byte) (n int, err error) { } else { s.unread = nil } - return + return n, nil } // ReadData reads an entire data frame and returns the byte array -// from the data frame. If there is unread data from the result -// of a Read call, this function will return an ErrUnreadPartialData. +// from the data frame. If there is unread data from the result +// of a Read call, this function returns an [ErrUnreadPartialData]. func (s *Stream) ReadData() ([]byte, error) { debugMessage("(%p) Reading data from %d", s, s.streamId) if s.unread != nil { @@ -147,7 +147,7 @@ func (s *Stream) Wait() error { } // WaitTimeout waits for the stream to receive a reply or for timeout. -// When the timeout is reached, ErrTimeout will be returned. +// If the timeout is reached, [ErrTimeout] is returned. func (s *Stream) WaitTimeout(timeout time.Duration) error { var timeoutChan <-chan time.Time if timeout > time.Duration(0) { @@ -247,7 +247,7 @@ func (s *Stream) SendReply(headers http.Header, fin bool) error { } // Refuse sends a reset frame with the status refuse, only -// valid to be called once when handling a new stream. This +// valid to be called once when handling a new stream. This // may be used to indicate that a stream is not allowed // when http status codes are not being used. func (s *Stream) Refuse() error { @@ -266,7 +266,7 @@ func (s *Stream) Cancel() error { } // ReceiveHeader receives a header sent on the other side -// of the stream. This function will block until a header +// of the stream. This function will block until a header // is received or stream is closed. func (s *Stream) ReceiveHeader() (http.Header, error) { select { @@ -274,11 +274,11 @@ func (s *Stream) ReceiveHeader() (http.Header, error) { break case header, ok := <-s.headerChan: if !ok { - return nil, fmt.Errorf("header chan closed") + return nil, errors.New("header chan closed") } return header, nil } - return nil, fmt.Errorf("stream closed") + return nil, errors.New("stream closed") } // Parent returns the parent stream diff --git a/utils.go b/utils.go index e9f7fff..84bac0b 100644 --- a/utils.go +++ b/utils.go @@ -21,9 +21,7 @@ import ( "os" ) -var ( - DEBUG = os.Getenv("DEBUG") -) +var DEBUG = os.Getenv("DEBUG") func debugMessage(fmt string, args ...interface{}) { if DEBUG != "" {