You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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")
deferf.Close()
r, _:=pcapgo.NewReader(f)
pktsrc:=gopacket.NewPacketSource(r, r.LinkType())
f.Close() // close the reader hereforpacket:=rangepktsrc.Packets() { // this will block after the last packets// do something
}
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.
The text was updated successfully, but these errors were encountered:
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.
TL;DR
The error handling in
(*PacketSource).packetsToChannel()
is incomplete and may lead to starvation. A catch-all rule is missing.Minimal Example
Implementation
Behavior
The program hangs instead of terminating.
Analysis
gopacket/packet.go
Lines 815 to 845 in 32ee382
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()
returnsread /path/to/the/file: file already closed
, which wrapsos.ErrClosed
and could be tested witherrors.Is(err, os.ErrClosed)
, but is not equal to any existing error. Therefore it will not break out of thefor
loop and the channel will never be closed.Proposed fix
errors.Is
or its equivalent, noterr == ErrToCompare
.io.Reader
.I'd be happy to open a PR if the proposed fix sounds reasonable.
The text was updated successfully, but these errors were encountered: