`(*PacketSource).packetsToChannel()` error handling is incomplete
- Dominant language
- Go
- Stars
- 6.8k
- Forks
- 1.2k
- PR merge metrics
- No merged PRs in 30d
Description
## TL;DR
The error handling in `(*PacketSource).packetsToChannel()` is incomplete and may lead to starvation. A catch-all rule is missing.
## Minimal Example
### Implementation
```go
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
https://github.com/google/gopacket/blob/32ee38206866f44a74a6033ec26aeeb474506804/packet.go#L815-L845
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.
Contributor guide
Assessment
This issue has not been assessed yet.