(*PacketSource).packetsToChannel() error handling is incomplete

[gaukas]

TL;DR

The error handling in (*PacketSource).packetsToChannel() is incomplete and may lead to starvation. A catch-all rule is missing.

Minimal Example

Implementation

	f, _ := os.Open("../testdata/somepackets.pcap")
	defer f.Close()
	r, _ := pcapgo.NewReader(f)

	pktsrc := gopacket.NewPacketSource(r, r.LinkType())
	
	f.Close() // close the reader here

	for packet := range pktsrc.Packets() { // this will block after the last packets
		// do something
	}

Behavior

The program hangs instead of terminating.

Analysis

gopacket/packet.go

Lines 815 to 845 in 32ee382

	func (p *PacketSource) packetsToChannel() {
	defer close(p.c)
	for {
	packet, err := p.NextPacket()
	if err == nil {
	p.c <- packet
	continue
	}
	// Immediately retry for temporary network errors
	if nerr, ok := err.(net.Error); ok && nerr.Temporary() {
	continue
	}
	// Immediately retry for EAGAIN
	if err == syscall.EAGAIN {
	continue
	}
	// Immediately break for known unrecoverable errors
	if err == io.EOF || err == io.ErrUnexpectedEOF ||
	err == io.ErrNoProgress || err == io.ErrClosedPipe || err == io.ErrShortBuffer ||
	err == syscall.EBADF ||
	strings.Contains(err.Error(), "use of closed file") {
	break
	}
	// Sleep briefly and try again
	time.Sleep(time.Millisecond * time.Duration(5))
	}
	}

The function implementation missed the case where the returned error is not matched to any known cases.

In the current latest version of Go (1.21.4), (*os.File).Read() returns read /path/to/the/file: file already closed, which wraps os.ErrClosed and could be tested with errors.Is(err, os.ErrClosed), but is not equal to any existing error. Therefore it will not break out of the for loop and the channel will never be closed.

Proposed fix

• Testing against known errors should use errors.Is or its equivalent, not err == ErrToCompare.
• Should also add a catch-all case for unknown errors, since it is possible for the user to use a custom io.Reader.

I'd be happy to open a PR if the proposed fix sounds reasonable.

[gaukas]

Just realized errors.Is is added in Go 1.13. Since the go.mod of this package still specifies go 1.12, either we will create version-dependent packet.go file for Go 1.13 and above, or we shall bump up the minimum required version.